Making the controller handle unexpected provision results

Fixes kubernetes-retired#914

pkg/controller/controller_instance.go

@@ -268,7 +268,6 @@ func (c *controller) reconcileInstance(instance *v1alpha1.Instance) error {
 		c.recorder.Event(instance, api.EventTypeWarning, errorFindingNamespaceInstanceReason, s)
 		return err
 	}
-
 	request := &brokerapi.CreateServiceInstanceRequest{
 		ServiceID:         serviceClass.ExternalID,
 		PlanID:            servicePlan.ExternalID,

pkg/controller/controller_instance_test.go

@@ -558,6 +558,74 @@ func TestReconcileInstanceAsynchronousNoOperation(t *testing.T) {
 	assertInstanceLastOperation(t, updatedInstance, "")
 }
 
+// TestReconcileInstanceAsynchronousUnsupportedBrokerError tests to ensure that, on an asynchronous
+// provision, an Instance's conditions get set with a Broker failure that is not OSB API spec
+// compliant
+func TestReconcileInstanceAsynchronousUnsupportedBrokerError(t *testing.T) {
+	fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t)
+
+	fakeBrokerClient.CatalogClient.RetCatalog = getTestCatalog()
+	fakeBrokerClient.InstanceClient.DashboardURL = testDashboardURL
+
+	fakeKubeClient.AddReactor("get", "namespaces", func(action clientgotesting.Action) (bool, runtime.Object, error) {
+		return true, &v1.Namespace{
+			ObjectMeta: metav1.ObjectMeta{
+				UID: types.UID("test_uid_foo"),
+			},
+		}, nil
+	})
+
+	sharedInformers.Brokers().Informer().GetStore().Add(getTestBroker())
+	sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass())
+
+	// Specify an error from the instance client
+	fakeBrokerClient.InstanceClient.ResponseCode = http.StatusInternalServerError
+	// And specify that we want broker to return an operation
+	fakeBrokerClient.InstanceClient.Operation = testOperation
+	instance := getTestInstance()
+
+	if testController.pollingQueue.Len() != 0 {
+		t.Fatalf("Expected the polling queue to be empty")
+	}
+
+	testController.reconcileInstance(instance)
+
+	actions := fakeCatalogClient.Actions()
+	assertNumberOfActions(t, actions, 1)
+
+	// verify no kube resources created.
+	// One single action comes from getting namespace uid
+	kubeActions := fakeKubeClient.Actions()
+	if e, a := 1, len(kubeActions); e != a {
+		t.Fatalf("Unexpected number of actions: expected %v, got %v", e, a)
+	}
+
+	updatedInstance := assertUpdateStatus(t, actions[0], instance)
+	assertInstanceReadyFalse(t, updatedInstance)
+
+	if si, ok := fakeBrokerClient.InstanceClient.Instances[instanceGUID]; !ok {
+		t.Fatalf("Did not find the created Instance in fakeInstanceClient after creation")
+	} else {
+		if len(si.Parameters) > 0 {
+			t.Fatalf("Unexpected parameters, expected none, got %+v", si.Parameters)
+		}
+
+		ns, _ := fakeKubeClient.Core().Namespaces().Get(instance.Namespace, metav1.GetOptions{})
+		if string(ns.UID) != si.OrganizationGUID {
+			t.Fatalf("Unexpected OrganizationGUID: expected %q, got %q", string(ns.UID), si.OrganizationGUID)
+		}
+		if string(ns.UID) != si.SpaceGUID {
+			t.Fatalf("Unexpected SpaceGUID: expected %q, got %q", string(ns.UID), si.SpaceGUID)
+		}
+	}
+
+	// The item should not have been added to the polling queue for later processing
+	if testController.pollingQueue.Len() != 0 {
+		t.Fatalf("Expected the asynchronous instance to end up in the polling queue")
+	}
+	assertAsyncOpInProgressFalse(t, updatedInstance)
+}
+
 func TestReconcileInstanceNamespaceError(t *testing.T) {
 	fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t)