diff --git a/pkg/controller/controller_broker.go b/pkg/controller/controller_broker.go index dd24ffa3c880..158567bafce9 100644 --- a/pkg/controller/controller_broker.go +++ b/pkg/controller/controller_broker.go @@ -540,11 +540,27 @@ func (c *controller) reconcileClusterServiceClassFromClusterServiceBrokerCatalog toUpdate.Spec.ExternalName = serviceClass.Spec.ExternalName toUpdate.Spec.ExternalMetadata = serviceClass.Spec.ExternalMetadata - if _, err := c.serviceCatalogClient.ClusterServiceClasses().Update(toUpdate); err != nil { + updatedServiceClass, err := c.serviceCatalogClient.ClusterServiceClasses().Update(toUpdate) + if err != nil { glog.Error(pcb.Messagef("Error updating %s: %v", pretty.ClusterServiceClassName(serviceClass), err)) return err } + if updatedServiceClass.Status.RemovedFromBrokerCatalog { + glog.V(4).Info(pcb.Messagef("Resetting RemovedFromBrokerCatalog status on %s", pretty.ClusterServiceClassName(serviceClass))) + updatedServiceClass.Status.RemovedFromBrokerCatalog = false + _, err := c.serviceCatalogClient.ClusterServiceClasses().UpdateStatus(updatedServiceClass) + if err != nil { + s := fmt.Sprintf("Error updating status of %s: %v", pretty.ClusterServiceClassName(updatedServiceClass), err) + glog.Warning(pcb.Message(s)) + c.recorder.Eventf(broker, corev1.EventTypeWarning, errorSyncingCatalogReason, s) + if err := c.updateClusterServiceBrokerCondition(broker, v1beta1.ServiceBrokerConditionReady, v1beta1.ConditionFalse, errorSyncingCatalogReason, errorSyncingCatalogMessage+s); err != nil { + return err + } + return err + } + } + return nil } diff --git a/pkg/controller/controller_broker_test.go b/pkg/controller/controller_broker_test.go index 0c0af7188591..188454a4c563 100644 --- a/pkg/controller/controller_broker_test.go +++ b/pkg/controller/controller_broker_test.go @@ -318,6 +318,62 @@ func TestReconcileClusterServiceBrokerRemovedClusterServiceClass(t *testing.T) { assertNumberOfActions(t, kubeActions, 0) } +// TestReconcileClusterServiceBrokerRemovedAndRestoredClusterServiceClass +// validates where Service Catalog has a class that is marked as +// RemovedFromBrokerCatalog but then the ServiceBroker adds the class back into +// its getCatalog response. This should result in the class's status being +// updated resetting the RemovedFromBrokerCatalog to false. +func TestReconcileClusterServiceBrokerRemovedAndRestoredClusterServiceClass(t *testing.T) { + fakeKubeClient, fakeCatalogClient, fakeClusterServiceBrokerClient, testController, sharedInformers := newTestController(t, getTestCatalogConfig()) + + testClusterServiceClass := getTestClusterServiceClass() + testClusterServicePlan := getTestClusterServicePlan() + testClusterServicePlanNonbindable := getTestClusterServicePlanNonbindable() + testClusterServiceClass.Status.RemovedFromBrokerCatalog = true + sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(testClusterServiceClass) + + fakeCatalogClient.AddReactor("list", "clusterserviceclasses", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1beta1.ClusterServiceClassList{ + Items: []v1beta1.ClusterServiceClass{ + *testClusterServiceClass, + }, + }, nil + }) + + fakeCatalogClient.AddReactor("update", "clusterserviceclasses", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, testClusterServiceClass, nil + }) + + if err := testController.reconcileClusterServiceBroker(getTestClusterServiceBroker()); err != nil { + t.Fatalf("This should not fail: %v", err) + } + + brokerActions := fakeClusterServiceBrokerClient.Actions() + assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertGetCatalog(t, brokerActions[0]) + + listRestrictions := clientgotesting.ListRestrictions{ + Labels: labels.Everything(), + Fields: fields.OneTermEqualSelector("spec.clusterServiceBrokerName", "test-broker"), + } + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 7) + assertList(t, actions[0], &v1beta1.ClusterServiceClass{}, listRestrictions) + assertList(t, actions[1], &v1beta1.ClusterServicePlan{}, listRestrictions) + assertUpdate(t, actions[2], testClusterServiceClass) + class := assertUpdateStatus(t, actions[3], testClusterServiceClass) + assertRemovedFromBrokerCatalogFalse(t, class) + assertCreate(t, actions[4], testClusterServicePlan) + assertCreate(t, actions[5], testClusterServicePlanNonbindable) + updatedClusterServiceBroker := assertUpdateStatus(t, actions[6], getTestClusterServiceBroker()) + assertClusterServiceBrokerReadyTrue(t, updatedClusterServiceBroker) + + // verify no kube resources created + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) +} + func TestReconcileClusterServiceBrokerRemovedClusterServicePlan(t *testing.T) { fakeKubeClient, fakeCatalogClient, fakeClusterServiceBrokerClient, testController, sharedInformers := newTestController(t, getTestCatalogConfig()) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index c8442538e59e..206a59369e90 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -1818,6 +1818,25 @@ func testActionFor(t *testing.T, name string, f failfFunc, action clientgotestin return fakeRtObject, true } +func assertRemovedFromBrokerCatalogFalse(t *testing.T, obj runtime.Object) { + assertRemovedFromBrokerCatalog(t, obj, false) +} + +func assertRemovedFromBrokerCatalogTrue(t *testing.T, obj runtime.Object) { + assertRemovedFromBrokerCatalog(t, obj, true) +} + +func assertRemovedFromBrokerCatalog(t *testing.T, obj runtime.Object, condition bool) { + clusterServiceClass, ok := obj.(*v1beta1.ClusterServiceClass) + if !ok { + fatalf(t, "Couldn't convert object %+v into a *v1beta1.ClusterServiceClass", obj) + } + + if clusterServiceClass.Status.RemovedFromBrokerCatalog != condition { + fatalf(t, "ClusterServiceClass.RemovedFromBrokerCatalog!=%v", condition) + } +} + func assertClusterServiceBrokerReadyTrue(t *testing.T, obj runtime.Object) { assertClusterServiceBrokerCondition(t, obj, v1beta1.ServiceBrokerConditionReady, v1beta1.ConditionTrue) }