diff --git a/pkg/controller/controller_broker.go b/pkg/controller/controller_broker.go index eb45dcf3412..94cc1dfdf1d 100644 --- a/pkg/controller/controller_broker.go +++ b/pkg/controller/controller_broker.go @@ -108,7 +108,7 @@ const ( successUnboundReason string = "UnboundSuccessfully" asyncProvisioningReason string = "Provisioning" asyncProvisioningMessage string = "The instance is being provisioned asynchronously" - asyncDeprovisioningReason string = "Derovisioning" + asyncDeprovisioningReason string = "Deprovisioning" asyncDeprovisioningMessage string = "The instance is being deprovisioned asynchronously" ) diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index bd548accbae..e7e3941e13e 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -254,10 +254,14 @@ func (c *controller) reconcileInstanceDelete(instance *v1alpha1.Instance) error if err != nil { return err } - } else { - glog.V(5).Infof("Deprovision call to broker succeeded for Instance %v/%v, finalizing", instance.Namespace, instance.Name) + + c.recorder.Eventf(instance, api.EventTypeNormal, asyncDeprovisioningReason, asyncDeprovisioningMessage) + + return nil } + glog.V(5).Infof("Deprovision call to broker succeeded for Instance %v/%v, finalizing", instance.Namespace, instance.Name) + setInstanceCondition( toUpdate, v1alpha1.InstanceConditionReady, @@ -318,6 +322,10 @@ func (c *controller) reconcileInstance(instance *v1alpha1.Instance) error { return nil } + if instance.Status.AsyncOpInProgress { + return c.pollInstanceInternal(instance) + } + // If there's no async op in progress, determine whether the checksum // has been invalidated by a change to the object. If the instance's // checksum matches the calculated checksum, there is no work to do. @@ -334,38 +342,31 @@ func (c *controller) reconcileInstance(instance *v1alpha1.Instance) error { // communicating with the broker. In the future the same logic will // result in an instance that requires update being processed by the // controller. - if !instance.Status.AsyncOpInProgress { - if instance.Status.Checksum != nil && instance.DeletionTimestamp == nil { - instanceChecksum := checksum.InstanceSpecChecksum(instance.Spec) - if instanceChecksum == *instance.Status.Checksum { - glog.V(4).Infof( - "Not processing event for Instance %v/%v because checksum showed there is no work to do", - instance.Namespace, - instance.Name, - ) - return nil - } + if instance.Status.Checksum != nil && instance.DeletionTimestamp == nil { + instanceChecksum := checksum.InstanceSpecChecksum(instance.Spec) + if instanceChecksum == *instance.Status.Checksum { + glog.V(4).Infof( + "Not processing event for Instance %v/%v because checksum showed there is no work to do", + instance.Namespace, + instance.Name, + ) + return nil } } glog.V(4).Infof("Processing Instance %v/%v", instance.Namespace, instance.Name) - // if the instance is marked for deletion, handle that first. if instance.ObjectMeta.DeletionTimestamp != nil { return c.reconcileInstanceDelete(instance) } + glog.V(4).Infof("Adding/Updating Instance %v/%v", instance.Namespace, instance.Name) + serviceClass, servicePlan, brokerName, brokerClient, err := c.getServiceClassPlanAndBroker(instance) if err != nil { return err } - if instance.Status.AsyncOpInProgress { - return c.pollInstance(serviceClass, servicePlan, brokerName, brokerClient, instance) - } - - glog.V(4).Infof("Adding/Updating Instance %v/%v", instance.Namespace, instance.Name) - // we will definitely update the instance's status - make a deep copy now // for use later in this method. clone, err := api.Scheme.DeepCopy(instance) diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index edc771a059c..ccdfc44968a 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -779,6 +779,79 @@ func TestReconcileInstanceDelete(t *testing.T) { } } +func TestReconcileInstanceDeleteAsynchronous(t *testing.T) { + key := osb.OperationKey(testOperation) + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + DeprovisionReaction: &fakeosb.DeprovisionReaction{ + Response: &osb.DeprovisionResponse{ + Async: true, + OperationKey: &key, + }, + }, + }) + + sharedInformers.Brokers().Informer().GetStore().Add(getTestBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + + instance := getTestInstance() + instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} + instance.ObjectMeta.Finalizers = []string{v1alpha1.FinalizerServiceCatalog} + // we only invoke the broker client to deprovision if we have a checksum set + // as that implies a previous success. + checksum := checksum.InstanceSpecChecksum(instance.Spec) + instance.Status.Checksum = &checksum + + fakeCatalogClient.AddReactor("get", "instances", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, instance, nil + }) + + if testController.pollingQueue.Len() != 0 { + t.Fatalf("Expected the polling queue to be empty") + } + + err := testController.reconcileInstance(instance) + if err != nil { + t.Fatalf("This should not fail") + } + + // The item should've been added to the pollingQueue for later processing + + // TODO: add a way to peek into rate-limited adds that are still pending, + // then uncomment. + // if testController.pollingQueue.Len() != 1 { + // t.Fatalf("Expected the asynchronous instance to end up in the polling queue") + // } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ + AcceptsIncomplete: true, + InstanceID: instanceGUID, + ServiceID: serviceClassGUID, + PlanID: planGUID, + }) + + // Verify no core kube actions occurred + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + + actions := fakeCatalogClient.Actions() + // The action should be: + // 0. Updating the ready condition + assertNumberOfActions(t, actions, 1) + + updatedInstance := assertUpdateStatus(t, actions[0], instance) + assertInstanceReadyFalse(t, updatedInstance) + + events := getRecordedEvents(testController) + assertNumEvents(t, events, 1) + + expectedEvent := api.EventTypeNormal + " " + asyncDeprovisioningReason + " " + "The instance is being deprovisioned asynchronously" + if e, a := expectedEvent, events[0]; e != a { + t.Fatalf("Received unexpected event: %v", a) + } +} + // TestReconcileInstanceDeleteFailedInstance tests that a failed instance will // be finalized, but no deprovision request will be sent to the broker. func TestReconcileInstanceDeleteFailedInstance(t *testing.T) {