Skip to content

Commit 0e90d82

Browse files
n3wscottarschles
n3wscott
authored and
arschles
committed
Add a pretty formatter for ClusterService[Class|Plan] (#1408)
* First example using the log context builder. * No format. * Remote controller logging class, not needed yet. * Fixing cr date. * Adding function documentation. * Kind is always type Kind. * Start of log names. * Add comment for type. * I feel so pretty. Oh so pretty. Pretty log lines. * Move type to it's own file. * Renaming files, fixing receiver names. * Working on an example for Pretty Names for classes that have internal and external names. * Remove old logging file. * Merge. * Finish controller.go. * Fix comment. * finished controller_broker.go as an example. * Fixing unit test.
1 parent fb874df commit 0e90d82

File tree

5 files changed

+203
-76
lines changed

5 files changed

+203
-76
lines changed

‎pkg/controller/controller_broker.go

Lines changed: 48 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import (
2525

2626
"github.com/kubernetes-incubator/service-catalog/pkg/api"
2727
"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1"
28+
"github.com/kubernetes-incubator/service-catalog/pkg/pretty"
2829
corev1 "k8s.io/api/core/v1"
2930
"k8s.io/apimachinery/pkg/api/errors"
3031
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
@@ -341,13 +342,13 @@ func (c *controller) reconcileClusterServiceBroker(broker *v1beta1.ClusterServic
341342
delete(existingServicePlanMap, payloadServicePlan.Name)
342343

343344
glog.V(4).Infof(
344-
"%s %q: reconciling ClusterServicePlan (K8S: %q ExternalName: %q)",
345-
typeCSB, broker.Name, payloadServicePlan.Name, payloadServicePlan.Spec.ExternalName,
345+
"ClusterServiceBroker %q: reconciling %s",
346+
broker.Name, pretty.ClusterServicePlanName(payloadServicePlan),
346347
)
347348
if err := c.reconcileClusterServicePlanFromClusterServiceBrokerCatalog(broker, payloadServicePlan, existingServicePlan); err != nil {
348349
s := fmt.Sprintf(
349-
"Error reconciling ClusterServicePlan (K8S: %q ExternalName: %q): %s",
350-
payloadServicePlan.Name, payloadServicePlan.Spec.ExternalName, err,
350+
"Error reconciling %s: %s",
351+
pretty.ClusterServicePlanName(payloadServicePlan), err,
351352
)
352353
glog.Warningf(
353354
"%s %q: %s",
@@ -359,8 +360,8 @@ func (c *controller) reconcileClusterServiceBroker(broker *v1beta1.ClusterServic
359360
return err
360361
}
361362
glog.V(5).Infof(
362-
"%s %q: Reconciled ClusterServicePlan (K8S: %q ExternalName: %q)",
363-
typeCSB, broker.Name, payloadServicePlan.Name, payloadServicePlan.Spec.ExternalName,
363+
"%s %q: Reconciled %s",
364+
typeCSB, broker.Name, pretty.ClusterServicePlanName(payloadServicePlan),
364365
)
365366
}
366367

@@ -371,16 +372,15 @@ func (c *controller) reconcileClusterServiceBroker(broker *v1beta1.ClusterServic
371372
continue
372373
}
373374
glog.V(4).Infof(
374-
"%s %q: ClusterServicePlan (K8S: %q ExternalName: %q) has been removed from broker's catalog; marking",
375-
typeCSB, broker.Name, existingServicePlan.Name, existingServicePlan.Spec.ExternalName,
375+
"%s %q: %s has been removed from broker's catalog; marking",
376+
typeCSB, pretty.ClusterServicePlanName(existingServicePlan),
376377
)
377378
existingServicePlan.Status.RemovedFromBrokerCatalog = true
378379
_, err := c.serviceCatalogClient.ClusterServicePlans().UpdateStatus(existingServicePlan)
379380
if err != nil {
380381
s := fmt.Sprintf(
381-
"Error updating status of ClusterServicePlan (K8S: %q ExternalName: %q): %v",
382-
existingServicePlan.Name,
383-
existingServicePlan.Spec.ExternalName,
382+
"Error updating status of %s: %v",
383+
pretty.ClusterServicePlanName(existingServicePlan),
384384
err,
385385
)
386386
glog.Warningf(
@@ -403,13 +403,13 @@ func (c *controller) reconcileClusterServiceBroker(broker *v1beta1.ClusterServic
403403
delete(existingServiceClassMap, payloadServiceClass.Name)
404404

405405
glog.V(4).Infof(
406-
"%s %q: Reconciling ClusterServiceClass (K8S: %q ExternalName: %q)",
407-
typeCSB, broker.Name, payloadServiceClass.Name, payloadServiceClass.Spec.ExternalName,
406+
"%s %q: Reconciling %s",
407+
typeCSB, broker.Name, pretty.ClusterServiceClassName(payloadServiceClass),
408408
)
409409
if err := c.reconcileClusterServiceClassFromClusterServiceBrokerCatalog(broker, payloadServiceClass, existingServiceClass); err != nil {
410410
s := fmt.Sprintf(
411-
"Error reconciling ClusterServiceClass (K8S: %q ExternalName: %q) (broker %q): %s",
412-
payloadServiceClass.Name, payloadServiceClass.Spec.ExternalName, broker.Name, err,
411+
"Error reconciling %s (broker %q): %s",
412+
pretty.ClusterServiceClassName(payloadServiceClass), broker.Name, err,
413413
)
414414
glog.Warningf(
415415
`%s %q: %s`,
@@ -424,8 +424,8 @@ func (c *controller) reconcileClusterServiceBroker(broker *v1beta1.ClusterServic
424424
}
425425

426426
glog.V(5).Infof(
427-
"%s %q: Reconciled ClusterServiceClass (K8S: %q ExternalName: %q)",
428-
typeCSB, broker.Name, payloadServiceClass.Name, payloadServiceClass.Spec.ExternalName,
427+
"%s %q: Reconciled %s",
428+
typeCSB, broker.Name, pretty.ClusterServiceClassName(payloadServiceClass),
429429
)
430430
}
431431

@@ -437,17 +437,15 @@ func (c *controller) reconcileClusterServiceBroker(broker *v1beta1.ClusterServic
437437
}
438438

439439
glog.V(4).Infof(
440-
"%s %q: ClusterServiceClass (K8S: %q ExternalName: %q) has been removed from broker's catalog; marking",
441-
typeCSB, broker.Name, existingServiceClass.Name, existingServiceClass.Spec.ExternalName,
440+
"%s %q: %s has been removed from broker's catalog; marking",
441+
typeCSB, broker.Name, pretty.ClusterServiceClassName(existingServiceClass),
442442
)
443443
existingServiceClass.Status.RemovedFromBrokerCatalog = true
444444
_, err := c.serviceCatalogClient.ClusterServiceClasses().UpdateStatus(existingServiceClass)
445445
if err != nil {
446446
s := fmt.Sprintf(
447-
"Error updating status of ClusterServiceClass (K8S: %q ExternalName: %q): %v",
448-
existingServiceClass.Name,
449-
existingServiceClass.Spec.ExternalName,
450-
err,
447+
"Error updating status of %s: %v",
448+
pretty.ClusterServiceClassName(existingServiceClass), err,
451449
)
452450
glog.Warningf(
453451
"%s %q: %s",
@@ -493,16 +491,10 @@ func (c *controller) reconcileClusterServiceBroker(broker *v1beta1.ClusterServic
493491
)
494492

495493
for _, plan := range existingServicePlans {
496-
glog.V(4).Infof(
497-
"%s %q: deleting ClusterServicePlan (K8S: %q ExternalName: %q)",
498-
typeCSB, broker.Name, plan.Name, plan.Spec.ExternalName,
499-
)
494+
glog.V(4).Infof("%s %q: deleting %s", typeCSB, broker.Name, pretty.ClusterServicePlanName(&plan))
500495
err := c.serviceCatalogClient.ClusterServicePlans().Delete(plan.Name, &metav1.DeleteOptions{})
501496
if err != nil && !errors.IsNotFound(err) {
502-
s := fmt.Sprintf(
503-
"Error deleting ClusterServicePlan (K8S: %q ExternalName: %q): %s",
504-
plan.Name, plan.Spec.ExternalName, err,
505-
)
497+
s := fmt.Sprintf("Error deleting %s: %s", pretty.ClusterServicePlanName(&plan), err)
506498
glog.Warningf(
507499
"%s %q: %s",
508500
typeCSB, broker.Name, s,
@@ -521,15 +513,12 @@ func (c *controller) reconcileClusterServiceBroker(broker *v1beta1.ClusterServic
521513

522514
for _, svcClass := range existingServiceClasses {
523515
glog.V(4).Infof(
524-
"%s %q: deleting ClusterServiceClass (K8S: %q ExternalName: %q)",
525-
typeCSB, broker.Name, svcClass.Name, svcClass.Spec.ExternalName,
516+
"%s %q: deleting %s",
517+
typeCSB, broker.Name, pretty.ClusterServiceClassName(&svcClass),
526518
)
527519
err = c.serviceCatalogClient.ClusterServiceClasses().Delete(svcClass.Name, &metav1.DeleteOptions{})
528520
if err != nil && !errors.IsNotFound(err) {
529-
s := fmt.Sprintf(
530-
"Error deleting ClusterServiceClass (K8S: %q ExternalName: %q): %s",
531-
svcClass.Name, svcClass.Spec.ExternalName, err,
532-
)
521+
s := fmt.Sprintf("Error deleting %s: %s", pretty.ClusterServiceClassName(&svcClass), err)
533522
glog.Warningf(
534523
"%s %q: %s",
535524
typeCSB, broker.Name, s,
@@ -591,22 +580,22 @@ func (c *controller) reconcileClusterServiceClassFromClusterServiceBrokerCatalog
591580
// certainly evaluate to true.
592581
if otherServiceClass.Spec.ClusterServiceBrokerName != broker.Name {
593582
errMsg := fmt.Sprintf(
594-
"%s %q: ClusterServiceClass (K8S: %q ExternalName: %q) already exists for Broker %q",
595-
typeCSB, broker.Name, serviceClass.Name, serviceClass.Spec.ExternalName, otherServiceClass.Spec.ClusterServiceBrokerName,
583+
"%s %q: %s already exists for Broker %q",
584+
typeCSB, broker.Name, pretty.ClusterServiceClassName(serviceClass), otherServiceClass.Spec.ClusterServiceBrokerName,
596585
)
597586
glog.Error(errMsg)
598587
return fmt.Errorf(errMsg)
599588
}
600589
}
601590

602591
glog.V(5).Infof(
603-
"%s %q: fresh ClusterServiceClass (K8S: %q ExternalName: %q); creating",
604-
typeCSB, broker.Name, serviceClass.Name, serviceClass.Spec.ExternalName,
592+
"%s %q: fresh %s; creating",
593+
typeCSB, broker.Name, pretty.ClusterServiceClassName(serviceClass),
605594
)
606595
if _, err := c.serviceCatalogClient.ClusterServiceClasses().Create(serviceClass); err != nil {
607596
glog.Errorf(
608-
"%s %q: Error creating ClusterServiceClass (K8S: %q ExternalName: %q): %v",
609-
typeCSB, broker.Name, serviceClass.Name, serviceClass.Spec.ExternalName, err,
597+
"%s %q: Error creating %s: %v",
598+
typeCSB, broker.Name, pretty.ClusterServiceClassName(serviceClass), err,
610599
)
611600
return err
612601
}
@@ -616,16 +605,16 @@ func (c *controller) reconcileClusterServiceClassFromClusterServiceBrokerCatalog
616605

617606
if existingServiceClass.Spec.ExternalID != serviceClass.Spec.ExternalID {
618607
errMsg := fmt.Sprintf(
619-
"%s %q: ClusterServiceClass (K8S: %q ExternalName: %q) already exists with OSB guid %q, received different guid %q",
620-
typeCSB, broker.Name, serviceClass.Name, serviceClass.Spec.ExternalName, existingServiceClass.Name, serviceClass.Name,
608+
"%s %q: %s already exists with OSB guid %q, received different guid %q",
609+
typeCSB, broker.Name, pretty.ClusterServiceClassName(serviceClass), existingServiceClass.Name, serviceClass.Name,
621610
)
622611
glog.Error(errMsg)
623612
return fmt.Errorf(errMsg)
624613
}
625614

626615
glog.V(5).Infof(
627-
"%s %q: Found existing ClusterServiceClass (K8S: %q ExternalName: %q); updating",
628-
typeCSB, broker.Name, serviceClass.Name, serviceClass.Spec.ExternalName,
616+
"%s %q: Found existing %s; updating",
617+
typeCSB, broker.Name, pretty.ClusterServiceClassName(serviceClass),
629618
)
630619

631620
// There was an existing service class -- project the update onto it and
@@ -646,8 +635,8 @@ func (c *controller) reconcileClusterServiceClassFromClusterServiceBrokerCatalog
646635

647636
if _, err := c.serviceCatalogClient.ClusterServiceClasses().Update(toUpdate); err != nil {
648637
glog.Errorf(
649-
"%s %q: Error updating ClusterServiceClass (K8S: %q ExternalName: %q): %v",
650-
typeCSB, broker.Name, serviceClass.Name, serviceClass.Spec.ExternalName, err,
638+
"%s %q: Error updating %s: %v",
639+
typeCSB, broker.Name, pretty.ClusterServiceClassName(serviceClass), err,
651640
)
652641
return err
653642
}
@@ -674,8 +663,8 @@ func (c *controller) reconcileClusterServicePlanFromClusterServiceBrokerCatalog(
674663
// certainly evaluate to true.
675664
if otherServicePlan.Spec.ClusterServiceBrokerName != broker.Name {
676665
errMsg := fmt.Sprintf(
677-
"ClusterServicePlan (K8S: %q ExternalName: %q) already exists for Broker %q",
678-
servicePlan.Name, servicePlan.Spec.ExternalName, otherServicePlan.Spec.ClusterServiceBrokerName,
666+
"%s already exists for Broker %q",
667+
pretty.ClusterServicePlanName(servicePlan), otherServicePlan.Spec.ClusterServiceBrokerName,
679668
)
680669
glog.Errorf(
681670
`%s %q: %s`,
@@ -689,8 +678,8 @@ func (c *controller) reconcileClusterServicePlanFromClusterServiceBrokerCatalog(
689678
// not exist. Create a new ClusterServicePlan.
690679
if _, err := c.serviceCatalogClient.ClusterServicePlans().Create(servicePlan); err != nil {
691680
glog.Errorf(
692-
"%s %q: Error creating ClusterServicePlan (K8S: %q, ExternalName: %q): %v",
693-
typeCSB, broker.Name, servicePlan.Name, servicePlan.Spec.ExternalName, err,
681+
"%s %q: Error creating %s: %v",
682+
typeCSB, broker.Name, pretty.ClusterServicePlanName(servicePlan), err,
694683
)
695684
return err
696685
}
@@ -700,8 +689,8 @@ func (c *controller) reconcileClusterServicePlanFromClusterServiceBrokerCatalog(
700689

701690
if existingServicePlan.Spec.ExternalID != servicePlan.Spec.ExternalID {
702691
errMsg := fmt.Sprintf(
703-
"ClusterServicePlan (K8S: %q ExternalName: %q) already exists with OSB guid %q, received different guid %q",
704-
servicePlan.Name, servicePlan.Spec.ExternalName, existingServicePlan.Spec.ExternalID, servicePlan.Spec.ExternalID,
692+
"%s already exists with OSB guid %q, received different guid %q",
693+
pretty.ClusterServicePlanName(servicePlan), existingServicePlan.Spec.ExternalID, servicePlan.Spec.ExternalID,
705694
)
706695
glog.Errorf(
707696
"%s %q: %s",
@@ -711,8 +700,8 @@ func (c *controller) reconcileClusterServicePlanFromClusterServiceBrokerCatalog(
711700
}
712701

713702
glog.V(5).Infof(
714-
"%s %q: Found existing ClusterServicePlan (K8S: %q ExternalName: %q); updating",
715-
typeCSB, broker.Name, servicePlan.Name, servicePlan.Spec.ExternalName,
703+
"%s %q: Found existing %s; updating",
704+
typeCSB, broker.Name, pretty.ClusterServicePlanName(servicePlan),
716705
)
717706

718707
// There was an existing service plan -- project the update onto it and
@@ -734,8 +723,8 @@ func (c *controller) reconcileClusterServicePlanFromClusterServiceBrokerCatalog(
734723

735724
if _, err := c.serviceCatalogClient.ClusterServicePlans().Update(toUpdate); err != nil {
736725
glog.Errorf(
737-
"%s %q: Error updating ClusterServicePlan (K8S: %q ExternalName: %q): %v",
738-
typeCSB, broker.Name, servicePlan.Name, servicePlan.Spec.ExternalName, err,
726+
"%s %q: Error updating %s: %v",
727+
typeCSB, broker.Name, pretty.ClusterServicePlanName(servicePlan), err,
739728
)
740729
return err
741730
}

‎pkg/pretty/context_builder.go

Lines changed: 0 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,23 +20,6 @@ import (
2020
"fmt"
2121
)
2222

23-
// Kind is used for the enum of the Type of object we are building context for.
24-
type Kind int
25-
26-
// Names of Types to use when creating pretty messages.
27-
const (
28-
ServiceInstance Kind = 1
29-
)
30-
31-
func (k Kind) String() string {
32-
switch k {
33-
case ServiceInstance:
34-
return "ServiceInstance"
35-
default:
36-
return ""
37-
}
38-
}
39-
4023
// ContextBuilder allows building up pretty message lines with context
4124
// that is important for debugging and tracing. This class helps create log
4225
// line formatting consistency. Pretty lines should be in the form:

‎pkg/pretty/pretty_kind.go

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
Copyright 2017 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package pretty
18+
19+
// Kind is used for the enum of the Type of object we are building context for.
20+
type Kind int
21+
22+
// Names of Types to use when creating pretty messages
23+
const (
24+
Unknown Kind = iota
25+
ClusterServiceClass
26+
ClusterServicePlan
27+
ServiceInstance
28+
)
29+
30+
func (k Kind) String() string {
31+
switch k {
32+
case ClusterServiceClass:
33+
return "ClusterServiceClass"
34+
case ClusterServicePlan:
35+
return "ClusterServicePlan"
36+
case ServiceInstance:
37+
return "ServiceInstance"
38+
default:
39+
return ""
40+
}
41+
}

‎pkg/pretty/pretty_names.go

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
/*
2+
Copyright 2017 The Kubernetes Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package pretty
18+
19+
import (
20+
"fmt"
21+
22+
"github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1"
23+
)
24+
25+
// Name prints in the form `<Kind> (K8S: <K8S-Name> ExternalName: <External-Name>)`
26+
// kind is required. k8sName and externalName are optional
27+
func Name(kind Kind, k8sName, externalName string) string {
28+
s := fmt.Sprintf("%s", kind)
29+
if k8sName != "" && externalName != "" {
30+
s += fmt.Sprintf(" (K8S: %q ExternalName: %q)", k8sName, externalName)
31+
} else if k8sName != "" {
32+
s += fmt.Sprintf(" (K8S: %q)", k8sName)
33+
} else if externalName != "" {
34+
s += fmt.Sprintf(" (ExternalName: %q)", externalName)
35+
}
36+
return s
37+
}
38+
39+
// ClusterServiceClassName returns a string with the k8s name and external name if available.
40+
func ClusterServiceClassName(serviceClass *v1beta1.ClusterServiceClass) string {
41+
if serviceClass != nil {
42+
return Name(ClusterServiceClass, serviceClass.Name, serviceClass.Spec.ExternalName)
43+
}
44+
return Name(ClusterServiceClass, "", "")
45+
}
46+
47+
// ClusterServicePlanName returns a string with the k8s name and external name if available.
48+
func ClusterServicePlanName(servicePlan *v1beta1.ClusterServicePlan) string {
49+
if servicePlan != nil {
50+
return Name(ClusterServicePlan, servicePlan.Name, servicePlan.Spec.ExternalName)
51+
}
52+
return Name(ClusterServicePlan, "", "")
53+
}

0 commit comments

Comments
 (0)