diff --git a/pkg/controller/case_test.go b/pkg/controller/case_test.go index 41b5304ebc9..13429ebcb08 100644 --- a/pkg/controller/case_test.go +++ b/pkg/controller/case_test.go @@ -18,6 +18,7 @@ package controller_test import ( "errors" + "encoding/json" "fmt" "testing" "time" @@ -36,6 +37,7 @@ import ( apierrors "k8s.io/apimachinery/pkg/api/errors" "k8s.io/apimachinery/pkg/apis/meta/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/runtime" "k8s.io/apimachinery/pkg/util/wait" k8sinformers "k8s.io/client-go/informers" fakek8s "k8s.io/client-go/kubernetes/fake" @@ -153,21 +155,41 @@ func (ct *controllerTest) TearDown() { close(ct.stopCh) } -// AsyncForInstances configures all fake OSB client instance operations (provision, update and deprovision) +// AsyncForInstanceProvisioning configures all fake OSB client provision // responses with async flag -func (ct *controllerTest) AsyncForInstances() { +func (ct *controllerTest) AsyncForInstanceProvisioning() { ct.fakeOSBClient.ProvisionReaction.(*fakeosb.ProvisionReaction).Response.Async = true +} + +// AsyncForInstanceUpdate configures all fake OSB client update +// responses with async flag +func (ct *controllerTest) AsyncForInstanceUpdate() { ct.fakeOSBClient.UpdateInstanceReaction.(*fakeosb.UpdateInstanceReaction).Response.Async = true - ct.fakeOSBClient.DeprovisionReaction.(*fakeosb.DeprovisionReaction).Response.Async = true } -// AsyncForInstances configures all fake OSB client binding operations (bind and unbind) +// AsyncForInstanceDeprovisioning configures all fake OSB client deprovision // responses with async flag -func (ct *controllerTest) AsyncForBindings() { - ct.fakeOSBClient.BindReaction.(*fakeosb.BindReaction).Response.Async = true +func (ct *controllerTest) AsyncForInstanceDeprovisioning() { + ct.fakeOSBClient.DeprovisionReaction.(*fakeosb.DeprovisionReaction).Response.Async = true +} + +// AsyncForUnbind configures fake OSB client unbind operation responses with async flag +func (ct *controllerTest) AsyncForUnbind() { ct.fakeOSBClient.UnbindReaction.(*fakeosb.UnbindReaction).Response.Async = true } +// AsyncForBind configures fake OSB client bind operation responses with async flag +func (ct *controllerTest) AsyncForBind() { + ct.fakeOSBClient.BindReaction.(*fakeosb.BindReaction).Response.Async = true +} + +// SyncForBindings configures all fake OSB client binding operations (bind and unbind) +// responses with async flag to false +func (ct *controllerTest) SyncForBindings() { + ct.fakeOSBClient.BindReaction.(*fakeosb.BindReaction).Response.Async = false + ct.fakeOSBClient.UnbindReaction.(*fakeosb.UnbindReaction).Response.Async = false +} + // AssertOSBBasicAuth verifies the last call to broker whether the correct basic auth credentials was used func (ct *controllerTest) AssertOSBBasicAuth(t *testing.T, username, password string) { require.NotNil(t, ct.osbClientCfg, "OSB Client was not created, wait for broker is ready") @@ -207,6 +229,8 @@ func (ct *controllerTest) numberOfOSBActionByType(actionType fakeosb.ActionType) // SetFirstOSBPollLastOperationReactionsInProgress makes the broker // responses inProgress in first numberOfInProgressResponses calls func (ct *controllerTest) SetFirstOSBPollLastOperationReactionsInProgress(numberOfInProgressResponses int) { + ct.fakeOSBClient.Lock() + defer ct.fakeOSBClient.Unlock() numberOfPolls := 0 ct.fakeOSBClient.PollLastOperationReaction = fakeosb.DynamicPollLastOperationReaction( func(_ *osb.LastOperationRequest) (*osb.LastOperationResponse, error) { @@ -219,9 +243,48 @@ func (ct *controllerTest) SetFirstOSBPollLastOperationReactionsInProgress(number }) } +// SetOSBPollLastOperationReactionsState makes the broker +// responses with given state +func (ct *controllerTest) SetOSBPollLastOperationReactionsState(state osb.LastOperationState) { + ct.fakeOSBClient.Lock() + defer ct.fakeOSBClient.Unlock() + ct.fakeOSBClient.PollLastOperationReaction = &fakeosb.PollLastOperationReaction{ + Response: &osb.LastOperationResponse{State: state}, + } +} + +// SetOSBPollBindingLastOperationReactionsState makes the broker +// responses with given state +func (ct *controllerTest) SetOSBPollBindingLastOperationReactionsState(state osb.LastOperationState) { + ct.fakeOSBClient.Lock() + defer ct.fakeOSBClient.Unlock() + ct.fakeOSBClient.PollBindingLastOperationReaction = &fakeosb.PollBindingLastOperationReaction{ + Response: &osb.LastOperationResponse{State: state}, + } +} + +// SetFirstOSBPollLastOperationReactionsInProgress makes the broker +// responses inProgress in first numberOfInProgressResponses calls +func (ct *controllerTest) SetFirstOSBPollLastOperationReactionsFailed(numberOfFailedResponses int) { + ct.fakeOSBClient.Lock() + defer ct.fakeOSBClient.Unlock() + numberOfPolls := 0 + ct.fakeOSBClient.PollLastOperationReaction = fakeosb.DynamicPollLastOperationReaction( + func(_ *osb.LastOperationRequest) (*osb.LastOperationResponse, error) { + numberOfPolls++ + state := osb.StateFailed + if numberOfPolls > numberOfFailedResponses { + state = osb.StateSucceeded + } + return &osb.LastOperationResponse{State: state}, nil + }) +} + // SetFirstOSBProvisionReactionsHTTPError makes the broker // responses with error in first numberOfInProgressResponses calls func (ct *controllerTest) SetFirstOSBProvisionReactionsHTTPError(numberOfErrorResponses int, code int) { + ct.fakeOSBClient.Lock() + defer ct.fakeOSBClient.Unlock() numberOfPolls := 0 ct.fakeOSBClient.ProvisionReaction = fakeosb.DynamicProvisionReaction( func(_ *osb.ProvisionRequest) (*osb.ProvisionResponse, error) { @@ -235,8 +298,28 @@ func (ct *controllerTest) SetFirstOSBProvisionReactionsHTTPError(numberOfErrorRe }) } +// SetFirstOSBUnbindReactionsHTTPError makes the broker +// responses with error in first numberOfErrorResponses calls +func (ct *controllerTest) SetFirstOSBUnbindReactionsHTTPError(numberOfErrorResponses int, code int) { + ct.fakeOSBClient.Lock() + defer ct.fakeOSBClient.Unlock() + numberOfPolls := 0 + ct.fakeOSBClient.UnbindReaction = fakeosb.DynamicUnbindReaction( + func(_ *osb.UnbindRequest) (*osb.UnbindResponse, error) { + numberOfPolls++ + if numberOfPolls > numberOfErrorResponses { + return &osb.UnbindResponse{}, nil + } + return nil, osb.HTTPStatusCodeError{ + StatusCode: code, + } + }) +} + // SetOSBBindReactionWithHTTPError configures the broker Bind call response as HTTPStatusCodeError func (ct *controllerTest) SetOSBBindReactionWithHTTPError(code int) { + ct.fakeOSBClient.Lock() + defer ct.fakeOSBClient.Unlock() ct.fakeOSBClient.BindReaction = &fakeosb.BindReaction{ Error: osb.HTTPStatusCodeError{ StatusCode: code, @@ -266,11 +349,13 @@ func (ct *controllerTest) fixClusterServiceBroker() *v1beta1.ClusterServiceBroke } } +// CreateSimpleClusterServiceBroker creates a ClusterServiceBroker used in testing scenarios. func (ct *controllerTest) CreateSimpleClusterServiceBroker() error { _, err := ct.scInterface.ClusterServiceBrokers().Create(ct.fixClusterServiceBroker()) return err } +// CreateClusterServiceBrokerWithBasicAuth creates a ClusterServiceBroker with basic auth. func (ct *controllerTest) CreateClusterServiceBrokerWithBasicAuth() error { csb := ct.fixClusterServiceBroker() csb.Spec.AuthInfo = &v1beta1.ClusterServiceBrokerAuthInfo{ @@ -285,10 +370,33 @@ func (ct *controllerTest) CreateClusterServiceBrokerWithBasicAuth() error { return err } +// AddServiceClassRestrictionsToBroker updates a broker with a restrictions, which must filter out all existing classes. +func (ct *controllerTest) AddServiceClassRestrictionsToBroker() error { + classes, err := ct.scInterface.ClusterServiceClasses().List(metav1.ListOptions{}) + if err != nil { + return err + } + var restrictions []string + for _, cl := range classes.Items { + restrictions = append(restrictions, fmt.Sprintf("name!=%s", cl.Name)) + } + + csb, err := ct.scInterface.ClusterServiceBrokers().Get(testClusterServiceBrokerName, metav1.GetOptions{}) + csb.Spec.CatalogRestrictions = &v1beta1.CatalogRestrictions{ + ServiceClass: restrictions, + } + csb.Generation = csb.Generation + 1 + _, err = ct.scInterface.ClusterServiceBrokers().Update(csb) + return err +} + +// CreateServiceInstance creates a ServiceInstance which is used in testing scenarios. func (ct *controllerTest) CreateServiceInstance() error { _, err := ct.scInterface.ServiceInstances(testNamespace).Create(&v1beta1.ServiceInstance{ ObjectMeta: metav1.ObjectMeta{ Name: testServiceInstanceName, + // added by a Webhook, which is not tested here + Finalizers: []string{v1beta1.FinalizerServiceCatalog}, }, Spec: v1beta1.ServiceInstanceSpec{ PlanReference: v1beta1.PlanReference{ @@ -308,8 +416,34 @@ func (ct *controllerTest) CreateServiceInstance() error { return err } -func (ct *controllerTest) DeleteServiceInstance() error { - return ct.scInterface.ServiceInstances(testNamespace).Delete(testServiceInstanceName, &v1.DeleteOptions{}) +func (ct *controllerTest) UpdateServiceInstanceParameters() error { + si, err := ct.scInterface.ServiceInstances(testNamespace).Get(testServiceInstanceName, metav1.GetOptions{}) + if err != nil { + return nil + } + parameters := map[string]interface{}{ + "param-key": "new-param-value", + } + marshalledParams, err := json.Marshal(parameters) + if err != nil { + return fmt.Errorf("failed to marshal parameters %v : %v", parameters, err) + } + si.Spec.Parameters = &runtime.RawExtension{Raw: marshalledParams} + si.Generation = si.Generation + 1 + + _, err = ct.scInterface.ServiceInstances(testNamespace).Update(si) + return err +} + +// Deprovision sets deletion timestamp which is done by K8s in a cluster while ServiceInstance deletion. +func (ct *controllerTest) Deprovision() error { + si, err := ct.scInterface.ServiceInstances(testNamespace).Get(testServiceInstanceName, v1.GetOptions{}) + if err != nil { + return err + } + si.DeletionTimestamp = ct.v1Now() + _, err = ct.scInterface.ServiceInstances(testNamespace).Update(si) + return err } func (ct *controllerTest) CreateBinding() error { @@ -331,10 +465,24 @@ func (ct *controllerTest) CreateBinding() error { return err } +// Unbind sets deletion timestamp which is done by K8s in a cluster. It triggers unbinding process. +func (ct *controllerTest) Unbind() error { + sb, err := ct.scInterface.ServiceBindings(testNamespace).Get(testBindingName, v1.GetOptions{}) + if err != nil { + return err + } + sb.DeletionTimestamp = ct.v1Now() + _, err = ct.scInterface.ServiceBindings(testNamespace).Update(sb) + return err +} + +// DeleteBinding removes the ServiceBinding resource. func (ct *controllerTest) DeleteBinding() error { return ct.scInterface.ServiceBindings(testNamespace).Delete(testBindingName, &v1.DeleteOptions{}) } +// CreateSecretWithBasicAuth creates a secret with credentials +// referenced by a ClusterServiceBroker created by CreateClusterServiceBrokerWithBasicAuth method. func (ct *controllerTest) CreateSecretWithBasicAuth(username, password string) error { _, err := ct.k8sClient.CoreV1().Secrets(testNamespace).Create(&corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -349,6 +497,7 @@ func (ct *controllerTest) CreateSecretWithBasicAuth(username, password string) e return err } +// UpdateSecretWithBasicAuth updates a secret with basic auth func (ct *controllerTest) UpdateSecretWithBasicAuth(username, password string) error { _, err := ct.k8sClient.CoreV1().Secrets(testNamespace).Update(&corev1.Secret{ ObjectMeta: metav1.ObjectMeta{ @@ -363,6 +512,28 @@ func (ct *controllerTest) UpdateSecretWithBasicAuth(username, password string) e return err } +// MarkClusterServiceClassRemoved marks the cluster service class to be removed (sets the RemovedFromBrokerCatalog flag to true) +func (ct *controllerTest) MarkClusterServiceClassRemoved() error { + csc, err := ct.scInterface.ClusterServiceClasses().Get(testClassExternalID, metav1.GetOptions{}) + if err != nil { + return err + } + csc.Status.RemovedFromBrokerCatalog = true + _, err = ct.scInterface.ClusterServiceClasses().UpdateStatus(csc) + return err +} + +// MarkClusterServicePlanRemoved marks the cluster service plan to be removed (sets the RemovedFromBrokerCatalog flag to true) +func (ct *controllerTest) MarkClusterServicePlanRemoved() error { + csp, err := ct.scInterface.ClusterServicePlans().Get(testPlanExternalID, metav1.GetOptions{}) + if err != nil { + return err + } + csp.Status.RemovedFromBrokerCatalog = true + _, err = ct.scInterface.ClusterServicePlans().UpdateStatus(csp) + return err +} + func (ct *controllerTest) AssertClusterServiceClassAndPlan(t *testing.T) { err := ct.WaitForClusterServiceClass() if err != nil { @@ -395,6 +566,14 @@ func (ct *controllerTest) WaitForNotReadyBinding() error { }) } +func (ct *controllerTest) WaitForBindingInProgress() error { + return ct.waitForBindingStatusCondition(v1beta1.ServiceBindingCondition{ + Type: v1beta1.ServiceBindingConditionReady, + Status: v1beta1.ConditionFalse, + Reason: "Binding", + }) +} + func (ct *controllerTest) WaitForBindingOrphanMitigationSuccessful() error { return ct.waitForBindingStatusCondition(v1beta1.ServiceBindingCondition{ Type: v1beta1.ServiceBindingConditionReady, @@ -410,6 +589,49 @@ func (ct *controllerTest) WaitForBindingFailed() error { }) } +func (ct *controllerTest) WaitForUnbindStatus(status v1beta1.ServiceBindingUnbindStatus) error { + var lastBinding *v1beta1.ServiceBinding + err := wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) { + binding, err := ct.scInterface.ServiceBindings(testNamespace).Get(testBindingName, v1.GetOptions{}) + if err != nil { + return false, fmt.Errorf("error getting Binding: %v", err) + } + + if binding.Status.UnbindStatus == status { + return true, nil + } + + lastBinding = binding + return false, nil + }) + if err == wait.ErrWaitTimeout { + return fmt.Errorf("binding with proper unbinding status not found, the existing binding status: %+v", lastBinding.Status) + } + return err +} + +func (ct *controllerTest) WaitForDeprovisionStatus(status v1beta1.ServiceInstanceDeprovisionStatus) error { + var lastInstance *v1beta1.ServiceInstance + err := wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) { + si, err := ct.scInterface.ServiceInstances(testNamespace).Get(testServiceInstanceName, v1.GetOptions{}) + if err != nil { + return false, fmt.Errorf("error getting Binding: %v", err) + } + + if si.Status.DeprovisionStatus == status { + return true, nil + } + + lastInstance = si + return false, nil + }) + if err == wait.ErrWaitTimeout { + return fmt.Errorf("service instance with proper deprovision status not found, "+ + "the existing service instance status: %+v", lastInstance.Status) + } + return err +} + func (ct *controllerTest) waitForBindingStatusCondition(condition v1beta1.ServiceBindingCondition) error { var lastBinding *v1beta1.ServiceBinding err := wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) { @@ -451,16 +673,26 @@ func (ct *controllerTest) WaitForServiceInstanceRemoved() error { } func (ct *controllerTest) WaitForReadyInstance() error { - condition := v1beta1.ServiceInstanceCondition{ + return ct.waitForInstanceCondition(v1beta1.ServiceInstanceCondition{ Type: v1beta1.ServiceInstanceConditionReady, Status: v1beta1.ConditionTrue, - } + }) +} + +func (ct *controllerTest) WaitForInstanceUpdating() error { + return ct.waitForInstanceCondition(v1beta1.ServiceInstanceCondition{ + Type: v1beta1.ServiceInstanceConditionReady, + Status: v1beta1.ConditionFalse, + Reason: "UpdatingInstance", + }) +} +func (ct *controllerTest) waitForInstanceCondition(condition v1beta1.ServiceInstanceCondition) error { var lastInstance *v1beta1.ServiceInstance err := wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) { instance, err := ct.scInterface.ServiceInstances(testNamespace).Get(testServiceInstanceName, v1.GetOptions{}) if err != nil { - return false, fmt.Errorf("error getting Broker: %v", err) + return false, fmt.Errorf("error getting Instance: %v", err) } lastInstance = instance @@ -474,7 +706,28 @@ func (ct *controllerTest) WaitForReadyInstance() error { return false, nil }) if err == wait.ErrWaitTimeout { - return fmt.Errorf("the instance is not ready, current status: %+v", lastInstance.Status) + return fmt.Errorf("the instance is in expected state (expected condition %+v), current status: %+v", condition, lastInstance.Status) + } + return err +} + +func (ct *controllerTest) WaitForAsyncProvisioningInProgress() error { + var lastInstance *v1beta1.ServiceInstance + err := wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) { + instance, err := ct.scInterface.ServiceInstances(testNamespace).Get(testServiceInstanceName, v1.GetOptions{}) + if err != nil { + return false, fmt.Errorf("error getting ServiceInstance: %v", err) + } + lastInstance = instance + + if instance.Status.AsyncOpInProgress { + return true, nil + } + + return false, nil + }) + if err == wait.ErrWaitTimeout { + return fmt.Errorf("the instance is not in progress, current status: %+v", lastInstance.Status) } return err } @@ -520,6 +773,28 @@ func (ct *controllerTest) WaitForClusterServiceClass() error { }) } +func (ct *controllerTest) WaitForClusterServiceClassToNotExists() error { + return wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) { + _, err := ct.scInterface.ClusterServiceClasses().Get(testClassExternalID, v1.GetOptions{}) + if err != nil && apierrors.IsNotFound(err) { + return true, nil + } + + return false, err + }) +} + +func (ct *controllerTest) WaitForClusterServicePlanToNotExists() error { + return wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) { + _, err := ct.scInterface.ClusterServicePlans().Get(testPlanExternalID, v1.GetOptions{}) + if err != nil && apierrors.IsNotFound(err) { + return true, nil + } + + return false, err + }) +} + func (ct *controllerTest) WaitForClusterServicePlan() error { err := wait.PollImmediate(pollingInterval, pollingTimeout, func() (bool, error) { _, err := ct.scInterface.ClusterServicePlans().Get(testPlanExternalID, v1.GetOptions{}) @@ -538,6 +813,10 @@ func (ct *controllerTest) WaitForClusterServicePlan() error { } return err } +func (ct *controllerTest) v1Now() *metav1.Time { + n := v1.NewTime(time.Now()) + return &n +} func fixtureHappyPathBrokerClientConfig() fakeosb.FakeClientConfiguration { return fakeosb.FakeClientConfiguration{ diff --git a/pkg/controller/controller_flow_binding_test.go b/pkg/controller/controller_flow_binding_test.go new file mode 100644 index 00000000000..338fe9006a1 --- /dev/null +++ b/pkg/controller/controller_flow_binding_test.go @@ -0,0 +1,195 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller_test + +import ( + "fmt" + "net/http" + "testing" + + "github.com/kubernetes-sigs/service-catalog/pkg/apis/servicecatalog/v1beta1" + scfeatures "github.com/kubernetes-sigs/service-catalog/pkg/features" + "github.com/pmorie/go-open-service-broker-client/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + utilfeature "k8s.io/apiserver/pkg/util/feature" +) + +// TestServiceBindingOrphanMitigation tests whether a binding has a proper status (OrphanMitigationSuccessful) after +// a bind request returns a status code that should trigger orphan mitigation. +func TestServiceBindingOrphanMitigation(t *testing.T) { + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + // configure broker to respond with HTTP 500 for bind operation + ct.SetOSBBindReactionWithHTTPError(http.StatusInternalServerError) + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + require.NoError(t, ct.CreateServiceInstance()) + require.NoError(t, ct.WaitForReadyInstance()) + + // WHEN + assert.NoError(t, ct.CreateBinding()) + + // THEN + assert.NoError(t, ct.WaitForBindingOrphanMitigationSuccessful()) +} + +// TestServiceBindingFailure tests that a binding gets a failure condition when the +// broker returns a failure response for a bind operation. +func TestServiceBindingFailure(t *testing.T) { + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + // configure broker to respond with HTTP 409 for bind operation + ct.SetOSBBindReactionWithHTTPError(http.StatusConflict) + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) + require.NoError(t, ct.CreateServiceInstance()) + require.NoError(t, ct.WaitForReadyInstance()) + + // WHEN + assert.NoError(t, ct.CreateBinding()) + + // THEN + assert.NoError(t, ct.WaitForBindingFailed()) +} + +// TestServiceBindingRetryForNonExistingInstance try to bind to invalid service instance names. +// After the instance is created - the binding should became ready. +func TestServiceBindingRetryForNonExistingInstance(t *testing.T) { + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) + + // WHEN + // create a binding for non existing instance + assert.NoError(t, ct.CreateBinding()) + assert.NoError(t, ct.WaitForNotReadyBinding()) + // create an instance referenced by the binding + assert.NoError(t, ct.CreateServiceInstance()) + assert.NoError(t, ct.WaitForReadyInstance()) + + // THEN + assert.NoError(t, ct.WaitForReadyBinding()) +} + +// TestServiceBindingDeleteWithAsyncBindInProgress tests that you can delete a +// binding during an async bind operation. Verify the binding is deleted when +// the bind operation completes regardless of success or failure. +func TestServiceBindingDeleteWithAsyncBindInProgress(t *testing.T) { + + for tn, state := range map[string]v2.LastOperationState{ + "binding succeeds": v2.StateSucceeded, + "binding fails": v2.StateFailed, + } { + t.Run(tn, func(t *testing.T) { + // Enable the AsyncBindingOperations feature + utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.AsyncBindingOperations)) + defer utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.AsyncBindingOperations)) + + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + ct.AsyncForBind() + ct.SetOSBPollBindingLastOperationReactionsState(v2.StateInProgress) + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) + assert.NoError(t, ct.CreateServiceInstance()) + assert.NoError(t, ct.WaitForReadyInstance()) + assert.NoError(t, ct.CreateBinding()) + assert.NoError(t, ct.WaitForBindingInProgress()) + + // WHEN + assert.NoError(t, ct.Unbind()) + // let's finish binding with a given state + ct.SetOSBPollBindingLastOperationReactionsState(state) + + // THEN + assert.NoError(t, ct.WaitForUnbindStatus(v1beta1.ServiceBindingUnbindStatusSucceeded)) + // at least one unbind call + assert.NotZero(t, ct.NumberOfOSBUnbindingCalls()) + }) + } +} + +// TestDeleteServiceBindingRetry tests whether deletion of a service binding +// retries after failing. +func TestDeleteServiceBindingFailureRetry(t *testing.T) { + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + ct.SetFirstOSBUnbindReactionsHTTPError(1, http.StatusInternalServerError) + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) + assert.NoError(t, ct.CreateServiceInstance()) + assert.NoError(t, ct.WaitForReadyInstance()) + assert.NoError(t, ct.CreateBinding()) + assert.NoError(t, ct.WaitForReadyBinding()) + + // WHEN + assert.NoError(t, ct.Unbind()) + + // THEN + assert.NoError(t, ct.WaitForUnbindStatus(v1beta1.ServiceBindingUnbindStatusSucceeded)) + + // THEN + assert.NoError(t, ct.WaitForUnbindStatus(v1beta1.ServiceBindingUnbindStatusSucceeded)) + // at least two unbind calls + assert.True(t, ct.NumberOfOSBUnbindingCalls() > 1) +} + +// TestDeleteServiceBindingRetry tests whether deletion of a service binding +// retries after failing an asynchronous unbind. +func TestDeleteServiceBindingFailureRetryAsync(t *testing.T) { + // GIVEN + utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.AsyncBindingOperations)) + defer utilfeature.DefaultMutableFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.AsyncBindingOperations)) + + ct := newControllerTest(t) + defer ct.TearDown() + //async bind/unbind + ct.AsyncForBind() + ct.AsyncForUnbind() + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) + assert.NoError(t, ct.CreateServiceInstance()) + assert.NoError(t, ct.WaitForReadyInstance()) + assert.NoError(t, ct.CreateBinding()) + assert.NoError(t, ct.WaitForReadyBinding()) + + // WHEN + // configure the broker last unbind operation to fail + ct.SetOSBPollBindingLastOperationReactionsState(v2.StateFailed) + assert.NoError(t, ct.Unbind()) + + // THEN + assert.NoError(t, ct.WaitForUnbindStatus(v1beta1.ServiceBindingUnbindStatusRequired)) + // sync unbind call returns OK + ct.SyncForBindings() + + // THEN + // we expect to retry unbind with a success + assert.NoError(t, ct.WaitForUnbindStatus(v1beta1.ServiceBindingUnbindStatusSucceeded)) +} diff --git a/pkg/controller/controller_flow_instance_test.go b/pkg/controller/controller_flow_instance_test.go new file mode 100644 index 00000000000..062ec3ef9ce --- /dev/null +++ b/pkg/controller/controller_flow_instance_test.go @@ -0,0 +1,142 @@ +/* +Copyright 2018 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package controller_test + +import ( + "net/http" + "testing" + + "github.com/kubernetes-sigs/service-catalog/pkg/apis/servicecatalog/v1beta1" + "github.com/pmorie/go-open-service-broker-client/v2" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestProvisionInstanceWithRetries tests creating a ServiceInstance +// with retry after temporary error without orphan mitigation. +func TestProvisionInstanceWithRetries(t *testing.T) { + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + // configure first provision response with HTTP error + ct.SetFirstOSBProvisionReactionsHTTPError(1, http.StatusConflict) + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + + // WHEN + assert.NoError(t, ct.CreateServiceInstance()) + + // THEN + assert.NoError(t, ct.WaitForReadyInstance()) +} + +// TestRetryAsyncDeprovision tests whether asynchronous deprovisioning retries +// by attempting a new deprovision after failing. +func TestRetryAsyncDeprovision(t *testing.T) { + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + ct.AsyncForInstanceDeprovisioning() + ct.SetFirstOSBPollLastOperationReactionsFailed(1) + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) + + assert.NoError(t, ct.CreateServiceInstance()) + assert.NoError(t, ct.WaitForReadyInstance()) + + // WHEN + assert.NoError(t, ct.Deprovision()) + + // THEN + assert.NoError(t, ct.WaitForDeprovisionStatus(v1beta1.ServiceInstanceDeprovisionStatusSucceeded)) + // first deprovisioning fails, expected second one + assert.True(t, ct.NumberOfOSBDeprovisionCalls() > 1) +} + +// TestServiceInstanceDeleteWithAsyncProvisionInProgress tests that you can +// delete an instance during an async provision. Verify the instance is deleted +// when the provisioning completes regardless of success or failure. +func TestServiceInstanceDeleteWithAsyncProvisionInProgress(t *testing.T) { + for tn, state := range map[string]v2.LastOperationState{ + "provision succeeds": v2.StateSucceeded, + "provision fails": v2.StateFailed, + } { + t.Run(tn, func(t *testing.T) { + t.Parallel() + + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + ct.AsyncForInstanceProvisioning() + ct.SetOSBPollLastOperationReactionsState(v2.StateInProgress) + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) + assert.NoError(t, ct.CreateServiceInstance()) + assert.NoError(t, ct.WaitForAsyncProvisioningInProgress()) + + // WHEN + assert.NoError(t, ct.Deprovision()) + // let's finish provisioning with a given state + ct.SetOSBPollLastOperationReactionsState(state) + + // THEN + assert.NoError(t, ct.WaitForDeprovisionStatus(v1beta1.ServiceInstanceDeprovisionStatusSucceeded)) + // at least one deprovisioning call + assert.NotZero(t, ct.NumberOfOSBDeprovisionCalls()) + }) + } +} + +// TestServiceInstanceDeleteWithAsyncUpdateInProgress tests that you can delete +// an instance during an async update. That is, if you request a delete during +// an instance update, the instance will be deleted when the update completes +// regardless of success or failure. +func TestServiceInstanceDeleteWithAsyncUpdateInProgress(t *testing.T) { + for tn, state := range map[string]v2.LastOperationState{ + "update succeeds": v2.StateSucceeded, + "update fails": v2.StateFailed, + } { + t.Run(tn, func(t *testing.T) { + t.Parallel() + + // GIVEN + ct := newControllerTest(t) + defer ct.TearDown() + ct.AsyncForInstanceUpdate() + ct.SetOSBPollLastOperationReactionsState(v2.StateInProgress) + require.NoError(t, ct.CreateSimpleClusterServiceBroker()) + require.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) + assert.NoError(t, ct.CreateServiceInstance()) + assert.NoError(t, ct.WaitForReadyInstance()) + assert.NoError(t, ct.UpdateServiceInstanceParameters()) + assert.NoError(t, ct.WaitForInstanceUpdating()) + + // WHEN + assert.NoError(t, ct.Deprovision()) + // let's finish updating with a given state + ct.SetOSBPollLastOperationReactionsState(state) + + // THEN + assert.NoError(t, ct.WaitForDeprovisionStatus(v1beta1.ServiceInstanceDeprovisionStatusSucceeded)) + // at least one deprovisioning call + assert.NotZero(t, ct.NumberOfOSBDeprovisionCalls()) + }) + } +} diff --git a/pkg/controller/controller_flow_test.go b/pkg/controller/controller_flow_test.go index edd4813fd16..4e41c304ec2 100644 --- a/pkg/controller/controller_flow_test.go +++ b/pkg/controller/controller_flow_test.go @@ -18,13 +18,14 @@ package controller_test import ( "fmt" - "net/http" "testing" scfeatures "github.com/kubernetes-sigs/service-catalog/pkg/features" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" utilfeature "k8s.io/apiserver/pkg/util/feature" + + "github.com/kubernetes-sigs/service-catalog/pkg/apis/servicecatalog/v1beta1" ) // TestBasicFlowWithBasicAuth tests whether the controller uses correct credentials when the secret changes @@ -63,20 +64,26 @@ func TestBasicFlowWithBasicAuth(t *testing.T) { // - provision Instance // - update Instance // - make Binding +// - unbind +// - deprovision func TestBasicFlow(t *testing.T) { for tn, setupFunc := range map[string]func(ts *controllerTest){ "sync": func(ts *controllerTest) { }, "async instances with multiple polls": func(ct *controllerTest) { - ct.AsyncForInstances() + ct.AsyncForInstanceProvisioning() + ct.AsyncForInstanceDeprovisioning() ct.SetFirstOSBPollLastOperationReactionsInProgress(2) }, "async bindings": func(ct *controllerTest) { - ct.AsyncForBindings() + ct.AsyncForBind() + ct.AsyncForUnbind() }, "async instances and bindings": func(ct *controllerTest) { - ct.AsyncForInstances() - ct.AsyncForBindings() + ct.AsyncForInstanceProvisioning() + ct.AsyncForInstanceDeprovisioning() + ct.AsyncForBind() + ct.AsyncForUnbind() }, } { t.Run(tn, func(t *testing.T) { @@ -102,6 +109,8 @@ func TestBasicFlow(t *testing.T) { // expected at least one provision call assert.NotZero(t, ct.NumberOfOSBProvisionCalls()) + // Binding + // WHEN assert.NoError(t, ct.CreateBinding()) @@ -109,87 +118,81 @@ func TestBasicFlow(t *testing.T) { assert.NoError(t, ct.WaitForReadyBinding()) // expected at least one binding call assert.NotZero(t, ct.NumberOfOSBBindingCalls()) - }) - } -} -// TestServiceBindingOrphanMitigation tests whether a binding has a proper status (OrphanMitigationSuccessful) after -// a bind request returns a status code that should trigger orphan mitigation. -func TestServiceBindingOrphanMitigation(t *testing.T) { - // GIVEN - ct := newControllerTest(t) - defer ct.TearDown() - // configure broker to respond with HTTP 500 for bind operation - ct.SetOSBBindReactionWithHTTPError(http.StatusInternalServerError) - require.NoError(t, ct.CreateSimpleClusterServiceBroker()) - require.NoError(t, ct.WaitForReadyBroker()) - require.NoError(t, ct.CreateServiceInstance()) - require.NoError(t, ct.WaitForReadyInstance()) + // Unbinding - // WHEN - ct.CreateBinding() + // WHEN + require.NoError(t, ct.Unbind()) - // THEN - assert.NoError(t, ct.WaitForBindingOrphanMitigationSuccessful()) + // THEN + assert.NoError(t, ct.WaitForUnbindStatus(v1beta1.ServiceBindingUnbindStatusSucceeded)) + assert.NotZero(t, ct.NumberOfOSBUnbindingCalls()) + + // Deprovisioning + + // GIVEN + // simulate k8s which removes the binding + assert.NoError(t, ct.DeleteBinding()) + + // WHEN + assert.NoError(t, ct.Deprovision()) + + // THEN + assert.NoError(t, ct.WaitForDeprovisionStatus(v1beta1.ServiceInstanceDeprovisionStatusSucceeded)) + assert.NotZero(t, ct.NumberOfOSBDeprovisionCalls()) + }) + } } -// TestServiceBindingFailure tests that a binding gets a failure condition when the -// broker returns a failure response for a bind operation. -func TestServiceBindingFailure(t *testing.T) { +// TestClusterServiceClassRemovedFromCatalogAfterFiltering tests whether catalog restrictions filters service classes +func TestClusterServiceClassRemovedFromCatalogAfterFiltering(t *testing.T) { + t.Parallel() // GIVEN ct := newControllerTest(t) defer ct.TearDown() - // configure broker to respond with HTTP 409 for bind operation - ct.SetOSBBindReactionWithHTTPError(http.StatusConflict) - require.NoError(t, ct.CreateSimpleClusterServiceBroker()) - require.NoError(t, ct.WaitForReadyBroker()) + assert.NoError(t, ct.CreateSimpleClusterServiceBroker()) + assert.NoError(t, ct.WaitForReadyBroker()) ct.AssertClusterServiceClassAndPlan(t) - require.NoError(t, ct.CreateServiceInstance()) - require.NoError(t, ct.WaitForReadyInstance()) // WHEN - assert.NoError(t, ct.CreateBinding()) + assert.NoError(t, ct.AddServiceClassRestrictionsToBroker()) // THEN - assert.NoError(t, ct.WaitForBindingFailed()) + assert.NoError(t, ct.WaitForClusterServiceClassToNotExists()) } -// TestServiceBindingRetryForNonExistingInstance try to bind to invalid service instance names. -// After the instance is created - the binding shoul became ready. -func TestServiceBindingRetryForNonExistingInstance(t *testing.T) { +// TestClusterServiceClassRemovedFromCatalogWithoutInstances tests whether a class marked as removed +// is removed by the controller. +func TestClusterServiceClassRemovedFromCatalogWithoutInstances(t *testing.T) { + t.Parallel() // GIVEN ct := newControllerTest(t) defer ct.TearDown() - require.NoError(t, ct.CreateSimpleClusterServiceBroker()) - require.NoError(t, ct.WaitForReadyBroker()) + assert.NoError(t, ct.CreateSimpleClusterServiceBroker()) + assert.NoError(t, ct.WaitForReadyBroker()) ct.AssertClusterServiceClassAndPlan(t) // WHEN - // create a binding for non existing instance - assert.NoError(t, ct.CreateBinding()) - assert.NoError(t, ct.WaitForNotReadyBinding()) - // create an instance referenced by the binding - assert.NoError(t, ct.CreateServiceInstance()) - assert.NoError(t, ct.WaitForReadyInstance()) + ct.MarkClusterServiceClassRemoved() // THEN - assert.NoError(t, ct.WaitForReadyBinding()) + assert.NoError(t, ct.WaitForClusterServiceClassToNotExists()) } -// TestProvisionInstanceWithRetries tests creating a ServiceInstance -// with retry after temporary error without orphan mitigation. -func TestProvisionInstanceWithRetries(t *testing.T) { +// TestClusterServiceClassRemovedFromCatalogWithoutInstances tests whether a plan marked as removed +// is removed by the controller. +func TestClusterServicePlanRemovedFromCatalogWithoutInstances(t *testing.T) { + t.Parallel() // GIVEN ct := newControllerTest(t) defer ct.TearDown() - // configure first provision response with HTTP error - ct.SetFirstOSBProvisionReactionsHTTPError(1, http.StatusConflict) - require.NoError(t, ct.CreateSimpleClusterServiceBroker()) - require.NoError(t, ct.WaitForReadyBroker()) + assert.NoError(t, ct.CreateSimpleClusterServiceBroker()) + assert.NoError(t, ct.WaitForReadyBroker()) + ct.AssertClusterServiceClassAndPlan(t) // WHEN - assert.NoError(t, ct.CreateServiceInstance()) + ct.MarkClusterServicePlanRemoved() // THEN - assert.NoError(t, ct.WaitForReadyInstance()) + assert.NoError(t, ct.WaitForClusterServicePlanToNotExists()) }