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

Move serviceBroker references to clusterServiceBroker in controllers #1911

Merged
merged 11 commits into from
Apr 30, 2018
Merged

Move serviceBroker references to clusterServiceBroker in controllers #1911

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
d9079e8
Rename broker controller files
eriknelson Apr 4, 2018
7a685d7
Remove extra space
eriknelson Apr 4, 2018
d6b60d6
Update controller refs to ClusterServiceBroker
eriknelson Apr 4, 2018
4e5b28b
Add forgotten rename update
eriknelson Apr 4, 2018
648ae00
Fix doc href after file rename
eriknelson Apr 5, 2018
9536ef4
PR feedback
eriknelson Apr 5, 2018
d8eb2df
Add missing gofmt
eriknelson Apr 6, 2018
f91c5fe
Update status text matching new reason/messages
eriknelson Apr 6, 2018
1dd741a
Revert "ClusterCatalog" to "Catalog"
eriknelson Apr 24, 2018
da2d2dd
Revert const val changes
eriknelson Apr 29, 2018
bb917d1
Additional feedback items
eriknelson Apr 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions contrib/examples/prometheus/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ into the expression filter you should see metrics from Service Catlog.
really simple to add additional metrics, or drop me (jboyd01) a note if you have
ideas but not the time to implement. If you want to add metrics, briefly review
[pkg/metrics/metrics.go](../../../pkg/metrics/metrics.go) and
[pkg/controller/controller_broker.go](../../../pkg/controller/controller_broker.go)
[pkg/controller/controller_clusterservicebroker.go](../../../pkg/controller/controller_clusterservicebroker.go)
for reference.

## Useful metrics queries
Expand All @@ -86,4 +86,4 @@ tbd

Getting started with Prometheus: https://prometheus.io/docs/prometheus/latest/getting_started/
Basics for Querying Prometheus: https://prometheus.io/docs/prometheus/latest/querying/basics/
Instrumenting your App: https://godoc.org/github.com/prometheus/client_golang/prometheus
Instrumenting your App: https://godoc.org/github.com/prometheus/client_golang/prometheus
6 changes: 3 additions & 3 deletions docs/walkthrough.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ We can check the status of the broker:
$ svcat describe broker ups-broker
Name: ups-broker
URL: http://ups-broker-ups-broker.ups-broker.svc.cluster.local
Status: Ready - Successfully fetched catalog entries from broker @ 2018-03-02 16:03:52 +0000 UTC
Status: Ready - Successfully fetched cluster catalog entries from broker @ 2018-03-02 16:03:52 +0000 UTC

$ kubectl get clusterservicebrokers ups-broker -o yaml
apiVersion: servicecatalog.k8s.io/v1beta1
Expand All @@ -88,8 +88,8 @@ spec:
status:
conditions:
- lastTransitionTime: 2017-11-01T14:12:30Z
message: Successfully fetched catalog entries from broker.
reason: FetchedCatalog
message: Successfully fetched cluster catalog entries from broker.
Copy link
Contributor

Choose a reason for hiding this comment

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

Now this doc should have the changes reverted and we'll be good to go.

reason: FetchedClusterCatalog
status: "True"
type: Ready
reconciledGeneration: 1
Expand Down
22 changes: 11 additions & 11 deletions pkg/controller/controller.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ func NewController(
OSBAPIPreferredVersion: osbAPIPreferredVersion,
recorder: recorder,
reconciliationRetryDuration: reconciliationRetryDuration,
brokerQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "service-broker"),
clusterServiceBrokerQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "cluster-service-broker"),
clusterServiceClassQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "cluster-service-class"),
clusterServicePlanQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "cluster-service-plan"),
instanceQueue: workqueue.NewNamedRateLimitingQueue(workqueue.DefaultControllerRateLimiter(), "service-instance"),
Expand All @@ -105,11 +105,11 @@ func NewController(
clusterIDConfigMapNamespace: clusterIDConfigMapNamespace,
}

controller.brokerLister = brokerInformer.Lister()
controller.clusterServiceBrokerLister = brokerInformer.Lister()
brokerInformer.Informer().AddEventHandler(cache.ResourceEventHandlerFuncs{
AddFunc: controller.brokerAdd,
UpdateFunc: controller.brokerUpdate,
DeleteFunc: controller.brokerDelete,
AddFunc: controller.clusterServiceBrokerAdd,
UpdateFunc: controller.clusterServiceBrokerUpdate,
DeleteFunc: controller.clusterServiceBrokerDelete,
})

controller.clusterServiceClassLister = clusterServiceClassInformer.Lister()
Expand Down Expand Up @@ -157,7 +157,7 @@ type controller struct {
kubeClient kubernetes.Interface
serviceCatalogClient servicecatalogclientset.ServicecatalogV1beta1Interface
brokerClientCreateFunc osb.CreateFunc
brokerLister listers.ClusterServiceBrokerLister
clusterServiceBrokerLister listers.ClusterServiceBrokerLister
clusterServiceClassLister listers.ClusterServiceClassLister
instanceLister listers.ServiceInstanceLister
bindingLister listers.ServiceBindingLister
Expand All @@ -166,7 +166,7 @@ type controller struct {
OSBAPIPreferredVersion string
recorder record.EventRecorder
reconciliationRetryDuration time.Duration
brokerQueue workqueue.RateLimitingInterface
clusterServiceBrokerQueue workqueue.RateLimitingInterface
clusterServiceClassQueue workqueue.RateLimitingInterface
clusterServicePlanQueue workqueue.RateLimitingInterface
instanceQueue workqueue.RateLimitingInterface
Expand Down Expand Up @@ -199,7 +199,7 @@ func (c *controller) Run(workers int, stopCh <-chan struct{}) {
var waitGroup sync.WaitGroup

for i := 0; i < workers; i++ {
createWorker(c.brokerQueue, "ClusterServiceBroker", maxRetries, true, c.reconcileClusterServiceBrokerKey, stopCh, &waitGroup)
createWorker(c.clusterServiceBrokerQueue, "ClusterServiceBroker", maxRetries, true, c.reconcileClusterServiceBrokerKey, stopCh, &waitGroup)
createWorker(c.clusterServiceClassQueue, "ClusterServiceClass", maxRetries, true, c.reconcileClusterServiceClassKey, stopCh, &waitGroup)
createWorker(c.clusterServicePlanQueue, "ClusterServicePlan", maxRetries, true, c.reconcileClusterServicePlanKey, stopCh, &waitGroup)
createWorker(c.instanceQueue, "ServiceInstance", maxRetries, true, c.reconcileServiceInstanceKey, stopCh, &waitGroup)
Expand All @@ -220,7 +220,7 @@ func (c *controller) Run(workers int, stopCh <-chan struct{}) {
<-stopCh
glog.Info("Shutting down service-catalog controller")

c.brokerQueue.ShutDown()
c.clusterServiceBrokerQueue.ShutDown()
c.clusterServiceClassQueue.ShutDown()
c.clusterServicePlanQueue.ShutDown()
c.instanceQueue.ShutDown()
Expand Down Expand Up @@ -388,7 +388,7 @@ func (c *controller) getClusterServiceClassAndClusterServiceBroker(instance *v1b
}
}

broker, err := c.brokerLister.Get(serviceClass.Spec.ClusterServiceBrokerName)
broker, err := c.clusterServiceBrokerLister.Get(serviceClass.Spec.ClusterServiceBrokerName)
if err != nil {
return nil, "", nil, &operationError{
reason: errorNonexistentClusterServiceBrokerReason,
Expand Down Expand Up @@ -463,7 +463,7 @@ func (c *controller) getClusterServiceClassPlanAndClusterServiceBrokerForService
return nil, nil, "", nil, fmt.Errorf(s)
}

broker, err := c.brokerLister.Get(serviceClass.Spec.ClusterServiceBrokerName)
broker, err := c.clusterServiceBrokerLister.Get(serviceClass.Spec.ClusterServiceBrokerName)
if err != nil {
s := fmt.Sprintf("References a non-existent ClusterServiceBroker %q", serviceClass.Spec.ClusterServiceBrokerName)
glog.Warning(pcb.Message(s))
Expand Down
16 changes: 8 additions & 8 deletions pkg/controller/controller_broker.go → ...roller/controller_clusterservicebroker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ import (
// be easily combined with a follow on specific message.
const (
errorFetchingCatalogReason string = "ErrorFetchingCatalog"
errorFetchingCatalogMessage string = "Error fetching catalog. "
errorFetchingCatalogMessage string = "Error fetching catalog."
Copy link
Contributor

Choose a reason for hiding this comment

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

Note the trailing spaces; those are intentional because these strings are use to construct error messages. Would you restore the trailing spaces?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 Didn't realize those were deliberate; that makes sense.

errorSyncingCatalogReason string = "ErrorSyncingCatalog"
errorSyncingCatalogMessage string = "Error syncing catalog from ServiceBroker. "
errorSyncingCatalogMessage string = "Error syncing catalog from ServiceBroker."
Copy link
Contributor

Choose a reason for hiding this comment

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

s/ServiceBroker/ClusterServiceBroker

Copy link
Contributor

Choose a reason for hiding this comment

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

Also I suspect we will have the same reasons/messages for namespaced service brokers, so we should probably prefix the variable names with cluster- too.


errorListingClusterServiceClassesReason string = "ErrorListingServiceClasses"
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably missed in #1910: should error reason and message have Cluster?

errorListingClusterServiceClassesMessage string = "Error listing service classes."
Expand All @@ -61,7 +61,7 @@ const (
errorReconciliationRetryTimeoutReason string = "ErrorReconciliationRetryTimeout"
)

func (c *controller) brokerAdd(obj interface{}) {
func (c *controller) clusterServiceBrokerAdd(obj interface{}) {
// DeletionHandlingMetaNamespaceKeyFunc returns a unique key for the resource and
// handles the special case where the resource is of DeletedFinalStateUnknown type, which
// acts a place holder for resources that have been deleted from storage but the watch event
Expand All @@ -73,14 +73,14 @@ func (c *controller) brokerAdd(obj interface{}) {
glog.Errorf("Couldn't get key for object %+v: %v", obj, err)
return
}
c.brokerQueue.Add(key)
c.clusterServiceBrokerQueue.Add(key)
}

func (c *controller) brokerUpdate(oldObj, newObj interface{}) {
c.brokerAdd(newObj)
func (c *controller) clusterServiceBrokerUpdate(oldObj, newObj interface{}) {
c.clusterServiceBrokerAdd(newObj)
}

func (c *controller) brokerDelete(obj interface{}) {
func (c *controller) clusterServiceBrokerDelete(obj interface{}) {
broker, ok := obj.(*v1beta1.ClusterServiceBroker)
if broker == nil || !ok {
return
Expand Down Expand Up @@ -151,7 +151,7 @@ func shouldReconcileClusterServiceBroker(broker *v1beta1.ClusterServiceBroker, n
}

func (c *controller) reconcileClusterServiceBrokerKey(key string) error {
broker, err := c.brokerLister.Get(key)
broker, err := c.clusterServiceBrokerLister.Get(key)
pcb := pretty.NewContextBuilder(pretty.ClusterServiceBroker, "", key)
if errors.IsNotFound(err) {
glog.Info(pcb.Message("Not doing work because it has been deleted"))
Expand Down
0 pkg/controller/controller_broker_test.go → ...r/controller_clusterservicebroker_test.go
View file
Open in desktop
File renamed without changes.