Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Rework the logging for controller_instance. #1371

Merged
merged 12 commits into from
Oct 13, 2017

Conversation

n3wscott
Copy link
Contributor

After working with controller bindings, I learned some things about how we want to log info. Updating last PR to be more like the new way.

Part of #1279

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 11, 2017
Copy link
Contributor

@pmorie pmorie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Appreciate the follow-up greatly; can you be sure to disambiguate k8s/external name where that isn't already done?

@@ -264,8 +267,11 @@ func (c *controller) getClusterServiceClassPlanAndClusterServiceBroker(instance

servicePlan, err := c.servicePlanLister.Get(instance.Spec.ClusterServicePlanRef.Name)
if nil != err {
s := fmt.Sprintf(`ServiceInstance "%v/%v": references a non-existent ClusterServicePlan %q on ClusterServiceClass %q1`, instance.Namespace, instance.Name, instance.Spec.ExternalClusterServicePlanName, serviceClass.Spec.ExternalName)
glog.Warning(s)
s := fmt.Sprintf("References a non-existent ClusterServicePlan %q on ClusterServiceClass %q", instance.Spec.ExternalClusterServicePlanName, serviceClass.Spec.ExternalName)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we disambiguate the k8s and external names in this PR where it isn't already done?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure! I can add that.

@k8s-ci-robot k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 12, 2017
@n3wscott
Copy link
Contributor Author

@pmorie take another look, thanks!

@vaikas vaikas added the LGTM1 label Oct 13, 2017
@pmorie pmorie added the LGTM2 label Oct 13, 2017
@pmorie pmorie merged commit 07d3068 into kubernetes-retired:master Oct 13, 2017
@pmorie pmorie added this to the 0.1.0-rc2 milestone Oct 17, 2017
@n3wscott n3wscott deleted the controller_instancev2 branch October 18, 2017 22:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 LGTM2 size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants