Reset RemovedFromBrokerCatalog when broker re-adds a removed service …

…class (#1770)

* handle broker re-adding a previously removed service class

* fix imports

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
 }
 

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())
 

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)
 }