From 336a983f4b35b70936624b8b7d8caa5013ef9e6c Mon Sep 17 00:00:00 2001 From: Aaron Schlesinger Date: Thu, 8 Jun 2017 12:13:17 -0700 Subject: [PATCH] Making the controller handle unexpected provision results Fixes https://github.com/kubernetes-incubator/service-catalog/issues/914 --- pkg/controller/controller_instance.go | 1 - pkg/controller/controller_instance_test.go | 68 ++++++++++++++++++++++ 2 files changed, 68 insertions(+), 1 deletion(-) diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index 8fdc4d43ecb..2fbeb52ffbb 100644 --- a/pkg/controller/controller_instance.go +++ b/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, diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index 1d812375667..48934347819 100644 --- a/pkg/controller/controller_instance_test.go +++ b/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)