From 291ebedb93a7c3ef11048f79ca9b895bb8cd2695 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Tue, 8 May 2018 16:09:25 -0400 Subject: [PATCH 01/24] Initial ns instance loop updates --- pkg/apis/servicecatalog/plan_reference.go | 40 +- .../servicecatalog/v1beta1/plan_reference.go | 30 +- pkg/controller/controller.go | 74 ++- pkg/controller/controller_instance.go | 486 ++++++++++++++++-- pkg/controller/controller_instance_test.go | 2 +- 5 files changed, 555 insertions(+), 77 deletions(-) diff --git a/pkg/apis/servicecatalog/plan_reference.go b/pkg/apis/servicecatalog/plan_reference.go index b0c37bc5518..4dcee076de1 100644 --- a/pkg/apis/servicecatalog/plan_reference.go +++ b/pkg/apis/servicecatalog/plan_reference.go @@ -90,26 +90,6 @@ func (pr PlanReference) GetSpecifiedServiceClass() string { return "" } -// GetSpecifiedServicePlan returns the user-specified class value from either: -// * ServicePlanExternalName -// * ServicePlanExternalID -// * ServicePlanName -func (pr PlanReference) GetSpecifiedServicePlan() string { - if pr.ServicePlanExternalName != "" { - return pr.ServicePlanExternalName - } - - if pr.ServicePlanExternalID != "" { - return pr.ServicePlanExternalID - } - - if pr.ServicePlanName != "" { - return pr.ServicePlanName - } - - return "" -} - // GetSpecifiedClusterServicePlan returns the user-specified plan value from one of: // * ClusterServicePlanExternalName // * ClusterServicePlanExternalID @@ -131,6 +111,26 @@ func (pr PlanReference) GetSpecifiedClusterServicePlan() string { return "" } +// GetSpecifiedServicePlan returns the user-specified plan value from either: +// * ServicePlanExternalName +// * ServicePlanExternalID +// * ServicePlanName +func (pr PlanReference) GetSpecifiedServicePlan() string { + if pr.ServicePlanExternalName != "" { + return pr.ServicePlanExternalName + } + + if pr.ServicePlanExternalID != "" { + return pr.ServicePlanExternalID + } + + if pr.ServicePlanName != "" { + return pr.ServicePlanName + } + + return "" +} + // GetClusterServiceClassFilterFieldName returns the appropriate field name for filtering // a list of service catalog classes by the PlanReference. func (pr PlanReference) GetClusterServiceClassFilterFieldName() string { diff --git a/pkg/apis/servicecatalog/v1beta1/plan_reference.go b/pkg/apis/servicecatalog/v1beta1/plan_reference.go index 45f2812b415..3180cf95649 100644 --- a/pkg/apis/servicecatalog/v1beta1/plan_reference.go +++ b/pkg/apis/servicecatalog/v1beta1/plan_reference.go @@ -111,7 +111,7 @@ func (pr PlanReference) GetSpecifiedClusterServicePlan() string { return "" } -// GetSpecifiedServicePlan returns the user-specified class value from either: +// GetSpecifiedServicePlan returns the user-specified plan value from either: // * ServicePlanExternalName // * ServicePlanExternalID // * ServicePlanName @@ -159,6 +159,34 @@ func (pr PlanReference) GetClusterServicePlanFilterFieldName() string { return "" } +// GetServiceClassFilterFieldName returns the appropriate field name for filtering +// a list of service catalog classes by the PlanReference. +func (pr PlanReference) GetServiceClassFilterFieldName() string { + if pr.ServiceClassExternalName != "" { + return "spec.externalName" + } + + if pr.ServiceClassExternalID != "" { + return "spec.externalID" + } + + return "" +} + +// GetServicePlanFilterFieldName returns the appropriate field name for filtering +// a list of service catalog plans by the PlanReference. +func (pr PlanReference) GetServicePlanFilterFieldName() string { + if pr.ServicePlanExternalName != "" { + return "spec.externalName" + } + + if pr.ServicePlanExternalID != "" { + return "spec.externalID" + } + + return "" +} + // String representation of a PlanReference // Example: class_name/plan_name, class_id/plan_id func (pr PlanReference) String() string { diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 9912fd2698a..a1e8f56731a 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -415,6 +415,29 @@ func (c *controller) getClusterServiceClassPlanAndClusterServiceBroker(instance return serviceClass, servicePlan, brokerName, brokerClient, nil } +func (c *controller) getServiceClassPlanAndServiceBroker(instance *v1beta1.ServiceInstance) (*v1beta1.ServiceClass, *v1beta1.ServicePlan, string, osb.Client, error) { + serviceClass, brokerName, brokerClient, err := c.getServiceClassAndServiceBroker(instance) + if err != nil { + return nil, nil, "", nil, err + } + + var servicePlan *v1beta1.ServicePlan + if instance.Spec.ServicePlanRef != nil { + var err error + servicePlan, err = c.servicePlanLister.ServicePlans(instance.Namespace).Get(instance.Spec.ServicePlanRef.Name) + if nil != err { + return nil, nil, "", nil, &operationError{ + reason: errorNonexistentServicePlanReason, + message: fmt.Sprintf( + "The instance references a non-existent ServicePlan %q - %v", + instance.Spec.ServicePlanRef.Name, instance.Spec.PlanReference, + ), + } + } + } + return serviceClass, servicePlan, brokerName, brokerClient, nil +} + // getClusterServiceClassAndClusterServiceBroker is a sequence of operations that's done in couple of // places so this method fetches the Service Class and creates // a brokerClient to use for that method given an ServiceInstance. @@ -464,6 +487,55 @@ func (c *controller) getClusterServiceClassAndClusterServiceBroker(instance *v1b return serviceClass, broker.Name, brokerClient, nil } +// getServiceClassAndServiceBroker is a sequence of operations that's done in couple of +// places so this method fetches the Service Class and creates +// a brokerClient to use for that method given a ServiceInstance. +func (c *controller) getServiceClassAndServiceBroker(instance *v1beta1.ServiceInstance) (*v1beta1.ServiceClass, string, osb.Client, error) { + pcb := pretty.NewContextBuilder(pretty.ServiceInstance, instance.Namespace, instance.Name, "") + serviceClass, err := c.serviceClassLister.ServiceClasses(instance.Namespace).Get(instance.Spec.ServiceClassRef.Name) + if err != nil { + return nil, "", nil, &operationError{ + reason: errorNonexistentServiceClassReason, + message: fmt.Sprintf( + "The instance references a non-existent ServiceClass (K8S: %q ExternalName: %q)", + instance.Spec.ServiceClassRef.Name, instance.Spec.ServiceClassExternalName, + ), + } + } + + broker, err := c.serviceBrokerLister.ServiceBrokers(instance.Namespace).Get(serviceClass.Spec.ServiceBrokerName) + if err != nil { + return nil, "", nil, &operationError{ + reason: errorNonexistentServiceBrokerReason, + message: fmt.Sprintf( + "The instance references a non-existent broker %q", + serviceClass.Spec.ServiceBrokerName, + ), + } + + } + + authConfig, err := getAuthCredentialsFromServiceBroker(c.kubeClient, broker) + if err != nil { + return nil, "", nil, &operationError{ + reason: errorAuthCredentialsReason, + message: fmt.Sprintf( + "Error getting broker auth credentials for broker %q: %s", + broker.Name, err, + ), + } + } + + clientConfig := NewClientConfigurationForBroker(broker.ObjectMeta, &broker.Spec.CommonServiceBrokerSpec, authConfig) + glog.V(4).Info(pcb.Messagef("Creating client for ServiceBroker %v, URL: %v", broker.Name, broker.Spec.URL)) + brokerClient, err := c.brokerClientCreateFunc(clientConfig) + if err != nil { + return nil, "", nil, err + } + + return serviceClass, broker.Name, brokerClient, nil +} + // getClusterServiceClassPlanAndClusterServiceBrokerForServiceBinding is a sequence of operations that's // done to validate service plan, service class exist, and handles creating // a brokerclient to use for a given ServiceInstance. @@ -637,8 +709,6 @@ func getAuthCredentialsFromClusterServiceBroker(client kubernetes.Interface, bro // returns an error. If the AuthInfo field is nil, empty values are // returned. func getAuthCredentialsFromServiceBroker(client kubernetes.Interface, broker *v1beta1.ServiceBroker) (*osb.AuthConfig, error) { - // ERIK TODO: This method is mostly error handling boilerplate, is it worth consolidating with common elements? - // Main difference are just using the broker's namespace instead of the same namespace as the broker. if broker.Spec.AuthInfo == nil { return nil, nil } diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index 822fca9c3e9..6a879a2bc4b 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -25,6 +25,8 @@ import ( osb "github.com/pmorie/go-open-service-broker-client/v2" utilfeature "k8s.io/apiserver/pkg/util/feature" + "time" + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" "github.com/kubernetes-incubator/service-catalog/pkg/pretty" @@ -36,7 +38,6 @@ import ( "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/wait" "k8s.io/client-go/tools/cache" - "time" ) const ( @@ -64,14 +65,23 @@ const ( errorNonexistentClusterServiceClassMessage string = "ReferencesNonexistentServiceClass" errorNonexistentClusterServicePlanReason string = "ReferencesNonexistentServicePlan" errorNonexistentClusterServiceBrokerReason string = "ReferencesNonexistentBroker" + errorNonexistentServiceClassReason string = "ReferencesNonexistentServiceClass" + errorNonexistentServiceClassMessage string = "ReferencesNonexistentServiceClass" + errorNonexistentServicePlanReason string = "ReferencesNonexistentServicePlan" + errorNonexistentServiceBrokerReason string = "ReferencesNonexistentBroker" errorDeletedClusterServiceClassReason string = "ReferencesDeletedServiceClass" errorDeletedClusterServiceClassMessage string = "ReferencesDeletedServiceClass" errorDeletedClusterServicePlanReason string = "ReferencesDeletedServicePlan" errorDeletedClusterServicePlanMessage string = "ReferencesDeletedServicePlan" + errorDeletedServiceClassReason string = "ReferencesDeletedServiceClass" + errorDeletedServiceClassMessage string = "ReferencesDeletedServiceClass" + errorDeletedServicePlanReason string = "ReferencesDeletedServicePlan" + errorDeletedServicePlanMessage string = "ReferencesDeletedServicePlan" errorFindingNamespaceServiceInstanceReason string = "ErrorFindingNamespaceForInstance" errorOrphanMitigationFailedReason string = "OrphanMitigationFailed" errorInvalidDeprovisionStatusReason string = "InvalidDeprovisionStatus" errorInvalidDeprovisionStatusMessage string = "The deprovision status is invalid" + errorAmbiguousPlanReferenceScope string = "Couldn't determine if the instance refers to a Cluster or Namespaced ServiceClass/Plan" asyncProvisioningReason string = "Provisioning" asyncProvisioningMessage string = "The instance is being provisioned asynchronously" @@ -265,6 +275,8 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance) } reconciliationAction := getReconciliationActionForServiceInstance(instance) switch reconciliationAction { + + // ERIK CP case reconcileAdd: return c.reconcileServiceInstanceAdd(instance) case reconcileUpdate: @@ -345,7 +357,7 @@ func (c *controller) reconcileServiceInstanceAdd(instance *v1beta1.ServiceInstan c.prepareObservedGeneration(instance) } - // Update references to ClusterServicePlan / ClusterServiceClass if necessary. + // Update references to Plan/Class if necessary. modified, err := c.resolveReferences(instance) if err != nil { return err @@ -357,18 +369,7 @@ func (c *controller) reconcileServiceInstanceAdd(instance *v1beta1.ServiceInstan glog.V(4).Info(pcb.Message("Processing adding event")) - serviceClass, servicePlan, brokerName, brokerClient, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) - if err != nil { - return c.handleServiceInstanceReconciliationError(instance, err) - } - - // Check if the ServiceClass or ServicePlan has been deleted and do not allow - // creation of new ServiceInstances. - if err := c.checkForRemovedClassAndPlan(instance, serviceClass, servicePlan); err != nil { - return c.handleServiceInstanceReconciliationError(instance, err) - } - - request, inProgressProperties, err := c.prepareProvisionRequest(instance, serviceClass, servicePlan) + request, inProgressProperties, err := c.prepareProvisionRequest(instance) if err != nil { return c.handleServiceInstanceReconciliationError(instance, err) } @@ -384,9 +385,22 @@ func (c *controller) reconcileServiceInstanceAdd(instance *v1beta1.ServiceInstan return nil } + var prettyClass string + var brokerName string + var brokerClient osb.Client + if instance.Spec.ClusterServiceClassSpecified() { + var serviceClass *v1beta1.ClusterServiceClass + serviceClass, _, brokerName, brokerClient, _ = c.getClusterServiceClassPlanAndClusterServiceBroker(instance) + prettyClass = pretty.ClusterServiceClassName(serviceClass) + } else { + var serviceClass *v1beta1.ServiceClass + serviceClass, _, brokerName, brokerClient, _ = c.getServiceClassPlanAndServiceBroker(instance) + prettyClass = pretty.ServiceClassName(serviceClass) + } + glog.V(4).Info(pcb.Messagef( - "Provisioning a new ServiceInstance of %s at ClusterServiceBroker %q", - pretty.ClusterServiceClassName(serviceClass), brokerName, + "Provisioning a new ServiceInstance of %s at Broker %q", + prettyClass, brokerName, )) response, err := brokerClient.ProvisionInstance(request) @@ -394,7 +408,7 @@ func (c *controller) reconcileServiceInstanceAdd(instance *v1beta1.ServiceInstan if httpErr, ok := osb.IsHTTPError(err); ok { msg := fmt.Sprintf( "Error provisioning ServiceInstance of %s at ClusterServiceBroker %q: %s", - pretty.ClusterServiceClassName(serviceClass), brokerName, httpErr, + prettyClass, brokerName, httpErr, ) readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, errorProvisionCallFailedReason, msg) // Depending on the specific response, we may need to initiate orphan mitigation. @@ -474,7 +488,7 @@ func (c *controller) reconcileServiceInstanceUpdate(instance *v1beta1.ServiceIns // Check if the ServiceClass or ServicePlan has been deleted. If so, do // not allow plan upgrades, but do allow parameter changes. - if err := c.checkForRemovedClassAndPlan(instance, serviceClass, servicePlan); err != nil { + if err := c.checkForRemovedClusterClassAndPlan(instance, serviceClass, servicePlan); err != nil { return c.handleServiceInstanceReconciliationError(instance, err) } @@ -604,12 +618,32 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns return c.handleServiceInstanceReconciliationError(instance, err) } - serviceClass, brokerName, brokerClient, err := c.getClusterServiceClassAndClusterServiceBroker(instance) - if err != nil { - return c.handleServiceInstanceReconciliationError(instance, err) + var prettyName string + var brokerName string + var brokerClient osb.Client + if instance.Spec.ClusterServiceClassSpecified() { + serviceClass, name, bClient, err := c.getClusterServiceClassAndClusterServiceBroker(instance) + if err != nil { + return c.handleServiceInstanceReconciliationError(instance, err) + } + + brokerName = name + brokerClient = bClient + // we need the serviceClass SOLELY to get a value for a msg string >:( + prettyName = pretty.ClusterServiceClassName(serviceClass) + } else if instance.Spec.ServiceClassSpecified() { + serviceClass, name, bClient, err := c.getServiceClassAndServiceBroker(instance) + if err != nil { + return c.handleServiceInstanceReconciliationError(instance, err) + } + + brokerName = name + brokerClient = bClient + // we need the serviceClass SOLELY to get a value for a msg string >:( + prettyName = pretty.ServiceClassName(serviceClass) } - request, inProgressProperties, err := c.prepareDeprovisionRequest(instance, serviceClass) + request, inProgressProperties, err := c.prepareDeprovisionRequest(instance) if err != nil { return c.handleServiceInstanceReconciliationError(instance, err) } @@ -645,7 +679,7 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns if err != nil { msg := fmt.Sprintf( `Error deprovisioning, %s at ClusterServiceBroker %q: %v`, - pretty.ClusterServiceClassName(serviceClass), brokerName, err, + prettyName, brokerName, err, ) if httpErr, ok := osb.IsHTTPError(err); ok { msg = fmt.Sprintf("Deprovision call failed; received error response from broker: %v", httpErr) @@ -675,7 +709,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro instance = instance.DeepCopy() - serviceClass, servicePlan, _, brokerClient, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) + _, _, _, brokerClient, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) if err != nil { return c.handleServiceInstanceReconciliationError(instance, err) } @@ -687,7 +721,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro provisioning := instance.Status.CurrentOperation == v1beta1.ServiceInstanceOperationProvision && !mitigatingOrphan deleting := instance.Status.CurrentOperation == v1beta1.ServiceInstanceOperationDeprovision || mitigatingOrphan - request, err := c.prepareServiceInstanceLastOperationRequest(instance, serviceClass, servicePlan) + request, err := c.prepareServiceInstanceLastOperationRequest(instance) if err != nil { return c.handleServiceInstanceReconciliationError(instance, err) } @@ -869,12 +903,22 @@ func (c *controller) processServiceInstancePollingFailureRetryTimeout(instance * return c.finishPollingServiceInstance(instance) } -// resolveReferences checks to see if ClusterServiceClassRef and/or ClusterServicePlanRef are +// resolveReferences checks to see if (Cluster)ServiceClassRef and/or (Cluster)ServicePlanRef are // nil and if so, will resolve the references and update the instance. // If references needed to be resolved, and the instance status was successfully updated, the method returns true // If either can not be resolved, returns an error and sets the InstanceCondition // with the appropriate error message. func (c *controller) resolveReferences(instance *v1beta1.ServiceInstance) (bool, error) { + if instance.Spec.ClusterServiceClassSpecified() { + return c.resolveClusterReferences(instance) + } else if instance.Spec.ServiceClassSpecified() { + return c.resolveNamespacedReferences(instance) + } + + return false, stderrors.New(errorAmbiguousPlanReferenceScope) +} + +func (c *controller) resolveClusterReferences(instance *v1beta1.ServiceInstance) (bool, error) { if instance.Spec.ClusterServiceClassRef != nil && instance.Spec.ClusterServicePlanRef != nil { return false, nil } @@ -905,6 +949,37 @@ func (c *controller) resolveReferences(instance *v1beta1.ServiceInstance) (bool, return err == nil, err } +func (c *controller) resolveNamespacedReferences(instance *v1beta1.ServiceInstance) (bool, error) { + if instance.Spec.ServiceClassRef != nil && instance.Spec.ServicePlanRef != nil { + return false, nil + } + + var sc *v1beta1.ServiceClass + var err error + if instance.Spec.ServiceClassRef == nil { + instance, sc, err = c.resolveServiceClassRef(instance) + if err != nil { + return false, err + } + } + + if instance.Spec.ServicePlanRef == nil { + if sc == nil { + sc, err = c.serviceClassLister.ServiceClasses(instance.Namespace).Get(instance.Spec.ServiceClassRef.Name) + if err != nil { + return false, fmt.Errorf(`Couldn't find ServiceClass (K8S: %s)": %v`, instance.Spec.ServiceClassRef.Name, err.Error()) + } + } + + instance, err = c.resolveServicePlanRef(instance, sc.Spec.ServiceBrokerName) + if err != nil { + return false, err + } + } + _, err = c.updateServiceInstanceReferences(instance) + return err == nil, err +} + // resolveClusterServiceClassRef resolves a reference to a ClusterServiceClass // and updates the instance. // If ClusterServiceClass can not be resolved, returns an error, records an @@ -986,6 +1061,87 @@ func (c *controller) resolveClusterServiceClassRef(instance *v1beta1.ServiceInst return instance, sc, nil } +// resolveServiceClassRef resolves a reference to a ServiceClass +// and updates the instance. +// If ServiceClass can not be resolved, returns an error, records an +// Event, and sets the InstanceCondition with the appropriate error message. +func (c *controller) resolveServiceClassRef(instance *v1beta1.ServiceInstance) (*v1beta1.ServiceInstance, *v1beta1.ServiceClass, error) { + if !instance.Spec.ServiceClassSpecified() { + // ServiceInstance is in invalid state, should not ever happen. check + return nil, nil, fmt.Errorf("ServiceInstance %s/%s is in invalid state, neither ServiceClassExternalName, ServiceClassExternalID, nor ServiceClassName is set", instance.Namespace, instance.Name) + } + + pcb := pretty.NewContextBuilder(pretty.ServiceInstance, instance.Namespace, instance.Name, "") + var sc *v1beta1.ServiceClass + + if instance.Spec.ServiceClassName != "" { + glog.V(4).Info(pcb.Messagef("looking up a ServiceClass from K8S Name: %q", instance.Spec.ServiceClassName)) + + var err error + sc, err = c.serviceClassLister.ServiceClasses(instance.Namespace).Get(instance.Spec.ServiceClassName) + if err == nil { + instance.Spec.ServiceClassRef = &v1beta1.LocalObjectReference{ + Name: sc.Name, + } + glog.V(4).Info(pcb.Messagef( + "resolved ServiceClass %c to ServiceClass with external Name %q", + instance.Spec.PlanReference, sc.Spec.ExternalName, + )) + } else { + s := fmt.Sprintf( + "References a non-existent ServiceClass %c", + instance.Spec.PlanReference, + ) + glog.Warning(pcb.Message(s)) + c.updateServiceInstanceCondition( + instance, + v1beta1.ServiceInstanceConditionReady, + v1beta1.ConditionFalse, + errorNonexistentServiceClassReason, + "The instance references a ServiceClass that does not exist. "+s, + ) + c.recorder.Event(instance, corev1.EventTypeWarning, errorNonexistentServiceClassReason, s) + return nil, nil, fmt.Errorf(s) + } + } else { + filterField := instance.Spec.GetServiceClassFilterFieldName() + filterValue := instance.Spec.GetSpecifiedServiceClass() + + glog.V(4).Info(pcb.Messagef("looking up a ServiceClass from %s: %q", filterField, filterValue)) + listOpts := metav1.ListOptions{ + FieldSelector: fields.OneTermEqualSelector(filterField, filterValue).String(), + } + serviceClasses, err := c.serviceCatalogClient.ServiceClasses(instance.Namespace).List(listOpts) + if err == nil && len(serviceClasses.Items) == 1 { + sc = &serviceClasses.Items[0] + instance.Spec.ServiceClassRef = &v1beta1.LocalObjectReference{ + Name: sc.Name, + } + glog.V(4).Info(pcb.Messagef( + "resolved %c to K8S ServiceClass %q", + instance.Spec.PlanReference, sc.Name, + )) + } else { + s := fmt.Sprintf( + "References a non-existent ServiceClass %c or there is more than one (found: %d)", + instance.Spec.PlanReference, len(serviceClasses.Items), + ) + glog.Warning(pcb.Message(s)) + c.updateServiceInstanceCondition( + instance, + v1beta1.ServiceInstanceConditionReady, + v1beta1.ConditionFalse, + errorNonexistentServiceClassReason, + "The instance references a ServiceClass that does not exist. "+s, + ) + c.recorder.Event(instance, corev1.EventTypeWarning, errorNonexistentServiceClassReason, s) + return nil, nil, fmt.Errorf(s) + } + } + + return instance, sc, nil +} + // resolveClusterServicePlanRef resolves a reference to a ClusterServicePlan // and updates the instance. // If ClusterServicePlan can not be resolved, returns an error, records an @@ -1062,6 +1218,120 @@ func (c *controller) resolveClusterServicePlanRef(instance *v1beta1.ServiceInsta return instance, nil } +// resolveServicePlanRef resolves a reference to a ServicePlan +// and updates the instance. +// If ServicePlan can not be resolved, returns an error, records an +// Event, and sets the InstanceCondition with the appropriate error message. +func (c *controller) resolveServicePlanRef(instance *v1beta1.ServiceInstance, brokerName string) (*v1beta1.ServiceInstance, error) { + if !instance.Spec.ServicePlanSpecified() { + // ServiceInstance is in invalid state, should not ever happen. check + return nil, fmt.Errorf("ServiceInstance %s/%s is in invalid state, neither ServicePlanExternalName, ServicePlanExternalID, nor ServicePlanName is set", instance.Namespace, instance.Name) + } + + pcb := pretty.NewContextBuilder(pretty.ServiceInstance, instance.Namespace, instance.Name, "") + + if instance.Spec.ServicePlanName != "" { + sp, err := c.servicePlanLister.ServicePlans(instance.Namespace).Get(instance.Spec.ServicePlanName) + if err == nil { + instance.Spec.ServicePlanRef = &v1beta1.LocalObjectReference{ + Name: sp.Name, + } + glog.V(4).Info(pcb.Messagef( + "resolved ServicePlan with K8S name %q to ServicePlan with external name %q", + instance.Spec.ServicePlanName, sp.Spec.ExternalName, + )) + } else { + s := fmt.Sprintf( + "References a non-existent ServicePlan %v", + instance.Spec.PlanReference, + ) + glog.Warning(pcb.Message(s)) + c.updateServiceInstanceCondition( + instance, + v1beta1.ServiceInstanceConditionReady, + v1beta1.ConditionFalse, + errorNonexistentServicePlanReason, + "The instance references a ServicePlan that does not exist. "+s, + ) + c.recorder.Event(instance, corev1.EventTypeWarning, errorNonexistentServicePlanReason, s) + return nil, fmt.Errorf(s) + } + } else { + fieldSet := fields.Set{ + instance.Spec.GetServicePlanFilterFieldName(): instance.Spec.GetSpecifiedServicePlan(), + "spec.serviceClassRef.name": instance.Spec.ServiceClassRef.Name, + "spec.serviceBrokerName": brokerName, + } + fieldSelector := fields.SelectorFromSet(fieldSet).String() + listOpts := metav1.ListOptions{FieldSelector: fieldSelector} + servicePlans, err := c.serviceCatalogClient.ServicePlans(instance.Namespace).List(listOpts) + if err == nil && len(servicePlans.Items) == 1 { + sp := &servicePlans.Items[0] + instance.Spec.ServicePlanRef = &v1beta1.LocalObjectReference{ + Name: sp.Name, + } + glog.V(4).Info(pcb.Messagef("resolved %v to ServicePlan (K8S: %q)", + instance.Spec.PlanReference, sp.Name, + )) + } else { + s := fmt.Sprintf( + "References a non-existent ServicePlan %b on ServiceClass %s %c or there is more than one (found: %d)", + instance.Spec.PlanReference, instance.Spec.ServiceClassRef.Name, instance.Spec.PlanReference, len(servicePlans.Items), + ) + glog.Warning(pcb.Message(s)) + c.updateServiceInstanceCondition( + instance, + v1beta1.ServiceInstanceConditionReady, + v1beta1.ConditionFalse, + errorNonexistentServicePlanReason, + "The instance references a ServicePlan that does not exist. "+s, + ) + c.recorder.Event(instance, corev1.EventTypeWarning, errorNonexistentServicePlanReason, s) + return nil, fmt.Errorf(s) + } + } + + return instance, nil +} + +func (c *controller) prepareProvisionRequest(instance *v1beta1.ServiceInstance) (*osb.ProvisionRequest, *v1beta1.ServiceInstancePropertiesState, error) { + if instance.Spec.ClusterServiceClassSpecified() { + serviceClass, servicePlan, _, _, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) + if err != nil { + return nil, nil, err + } + // Check if the ClusterServiceClass or ClusterServicePlan has been deleted and do not allow + // creation of new ServiceInstances. + if err = c.checkForRemovedClusterClassAndPlan(instance, serviceClass, servicePlan); err != nil { + return nil, nil, err + } + request, inProgressProperties, err := c.innerPrepareProvisionRequest(instance, serviceClass.Spec.CommonServiceClassSpec, servicePlan.Spec.CommonServicePlanSpec) + if err != nil { + return nil, nil, err + } + return request, inProgressProperties, nil + } else if instance.Spec.ServiceClassSpecified() { + serviceClass, servicePlan, _, _, err := c.getServiceClassPlanAndServiceBroker(instance) + if err != nil { + return nil, nil, err + } + // Check if the ServiceClass or ServicePlan has been deleted and do not allow + // creation of new ServiceInstances. + if err = c.checkForRemovedClassAndPlan(instance, serviceClass, servicePlan); err != nil { + return nil, nil, err + } + request, inProgressProperties, err := c.innerPrepareProvisionRequest(instance, serviceClass.Spec.CommonServiceClassSpec, servicePlan.Spec.CommonServicePlanSpec) + if err != nil { + return nil, nil, err + } + return request, inProgressProperties, nil + } + + // If we're hitting this retun, it means we couldn't tell whether the class + // and plan were cluster or namespace scoped + return nil, nil, stderrors.New(errorAmbiguousPlanReferenceScope) +} + // newServiceInstanceCondition is a helper function that returns a // condition with the given type, status, reason and message, with its transition // time set to now. @@ -1376,10 +1646,10 @@ func (c *controller) recordStartOfServiceInstanceOperation(toUpdate *v1beta1.Ser return c.updateServiceInstanceStatus(toUpdate) } -// checkForRemovedClassAndPlan looks at serviceClass and servicePlan and -// if either has been deleted, will block a new instance creation. If -// -func (c *controller) checkForRemovedClassAndPlan(instance *v1beta1.ServiceInstance, serviceClass *v1beta1.ClusterServiceClass, servicePlan *v1beta1.ClusterServicePlan) error { +// checkForRemovedClusterClassAndPlan looks at clusterServiceClass and +// clusterServicePlan and if either has been deleted, will block a new instance +// creation. +func (c *controller) checkForRemovedClusterClassAndPlan(instance *v1beta1.ServiceInstance, serviceClass *v1beta1.ClusterServiceClass, servicePlan *v1beta1.ClusterServicePlan) error { classDeleted := serviceClass.Status.RemovedFromBrokerCatalog planDeleted := servicePlan.Status.RemovedFromBrokerCatalog @@ -1413,6 +1683,43 @@ func (c *controller) checkForRemovedClassAndPlan(instance *v1beta1.ServiceInstan } } +// checkForRemovedClassAndPlan looks at serviceClass and +// servicePlan and if either has been deleted, will block a new instance +// creation. +func (c *controller) checkForRemovedClassAndPlan(instance *v1beta1.ServiceInstance, serviceClass *v1beta1.ServiceClass, servicePlan *v1beta1.ServicePlan) error { + classDeleted := serviceClass.Status.RemovedFromBrokerCatalog + planDeleted := servicePlan.Status.RemovedFromBrokerCatalog + + if !classDeleted && !planDeleted { + // Neither has been deleted, life's good. + return nil + } + + isProvisioning := instance.Status.ProvisionStatus != v1beta1.ServiceInstanceProvisionStatusProvisioned + + // Regardless of what's been deleted, you can always update + // parameters (ie, not change plans) + if !isProvisioning && instance.Status.ExternalProperties != nil && + servicePlan.Spec.ExternalID == instance.Status.ExternalProperties.ServicePlanExternalID { + // Service Instance has already been provisioned and we're only + // updating parameters, so let it through. + return nil + } + + // At this point we know that plan is being changed + if planDeleted { + return &operationError{ + reason: errorDeletedServicePlanReason, + message: fmt.Sprintf("%s has been deleted; cannot provision.", pretty.ServicePlanName(servicePlan)), + } + } + + return &operationError{ + reason: errorDeletedServiceClassReason, + message: fmt.Sprintf("%s has been deleted; cannot provision.", pretty.ServiceClassName(serviceClass)), + } +} + // clearServiceInstanceCurrentOperation sets the fields of the instance's Status // to indicate that there is no current operation being performed. The Status // is *not* recorded in the registry. @@ -1462,7 +1769,7 @@ type requestHelper struct { // prepareRequestHelper is a helper function that generates a struct with // properties common to multiple request types. -func (c *controller) prepareRequestHelper(instance *v1beta1.ServiceInstance, servicePlan *v1beta1.ClusterServicePlan, setInProgressProperties bool) (*requestHelper, error) { +func (c *controller) prepareRequestHelper(instance *v1beta1.ServiceInstance, planName string, planID string, setInProgressProperties bool) (*requestHelper, error) { rh := &requestHelper{} if utilfeature.DefaultFeatureGate.Enabled(scfeatures.OriginatingIdentity) { @@ -1507,11 +1814,17 @@ func (c *controller) prepareRequestHelper(instance *v1beta1.ServiceInstance, ser rh.parameters = parameters rh.inProgressProperties = &v1beta1.ServiceInstancePropertiesState{ - ClusterServicePlanExternalName: servicePlan.Spec.ExternalName, - ClusterServicePlanExternalID: servicePlan.Spec.ExternalID, - Parameters: rawParametersWithRedaction, - ParametersChecksum: parametersChecksum, - UserInfo: instance.Spec.UserInfo, + Parameters: rawParametersWithRedaction, + ParametersChecksum: parametersChecksum, + UserInfo: instance.Spec.UserInfo, + } + + if instance.Spec.ClusterServiceClassSpecified() { + rh.inProgressProperties.ClusterServicePlanExternalName = planName + rh.inProgressProperties.ClusterServicePlanExternalID = planID + } else { + rh.inProgressProperties.ServicePlanExternalName = planName + rh.inProgressProperties.ServicePlanExternalID = planID } } @@ -1526,10 +1839,11 @@ func (c *controller) prepareRequestHelper(instance *v1beta1.ServiceInstance, ser return rh, nil } -// prepareProvisionRequest creates a provision request object to be passed to -// the broker client to provision the given instance. -func (c *controller) prepareProvisionRequest(instance *v1beta1.ServiceInstance, serviceClass *v1beta1.ClusterServiceClass, servicePlan *v1beta1.ClusterServicePlan) (*osb.ProvisionRequest, *v1beta1.ServiceInstancePropertiesState, error) { - rh, err := c.prepareRequestHelper(instance, servicePlan, true) +// innerPrepareProvisionRequest creates a provision request object to be passed to +// the broker client to provision the given instance, with a cluster scoped +// class and plan +func (c *controller) innerPrepareProvisionRequest(instance *v1beta1.ServiceInstance, classCommon v1beta1.CommonServiceClassSpec, planCommon v1beta1.CommonServicePlanSpec) (*osb.ProvisionRequest, *v1beta1.ServiceInstancePropertiesState, error) { + rh, err := c.prepareRequestHelper(instance, planCommon.ExternalName, planCommon.ExternalID, true) if err != nil { return nil, nil, err } @@ -1537,8 +1851,8 @@ func (c *controller) prepareProvisionRequest(instance *v1beta1.ServiceInstance, request := &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: instance.Spec.ExternalID, - ServiceID: serviceClass.Spec.ExternalID, - PlanID: servicePlan.Spec.ExternalID, + ServiceID: classCommon.ExternalID, + PlanID: planCommon.ExternalID, Parameters: rh.parameters, OrganizationGUID: string(rh.ns.UID), SpaceGUID: string(rh.ns.UID), @@ -1552,7 +1866,7 @@ func (c *controller) prepareProvisionRequest(instance *v1beta1.ServiceInstance, // prepareUpdateInstanceRequest creates an update instance request object to be // passed to the broker client to update the given instance. func (c *controller) prepareUpdateInstanceRequest(instance *v1beta1.ServiceInstance, serviceClass *v1beta1.ClusterServiceClass, servicePlan *v1beta1.ClusterServicePlan) (*osb.UpdateInstanceRequest, *v1beta1.ServiceInstancePropertiesState, error) { - rh, err := c.prepareRequestHelper(instance, servicePlan, true) + rh, err := c.prepareRequestHelper(instance, servicePlan.Spec.ExternalName, servicePlan.Spec.ExternalID, true) if err != nil { return nil, nil, err } @@ -1586,12 +1900,29 @@ func (c *controller) prepareUpdateInstanceRequest(instance *v1beta1.ServiceInsta // prepareDeprovisionRequest creates a deprovision request object to be passed // to the broker client to deprovision the given instance. -func (c *controller) prepareDeprovisionRequest(instance *v1beta1.ServiceInstance, serviceClass *v1beta1.ClusterServiceClass) (*osb.DeprovisionRequest, *v1beta1.ServiceInstancePropertiesState, error) { - rh, err := c.prepareRequestHelper(instance, nil, false) +func (c *controller) prepareDeprovisionRequest(instance *v1beta1.ServiceInstance) (*osb.DeprovisionRequest, *v1beta1.ServiceInstancePropertiesState, error) { + rh, err := c.prepareRequestHelper(instance, "", "", true) if err != nil { return nil, nil, err } + // Get the appropriate external id based for the cluster or namespaced + // service class + var scExternalID string + if instance.Spec.ClusterServiceClassSpecified() { + serviceClass, _, _, err := c.getClusterServiceClassAndClusterServiceBroker(instance) + if err != nil { + return nil, nil, c.handleServiceInstanceReconciliationError(instance, err) + } + scExternalID = serviceClass.Spec.ExternalID + } else if instance.Spec.ServiceClassSpecified() { + serviceClass, _, _, err := c.getServiceClassAndServiceBroker(instance) + if err != nil { + return nil, nil, c.handleServiceInstanceReconciliationError(instance, err) + } + scExternalID = serviceClass.Spec.ExternalID + } + // The plan reference in the spec might be updated since the latest // provisioning/update request, thus we need to take values from the original // provisioning request instead that we previously stored in status @@ -1607,10 +1938,18 @@ func (c *controller) prepareDeprovisionRequest(instance *v1beta1.ServiceInstance rh.inProgressProperties = instance.Status.ExternalProperties } + // Should come from rh.inProgressProperties.(Cluster)ServicePlanExternalID + var planExternalID string + if instance.Spec.ClusterServiceClassSpecified() { + planExternalID = rh.inProgressProperties.ClusterServicePlanExternalID + } else if instance.Spec.ServiceClassSpecified() { + planExternalID = rh.inProgressProperties.ServicePlanExternalID + } + request := &osb.DeprovisionRequest{ InstanceID: instance.Spec.ExternalID, - ServiceID: serviceClass.Spec.ExternalID, - PlanID: rh.inProgressProperties.ClusterServicePlanExternalID, + ServiceID: scExternalID, + PlanID: planExternalID, OriginatingIdentity: rh.originatingIdentity, AcceptsIncomplete: true, } @@ -1620,22 +1959,63 @@ func (c *controller) prepareDeprovisionRequest(instance *v1beta1.ServiceInstance // preparePollServiceInstanceRequest creates a request object to be passed to // the broker client to query the given instance's last operation endpoint. -func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta1.ServiceInstance, serviceClass *v1beta1.ClusterServiceClass, servicePlan *v1beta1.ClusterServicePlan) (*osb.LastOperationRequest, error) { - rh, err := c.prepareRequestHelper(instance, servicePlan, false) - if err != nil { - return nil, err +func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta1.ServiceInstance) (*osb.LastOperationRequest, error) { + + var rh *requestHelper + var scExternalID string + + if instance.Spec.ClusterServiceClassSpecified() { + serviceClass, servicePlan, _, _, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) + if err != nil { + return nil, c.handleServiceInstanceReconciliationError(instance, err) + } + + scExternalID = serviceClass.Spec.ExternalID + + // Sometimes the servicePlan is nil + var spExternalName string + var spExternalID string + if servicePlan != nil { + spExternalName = servicePlan.Spec.ExternalName + spExternalID = servicePlan.Spec.ExternalID + } + + rh, err = c.prepareRequestHelper(instance, spExternalName, spExternalID, false) + if err != nil { + return nil, err + } + } else if instance.Spec.ServiceClassSpecified() { + serviceClass, servicePlan, _, _, err := c.getServiceClassPlanAndServiceBroker(instance) + if err != nil { + return nil, c.handleServiceInstanceReconciliationError(instance, err) + } + + scExternalID = serviceClass.Spec.ExternalID + + // Sometimes the servicePlan is nil + var spExternalName string + var spExternalID string + if servicePlan != nil { + spExternalName = servicePlan.Spec.ExternalName + spExternalID = servicePlan.Spec.ExternalID + } + + rh, err = c.prepareRequestHelper(instance, spExternalName, spExternalID, false) + if err != nil { + return nil, err + } } if instance.Status.InProgressProperties == nil { pcb := pretty.NewInstanceContextBuilder(instance) - err = stderrors.New("Instance.Status.InProgressProperties can not be nil") + err := stderrors.New("Instance.Status.InProgressProperties can not be nil") glog.Errorf(pcb.Message(err.Error())) return nil, err } request := &osb.LastOperationRequest{ InstanceID: instance.Spec.ExternalID, - ServiceID: &serviceClass.Spec.ExternalID, + ServiceID: &scExternalID, PlanID: &instance.Status.InProgressProperties.ClusterServicePlanExternalID, OriginatingIdentity: rh.originatingIdentity, } diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index f528f0db250..ba2b4574544 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -5572,7 +5572,7 @@ func TestCheckClassAndPlanForDeletion(t *testing.T) { for _, tc := range cases { fakeKubeClient, fakeCatalogClient, fakeClusterServiceBrokerClient, testController, _ := newTestController(t, noFakeActions()) - err := testController.checkForRemovedClassAndPlan(tc.instance, tc.class, tc.plan) + err := testController.checkForRemovedClusterClassAndPlan(tc.instance, tc.class, tc.plan) if err != nil { if tc.success { t.Errorf("%q: Unexpected error %v", tc.name, err) From 5458df716b4698e7997b8084ebedd90af19fc1a2 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Thu, 14 Jun 2018 14:47:15 -0400 Subject: [PATCH 02/24] Disambiguate getServiceInstanceWithRefs --- pkg/controller/controller_binding_test.go | 4 +- pkg/controller/controller_instance_test.go | 80 +++++++++++----------- pkg/controller/controller_test.go | 18 ++--- 3 files changed, 51 insertions(+), 51 deletions(-) diff --git a/pkg/controller/controller_binding_test.go b/pkg/controller/controller_binding_test.go index 1843eea2c17..123dd765d4f 100644 --- a/pkg/controller/controller_binding_test.go +++ b/pkg/controller/controller_binding_test.go @@ -909,7 +909,7 @@ func TestReconcileServiceBindingServiceInstanceNotReady(t *testing.T) { sharedInformers.ClusterServiceBrokers().Informer().GetStore().Add(getTestClusterServiceBroker()) sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) - sharedInformers.ServiceInstances().Informer().GetStore().Add(getTestServiceInstanceWithRefs()) + sharedInformers.ServiceInstances().Informer().GetStore().Add(getTestServiceInstanceWithClusterRefs()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) binding := &v1beta1.ServiceBinding{ @@ -967,7 +967,7 @@ func TestReconcileServiceBindingNamespaceError(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() setServiceInstanceCondition(instance, v1beta1.ServiceInstanceConditionReady, v1beta1.ConditionTrue, "", "") sharedInformers.ServiceInstances().Informer().GetStore().Add(instance) diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index ba2b4574544..5aa616367b6 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -151,7 +151,7 @@ func TestReconcileServiceInstanceNonExistentClusterServiceBroker(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err == nil { t.Fatal("The broker referenced by the instance exists when it should not.") @@ -196,7 +196,7 @@ func TestReconcileServiceInstanceWithAuthError(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() fakeKubeClient.AddReactor("get", "secrets", func(action clientgotesting.Action) (bool, runtime.Object, error) { return true, nil, errors.New("no secret defined") @@ -588,7 +588,7 @@ func TestReconcileServiceInstanceWithParameters(t *testing.T) { }) } - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if tc.params != nil { instance.Spec.Parameters = &runtime.RawExtension{Raw: tc.params} @@ -861,7 +861,7 @@ func TestReconcileServiceInstanceWithProvisionCallFailure(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err != nil { t.Fatalf("unexpected error: %v", err) @@ -929,7 +929,7 @@ func TestReconcileServiceInstanceWithTemporaryProvisionFailure(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() ////////////////////////////////////// // Check 1st reconcilliation iteration (prepare/validate request & set status to in progress) @@ -1043,7 +1043,7 @@ func TestReconcileServiceInstanceWithTerminalProvisionFailure(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err != nil { t.Fatalf("unexpected error: %v", err) @@ -1119,7 +1119,7 @@ func TestReconcileServiceInstance(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err != nil { t.Fatalf("unexpected error: %v", err) @@ -1190,7 +1190,7 @@ func TestReconcileServiceInstanceFailsWithDeletedPlan(t *testing.T) { sp.Status.RemovedFromBrokerCatalog = true sharedInformers.ClusterServicePlans().Informer().GetStore().Add(sp) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err == nil { t.Fatalf("This should fail") @@ -1240,7 +1240,7 @@ func TestReconcileServiceInstanceFailsWithDeletedClass(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(sc) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err == nil { t.Fatalf("This should have failed") @@ -1395,7 +1395,7 @@ func TestReconcileServiceInstanceAsynchronous(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -1464,7 +1464,7 @@ func TestReconcileServiceInstanceAsynchronousNoOperation(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err != nil { t.Fatalf("unexpected error: %v", err) @@ -1527,7 +1527,7 @@ func TestReconcileServiceInstanceNamespaceError(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err == nil { t.Fatalf("There should not be a namespace for the ServiceInstance to be created in") @@ -1574,7 +1574,7 @@ func TestReconcileServiceInstanceDelete(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} // we only invoke the broker client to deprovision if we have a reconciled generation set @@ -1649,7 +1649,7 @@ func TestReconcileServiceInstanceDeleteBlockedByCredentials(t *testing.T) { credentials := getTestServiceBinding() sharedInformers.ServiceBindings().Informer().GetStore().Add(credentials) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} // we only invoke the broker client to deprovision if we have a reconciled generation set @@ -1759,7 +1759,7 @@ func TestReconcileServiceInstanceDeleteAsynchronous(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} // we only invoke the broker client to deprovision if we have a reconciled generation set @@ -2056,7 +2056,7 @@ func TestReconcileServiceInstanceDeleteFailedUpdate(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} instance.Status.ExternalProperties = &v1beta1.ServiceInstancePropertiesState{ @@ -2125,7 +2125,7 @@ func TestReconcileServiceInstanceDeleteDoesNotInvokeClusterServiceBroker(t *test sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} instance.Generation = 1 @@ -2171,7 +2171,7 @@ func TestFinalizerClearedWhen409ConflictEncounteredOnStatusUpdate(t *testing.T) sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ResourceVersion = "1" instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} @@ -2951,7 +2951,7 @@ func TestReconcileServiceInstanceSuccessOnFinalRetry(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Status.CurrentOperation = v1beta1.ServiceInstanceOperationProvision instance.Status.InProgressProperties = &v1beta1.ServiceInstancePropertiesState{ ClusterServicePlanExternalName: testClusterServicePlanName, @@ -3011,7 +3011,7 @@ func TestReconcileServiceInstanceUpdateInProgressPropertiesOnRetry(t *testing.T) sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Status.CurrentOperation = v1beta1.ServiceInstanceOperationProvision instance.Status.InProgressProperties = &v1beta1.ServiceInstancePropertiesState{ ClusterServicePlanExternalName: testClusterServicePlanName, @@ -3093,7 +3093,7 @@ func TestReconcileServiceInstanceFailureOnFinalRetry(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Status.CurrentOperation = v1beta1.ServiceInstanceOperationProvision instance.Status.InProgressProperties = &v1beta1.ServiceInstancePropertiesState{ ClusterServicePlanExternalID: testClusterServicePlanGUID, @@ -3281,7 +3281,7 @@ func TestReconcileServiceInstanceWithStatusUpdateError(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() fakeCatalogClient.AddReactor("update", "serviceinstances", func(action clientgotesting.Action) (bool, runtime.Object, error) { return true, nil, errors.New("update error") @@ -3639,7 +3639,7 @@ func TestReconcileInstanceUsingOriginatingIdentity(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if tc.includeUserInfo { instance.Spec.UserInfo = testUserInfo } @@ -3692,7 +3692,7 @@ func TestReconcileInstanceDeleteUsingOriginatingIdentity(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} // we only invoke the broker client to deprovision if we have a @@ -3847,7 +3847,7 @@ func TestReconcileServiceInstanceWithHTTPStatusCodeErrorOrphanMitigation(t *test sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -3905,7 +3905,7 @@ func TestReconcileServiceInstanceTimeoutTriggersOrphanMitigation(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() if err := reconcileServiceInstance(t, testController, instance); err != nil { t.Fatalf("unexpected error: %v", err) } @@ -4125,7 +4125,7 @@ func TestReconcileServiceInstanceOrphanMitigation(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} instance.Status.CurrentOperation = v1beta1.ServiceInstanceOperationProvision instance.Status.OrphanMitigationInProgress = true @@ -4223,7 +4223,7 @@ func TestReconcileServiceInstanceWithSecretParameters(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() parameters := map[string]interface{}{ "a": "1", @@ -4321,7 +4321,7 @@ func TestReconcileServiceInstanceWithSecretParameters(t *testing.T) { // nothing if references have already been set. func TestResolveReferencesReferencesAlreadySet(t *testing.T) { fakeKubeClient, fakeCatalogClient, _, testController, _ := newTestController(t, noFakeActions()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() modified, err := testController.resolveReferences(instance) if err != nil { t.Fatalf("resolveReferences failed unexpectedly: %q", err) @@ -4413,7 +4413,7 @@ func TestReconcileServiceInstanceUpdateParameters(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 instance.Status.ObservedGeneration = 1 @@ -4536,7 +4536,7 @@ func TestReconcileServiceInstanceDeleteParameters(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 instance.Status.ObservedGeneration = 1 @@ -4744,7 +4744,7 @@ func TestReconcileServiceInstanceUpdateDashboardURLResponse(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 instance.Status.ObservedGeneration = 1 @@ -4856,7 +4856,7 @@ func TestReconcileServiceInstanceUpdatePlan(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 instance.Status.ObservedGeneration = 1 @@ -5153,7 +5153,7 @@ func TestResolveReferencesForPlanChange(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() newPlanID := "new-plan-id" newPlanName := "new-plan-name" @@ -5285,7 +5285,7 @@ func TestReconcileServiceInstanceUpdateAsynchronous(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 instance.Status.ObservedGeneration = 1 @@ -5608,7 +5608,7 @@ func TestReconcileServiceInstanceDeleteDuringOngoingOperation(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} instance.Status.CurrentOperation = v1beta1.ServiceInstanceOperationProvision @@ -5688,7 +5688,7 @@ func TestReconcileServiceInstanceDeleteWithOngoingOperation(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} instance.Status.CurrentOperation = v1beta1.ServiceInstanceOperationProvision @@ -5772,7 +5772,7 @@ func TestReconcileServiceInstanceDeleteWithNonExistentPlan(t *testing.T) { sharedInformers.ClusterServiceBrokers().Informer().GetStore().Add(getTestClusterServiceBroker()) sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} // we only invoke the broker client to deprovision if we have a reconciled generation set @@ -5846,7 +5846,7 @@ func TestReconcileServiceInstanceUpdateMissingObservedGeneration(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 // Missing ObservedGeneration and Provisioned after updating Service Catalog @@ -5893,7 +5893,7 @@ func TestReconcileServiceInstanceUpdateMissingOrphanMitigation(t *testing.T) { sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 instance.Status.ObservedGeneration = 1 diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index e6271261ebd..2102edc7771 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -717,7 +717,7 @@ func getTestCatalog() *osb.CatalogResponse { // as ClusterServiceClassRef and ClusterServicePlanRef which means that the // ClusterServiceClass and ClusterServicePlan are fetched using // Service[Class|Plan]Lister.get(spec.Service[Class|Plan]Ref.Name) -func getTestServiceInstanceWithRefs() *v1beta1.ServiceInstance { +func getTestServiceInstanceWithClusterRefs() *v1beta1.ServiceInstance { sc := getTestServiceInstance() sc.Spec.ClusterServiceClassRef = &v1beta1.ClusterObjectReference{Name: testClusterServiceClassGUID} sc.Spec.ClusterServicePlanRef = &v1beta1.ClusterObjectReference{Name: testClusterServicePlanGUID} @@ -725,7 +725,7 @@ func getTestServiceInstanceWithRefs() *v1beta1.ServiceInstance { } func getTestServiceInstanceWithRefsAndExternalProperties() *v1beta1.ServiceInstance { - sc := getTestServiceInstanceWithRefs() + sc := getTestServiceInstanceWithClusterRefs() sc.Status.ExternalProperties = &v1beta1.ServiceInstancePropertiesState{ ClusterServicePlanExternalID: testClusterServicePlanGUID, ClusterServicePlanExternalName: testClusterServicePlanName, @@ -801,7 +801,7 @@ func getTestServiceInstanceNonbindableServiceBindablePlan() *v1beta1.ServiceInst } func getTestServiceInstanceBindableServiceNonbindablePlan() *v1beta1.ServiceInstance { - i := getTestServiceInstanceWithRefs() + i := getTestServiceInstanceWithClusterRefs() i.Spec.ClusterServicePlanExternalName = testNonbindableClusterServicePlanName i.Spec.ClusterServicePlanRef = &v1beta1.ClusterObjectReference{Name: testNonbindableClusterServicePlanGUID} @@ -819,7 +819,7 @@ func getTestServiceInstanceWithStatus(status v1beta1.ConditionStatus) *v1beta1.S } func getTestServiceInstanceWithFailedStatus() *v1beta1.ServiceInstance { - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Status = v1beta1.ServiceInstanceStatus{ Conditions: []v1beta1.ServiceInstanceCondition{{ Type: v1beta1.ServiceInstanceConditionFailed, @@ -831,7 +831,7 @@ func getTestServiceInstanceWithFailedStatus() *v1beta1.ServiceInstance { } func getTestServiceInstanceUpdatingPlan() *v1beta1.ServiceInstance { - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status = v1beta1.ServiceInstanceStatus{ Conditions: []v1beta1.ServiceInstanceCondition{{ @@ -853,7 +853,7 @@ func getTestServiceInstanceUpdatingPlan() *v1beta1.ServiceInstance { } func getTestServiceInstanceUpdatingParametersOfDeletedPlan() *v1beta1.ServiceInstance { - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 instance.Status = v1beta1.ServiceInstanceStatus{ Conditions: []v1beta1.ServiceInstanceCondition{{ @@ -876,7 +876,7 @@ func getTestServiceInstanceUpdatingParametersOfDeletedPlan() *v1beta1.ServiceIns // getTestServiceInstanceAsync returns an instance in async mode func getTestServiceInstanceAsyncProvisioning(operation string) *v1beta1.ServiceInstance { - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() operationStartTime := metav1.NewTime(time.Now().Add(-1 * time.Hour)) instance.Status = v1beta1.ServiceInstanceStatus{ @@ -906,7 +906,7 @@ func getTestServiceInstanceAsyncProvisioning(operation string) *v1beta1.ServiceI // getTestServiceInstanceAsyncUpdating returns an instance for which there is an // in-progress async update func getTestServiceInstanceAsyncUpdating(operation string) *v1beta1.ServiceInstance { - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 operationStartTime := metav1.NewTime(time.Now().Add(-1 * time.Hour)) @@ -941,7 +941,7 @@ func getTestServiceInstanceAsyncUpdating(operation string) *v1beta1.ServiceInsta } func getTestServiceInstanceAsyncDeprovisioning(operation string) *v1beta1.ServiceInstance { - instance := getTestServiceInstanceWithRefs() + instance := getTestServiceInstanceWithClusterRefs() instance.Generation = 2 operationStartTime := metav1.NewTime(time.Now().Add(-1 * time.Hour)) From 8b4ce7d7cabd439f2a0759fc88cbc90a4cea85dc Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 09:56:51 -0400 Subject: [PATCH 03/24] Update .gitignore to ignore debug.test --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 794ce8a7b93..ee8b92f59ee 100644 --- a/.gitignore +++ b/.gitignore @@ -25,3 +25,4 @@ contrib/examples/consumer/Gopkg.lock contrib/examples/consumer contrib/examples/vendor/* integration.test* +debug.test* From 0bc38019c839a3136190c9eeb70ae56af28168cd Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 10:00:21 -0400 Subject: [PATCH 04/24] Disambiguate getTestServiceInstanceWithClusterRefs * Also adds some namespaced helpers --- pkg/controller/controller_test.go | 33 +++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 2102edc7771..587cf645a9e 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -724,6 +724,13 @@ func getTestServiceInstanceWithClusterRefs() *v1beta1.ServiceInstance { return sc } +func getTestServiceInstanceWithNamespacedRefs() *v1beta1.ServiceInstance { + sc := getTestServiceInstanceNamespacedPlanRef() + sc.Spec.ServiceClassRef = &v1beta1.LocalObjectReference{Name: testServiceClassGUID} + sc.Spec.ServicePlanRef = &v1beta1.LocalObjectReference{Name: testServicePlanGUID} + return sc +} + func getTestServiceInstanceWithRefsAndExternalProperties() *v1beta1.ServiceInstance { sc := getTestServiceInstanceWithClusterRefs() sc.Status.ExternalProperties = &v1beta1.ServiceInstancePropertiesState{ @@ -759,6 +766,32 @@ func getTestServiceInstance() *v1beta1.ServiceInstance { } } +// instance referencing the result of getTestServiceClass() +// and getTestServicePlan() +// This version sets: +// ServiceClassExternalName and ServicePlanExternalName, so depending on the +// test, you may need to add reactors that deal with List due to the need +// to resolve Names to IDs for both ServiceClass and ServicePlan +func getTestServiceInstanceNamespacedPlanRef() *v1beta1.ServiceInstance { + return &v1beta1.ServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: testServiceInstanceName, + Namespace: testNamespace, + Generation: 1, + }, + Spec: v1beta1.ServiceInstanceSpec{ + PlanReference: v1beta1.PlanReference{ + ServiceClassExternalName: testServiceClassName, + ServicePlanExternalName: testServicePlanName, + }, + ExternalID: testServiceInstanceGUID, + }, + Status: v1beta1.ServiceInstanceStatus{ + DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, + }, + } +} + // instance referencing the result of getTestClusterServiceClass() // and getTestClusterServicePlan() // This version sets: From ff61185d9de9994ea405da784aca8067656b848a Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 10:01:25 -0400 Subject: [PATCH 05/24] Add namespaces to the test type producers --- pkg/controller/controller_ns_test.go | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/pkg/controller/controller_ns_test.go b/pkg/controller/controller_ns_test.go index f019fac8899..0e0b7ac1883 100644 --- a/pkg/controller/controller_ns_test.go +++ b/pkg/controller/controller_ns_test.go @@ -41,7 +41,10 @@ func getTestCommonServiceBrokerSpec() v1beta1.CommonServiceBrokerSpec { func getTestServiceBroker() *v1beta1.ServiceBroker { return &v1beta1.ServiceBroker{ - ObjectMeta: metav1.ObjectMeta{Name: testServiceBrokerName}, + ObjectMeta: metav1.ObjectMeta{ + Name: testServiceBrokerName, + Namespace: testNamespace, + }, Spec: v1beta1.ServiceBrokerSpec{ CommonServiceBrokerSpec: getTestCommonServiceBrokerSpec(), }, @@ -59,7 +62,10 @@ func getTestCommonServiceClassSpec() v1beta1.CommonServiceClassSpec { func getTestServiceClass() *v1beta1.ServiceClass { return &v1beta1.ServiceClass{ - ObjectMeta: metav1.ObjectMeta{Name: testServiceClassGUID}, + ObjectMeta: metav1.ObjectMeta{ + Name: testServiceClassGUID, + Namespace: testNamespace, + }, Spec: v1beta1.ServiceClassSpec{ ServiceBrokerName: testServiceBrokerName, CommonServiceClassSpec: getTestCommonServiceClassSpec(), @@ -77,7 +83,10 @@ func getTestCommonServicePlanSpec() v1beta1.CommonServicePlanSpec { func getTestServicePlan() *v1beta1.ServicePlan { return &v1beta1.ServicePlan{ - ObjectMeta: metav1.ObjectMeta{Name: testServicePlanGUID}, + ObjectMeta: metav1.ObjectMeta{ + Name: testServicePlanGUID, + Namespace: testNamespace, + }, Spec: v1beta1.ServicePlanSpec{ ServiceBrokerName: testServiceBrokerName, CommonServicePlanSpec: getTestCommonServicePlanSpec(), From faca699f4e74d9c81ea6fcbc3f524b85854a262a Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 10:33:20 -0400 Subject: [PATCH 06/24] Add TestReconcileServiceInstanceWithNamespacedRefs --- pkg/controller/controller_instance_test.go | 74 ++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index 5aa616367b6..7bbda4868b4 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -1177,6 +1177,80 @@ func TestReconcileServiceInstance(t *testing.T) { } } +// TestReconcileServiceInstanceNamespacedRefs tests synchronously provisioning a new service +func TestReconcileServiceInstanceNamespacedRefs(t *testing.T) { + fakeKubeClient, _, _, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + ProvisionReaction: &fakeosb.ProvisionReaction{ + Response: &osb.ProvisionResponse{ + DashboardURL: &testDashboardURL, + }, + }, + }) + + addGetNamespaceReaction(fakeKubeClient) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceWithNamespacedRefs() + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + // instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) + // fakeCatalogClient.ClearActions() + + // assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + // fakeKubeClient.ClearActions() + + // if err := reconcileServiceInstance(t, testController, instance); err != nil { + // t.Fatalf("This should not fail : %v", err) + // } + + // brokerActions := fakeClusterServiceBrokerClient.Actions() + // assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + // assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ + // AcceptsIncomplete: true, + // InstanceID: testServiceInstanceGUID, + // ServiceID: testClusterServiceClassGUID, + // PlanID: testClusterServicePlanGUID, + // OrganizationGUID: testNamespaceGUID, + // SpaceGUID: testNamespaceGUID, + // Context: testContext}) + + // instanceKey := testNamespace + "/" + testServiceInstanceName + + // // Since synchronous operation, must not make it into the polling queue. + // if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + // t.Fatalf("Expected polling queue to not have any record of test 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 err := checkKubeClientActions(kubeActions, []kubeClientAction{ + // {verb: "get", resourceName: "namespaces", checkType: checkGetActionType}, + // }); err != nil { + // t.Fatal(err) + // } + + // updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + // assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testClusterServicePlanName, testClusterServicePlanGUID, instance) + // assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) + + // events := getRecordedEvents(testController) + + // expectedEvent := normalEventBuilder(successProvisionReason).msg(successProvisionMessage) + // if err := checkEvents(events, expectedEvent.stringArr()); err != nil { + // t.Fatal(err) + // } +} + // TestReconcileServiceInstanceFailsWithDeletedPlan tests that a ServiceInstance is not // created if the ServicePlan specified is marked as RemovedFromCatalog. func TestReconcileServiceInstanceFailsWithDeletedPlan(t *testing.T) { From 0bd698b870f14a2a09efa80ea662eee2a0434d59 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 13:05:04 -0400 Subject: [PATCH 07/24] Passing cp --- pkg/controller/controller_instance_test.go | 42 ++++++++++++++++++---- pkg/controller/controller_test.go | 28 ++++++++++----- 2 files changed, 56 insertions(+), 14 deletions(-) diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index 7bbda4868b4..ff6e716b66d 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -1179,7 +1179,13 @@ func TestReconcileServiceInstance(t *testing.T) { // TestReconcileServiceInstanceNamespacedRefs tests synchronously provisioning a new service func TestReconcileServiceInstanceNamespacedRefs(t *testing.T) { - fakeKubeClient, _, _, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + fakeKubeClient, fakeCatalogClient, _, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ ProvisionReaction: &fakeosb.ProvisionReaction{ Response: &osb.ProvisionResponse{ DashboardURL: &testDashboardURL, @@ -1199,8 +1205,8 @@ func TestReconcileServiceInstanceNamespacedRefs(t *testing.T) { t.Fatalf("unexpected error: %v", err) } - // instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) - // fakeCatalogClient.ClearActions() + instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) + fakeCatalogClient.ClearActions() // assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) // fakeKubeClient.ClearActions() @@ -6007,15 +6013,39 @@ func generateChecksumOfParametersOrFail(t *testing.T, params map[string]interfac } func assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t *testing.T, fakeCatalogClient *fake.Clientset, instance *v1beta1.ServiceInstance) *v1beta1.ServiceInstance { - return assertServiceInstanceOperationInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance, v1beta1.ServiceInstanceOperationProvision, testClusterServicePlanName, testClusterServicePlanGUID) + var planName, planGUID string + if instance.Spec.ClusterServiceClassSpecified() { + planName = testClusterServicePlanName + planGUID = testClusterServicePlanGUID + } else { + planName = testServicePlanName + planGUID = testServicePlanGUID + } + return assertServiceInstanceOperationInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance, v1beta1.ServiceInstanceOperationProvision, planName, planGUID) } func assertServiceInstanceUpdateInProgressIsTheOnlyCatalogClientAction(t *testing.T, fakeCatalogClient *fake.Clientset, instance *v1beta1.ServiceInstance) *v1beta1.ServiceInstance { - return assertServiceInstanceOperationInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance, v1beta1.ServiceInstanceOperationUpdate, testClusterServicePlanName, testClusterServicePlanGUID) + var planName, planGUID string + if instance.Spec.ClusterServiceClassSpecified() { + planName = testClusterServicePlanName + planGUID = testClusterServicePlanGUID + } else { + planName = testServicePlanName + planGUID = testServicePlanGUID + } + return assertServiceInstanceOperationInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance, v1beta1.ServiceInstanceOperationUpdate, planName, planGUID) } func assertServiceInstanceDeprovisionInProgressIsTheOnlyCatalogClientAction(t *testing.T, fakeCatalogClient *fake.Clientset, instance *v1beta1.ServiceInstance) *v1beta1.ServiceInstance { - return assertServiceInstanceOperationInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance, v1beta1.ServiceInstanceOperationDeprovision, testClusterServicePlanName, testClusterServicePlanGUID) + var planName, planGUID string + if instance.Spec.ClusterServiceClassSpecified() { + planName = testClusterServicePlanName + planGUID = testClusterServicePlanGUID + } else { + planName = testServicePlanName + planGUID = testServicePlanGUID + } + return assertServiceInstanceOperationInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance, v1beta1.ServiceInstanceOperationDeprovision, planName, planGUID) } func assertServiceInstanceOperationInProgressIsTheOnlyCatalogClientAction(t *testing.T, fakeCatalogClient *fake.Clientset, instance *v1beta1.ServiceInstance, operation v1beta1.ServiceInstanceOperation, planName string, planGUID string) *v1beta1.ServiceInstance { diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 587cf645a9e..8ede2841cb8 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -2844,7 +2844,7 @@ func assertServiceInstanceInProgressPropertiesPlan(t *testing.T, obj runtime.Obj if !ok { fatalf(t, "Couldn't convert object %+v into a *v1beta1.ServiceInstance", obj) } - assertServiceInstancePropertiesStatePlan(t, "in-progress", instance.Status.InProgressProperties, planName, planID) + assertServiceInstancePropertiesStatePlan(t, "in-progress", instance, planName, planID) } func assertServiceInstanceInProgressPropertiesParameters(t *testing.T, obj runtime.Object, params map[string]interface{}, checksum string) { @@ -2883,7 +2883,7 @@ func assertServiceInstanceExternalPropertiesPlan(t *testing.T, obj runtime.Objec if !ok { fatalf(t, "Couldn't convert object %+v into a *v1beta1.ServiceInstance", obj) } - assertServiceInstancePropertiesStatePlan(t, "external", instance.Status.ExternalProperties, planName, planID) + assertServiceInstancePropertiesStatePlan(t, "external", instance, planName, planID) } func assertServiceInstanceExternalPropertiesParameters(t *testing.T, obj runtime.Object, params map[string]interface{}, checksum string) { @@ -2906,16 +2906,28 @@ func assertServiceInstanceExternalPropertiesUnchanged(t *testing.T, obj runtime. } } -func assertServiceInstancePropertiesStatePlan(t *testing.T, propsLabel string, actualProps *v1beta1.ServiceInstancePropertiesState, expectedPlanName string, expectedPlanID string) { +func assertServiceInstancePropertiesStatePlan(t *testing.T, propsLabel string, instance *v1beta1.ServiceInstance, expectedPlanName string, expectedPlanID string) { + actualProps := instance.Status.InProgressProperties if actualProps == nil { fatalf(t, "expected %v properties to not be nil", propsLabel) } - if e, a := expectedPlanName, actualProps.ClusterServicePlanExternalName; e != a { - fatalf(t, "unexpected %v properties external service plan name: expected %v, actual %v", propsLabel, e, a) - } - if e, a := expectedPlanID, actualProps.ClusterServicePlanExternalID; e != a { - fatalf(t, "unexpected %v properties external service plan ID: expected %v, actual %v", propsLabel, e, a) + + if instance.Spec.ClusterServicePlanSpecified() { + if e, a := expectedPlanName, actualProps.ClusterServicePlanExternalName; e != a { + fatalf(t, "unexpected %v properties external service plan name: expected %v, actual %v", propsLabel, e, a) + } + if e, a := expectedPlanID, actualProps.ClusterServicePlanExternalID; e != a { + fatalf(t, "unexpected %v properties external service plan ID: expected %v, actual %v", propsLabel, e, a) + } + } else { + if e, a := expectedPlanName, actualProps.ServicePlanExternalName; e != a { + fatalf(t, "unexpected %v properties external service plan name: expected %v, actual %v", propsLabel, e, a) + } + if e, a := expectedPlanID, actualProps.ServicePlanExternalID; e != a { + fatalf(t, "unexpected %v properties external service plan ID: expected %v, actual %v", propsLabel, e, a) + } } + } func assertServiceInstancePropertiesStateParameters(t *testing.T, propsLabel string, actualProps *v1beta1.ServiceInstancePropertiesState, expectedParams map[string]interface{}, expectedChecksum string) { From 614841ad7be5a05ce0f2f634d5c0b8e72f04e9cb Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 15:35:35 -0400 Subject: [PATCH 08/24] Add sync provision test for namespaced class/plan --- pkg/controller/controller_binding_test.go | 100 ++++---- .../controller_clusterservicebroker_test.go | 30 +-- pkg/controller/controller_instance_test.go | 236 +++++++++--------- pkg/controller/controller_test.go | 15 +- 4 files changed, 190 insertions(+), 191 deletions(-) diff --git a/pkg/controller/controller_binding_test.go b/pkg/controller/controller_binding_test.go index 123dd765d4f..46856ac0a15 100644 --- a/pkg/controller/controller_binding_test.go +++ b/pkg/controller/controller_binding_test.go @@ -70,7 +70,7 @@ func TestReconcileServiceBindingNonExistingServiceInstance(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -136,7 +136,7 @@ func TestReconcileServiceBindingUnresolvedClusterServiceClassReference(t *testin } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -203,7 +203,7 @@ func TestReconcileServiceBindingUnresolvedClusterServicePlanReference(t *testing } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -267,7 +267,7 @@ func TestReconcileServiceBindingNonExistingClusterServiceClass(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() // There is one action to update to failed status because there's @@ -341,7 +341,7 @@ func TestReconcileServiceBindingWithSecretConflict(t *testing.T) { assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err := reconcileServiceBinding(t, testController, binding) if err == nil { @@ -349,7 +349,7 @@ func TestReconcileServiceBindingWithSecretConflict(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -457,7 +457,7 @@ func TestReconcileServiceBindingWithParameters(t *testing.T) { assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err = reconcileServiceBinding(t, testController, binding) if err != nil { @@ -465,7 +465,7 @@ func TestReconcileServiceBindingWithParameters(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -599,7 +599,7 @@ func TestReconcileServiceBindingWithSecretTransform(t *testing.T) { assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err := testController.reconcileServiceBinding(binding) if err != nil { @@ -607,7 +607,7 @@ func TestReconcileServiceBindingWithSecretTransform(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -698,7 +698,7 @@ func TestReconcileServiceBindingNonbindableClusterServiceClass(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -781,7 +781,7 @@ func TestReconcileServiceBindingNonbindableClusterServiceClassBindablePlan(t *te assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err := reconcileServiceBinding(t, testController, binding) if err != nil { @@ -789,7 +789,7 @@ func TestReconcileServiceBindingNonbindableClusterServiceClassBindablePlan(t *te } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -873,7 +873,7 @@ func TestReconcileServiceBindingBindableClusterServiceClassNonbindablePlan(t *te } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -932,7 +932,7 @@ func TestReconcileServiceBindingServiceInstanceNotReady(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -992,7 +992,7 @@ func TestReconcileServiceBindingNamespaceError(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -1113,7 +1113,7 @@ func TestReconcileServiceBindingDelete(t *testing.T) { assertDeleteSecretAction(t, fakeKubeClient.Actions(), binding.Spec.SecretName) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err := reconcileServiceBinding(t, testController, binding) if err != nil { @@ -1121,7 +1121,7 @@ func TestReconcileServiceBindingDelete(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUnbind(t, brokerActions[0], &osb.UnbindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -1201,7 +1201,7 @@ func TestReconcileServiceBindingDeleteUnresolvedClusterServiceClassReference(t * } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() // The actions should be: @@ -1387,7 +1387,7 @@ func TestReconcileServiceBindingDeleteFailedServiceBinding(t *testing.T) { assertDeleteSecretAction(t, fakeKubeClient.Actions(), binding.Spec.SecretName) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err := reconcileServiceBinding(t, testController, binding) if err != nil { @@ -1395,7 +1395,7 @@ func TestReconcileServiceBindingDeleteFailedServiceBinding(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUnbind(t, brokerActions[0], &osb.UnbindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -1590,7 +1590,7 @@ func TestReconcileServiceBindingWithFailureCondition(t *testing.T) { assertNumberOfActions(t, actions, 0) brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) events := getRecordedEvents(testController) assertNumEvents(t, events, 0) @@ -1621,7 +1621,7 @@ func TestReconcileServiceBindingWithServiceBindingCallFailure(t *testing.T) { assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) if err := reconcileServiceBinding(t, testController, binding); err == nil { t.Fatal("ServiceBinding creation should fail") @@ -1643,7 +1643,7 @@ func TestReconcileServiceBindingWithServiceBindingCallFailure(t *testing.T) { assertServiceBindingOrphanMitigationSet(t, updatedServiceBinding, false) brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -1695,7 +1695,7 @@ func TestReconcileServiceBindingWithServiceBindingFailure(t *testing.T) { assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) if err := reconcileServiceBinding(t, testController, binding); err != nil { t.Fatalf("ServiceBinding creation should complete: %v", err) @@ -1717,7 +1717,7 @@ func TestReconcileServiceBindingWithServiceBindingFailure(t *testing.T) { assertServiceBindingOrphanMitigationSet(t, updatedServiceBinding, false) brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -2058,7 +2058,7 @@ func TestReconcileBindingUsingOriginatingIdentity(t *testing.T) { assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeBrokerClient.Actions(), 0) err := reconcileServiceBinding(t, testController, binding) if err != nil { @@ -2066,7 +2066,7 @@ func TestReconcileBindingUsingOriginatingIdentity(t *testing.T) { } brokerActions := fakeBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) actualRequest, ok := brokerActions[0].Request.(*osb.BindRequest) if !ok { t.Errorf("%v: unexpected request type; expected %T, got %T", tc.name, &osb.BindRequest{}, actualRequest) @@ -2122,7 +2122,7 @@ func TestReconcileBindingDeleteUsingOriginatingIdentity(t *testing.T) { assertDeleteSecretAction(t, fakeKubeClient.Actions(), binding.Spec.SecretName) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeBrokerClient.Actions(), 0) err := reconcileServiceBinding(t, testController, binding) if err != nil { @@ -2130,7 +2130,7 @@ func TestReconcileBindingDeleteUsingOriginatingIdentity(t *testing.T) { } brokerActions := fakeBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) actualRequest, ok := brokerActions[0].Request.(*osb.UnbindRequest) if !ok { t.Errorf("%v: unexpected request type; expected %T, got %T", tc.name, &osb.UnbindRequest{}, actualRequest) @@ -2178,7 +2178,7 @@ func TestReconcileBindingSuccessOnFinalRetry(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -2303,7 +2303,7 @@ func TestReconcileBindingWithSecretConflictFailedAfterFinalRetry(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -2372,7 +2372,7 @@ func TestReconcileServiceBindingWithStatusUpdateError(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -2477,7 +2477,7 @@ func TestReconcileServiceBindingWithSecretParameters(t *testing.T) { assertActionEquals(t, kubeActions[1], "get", "secrets") fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err = reconcileServiceBinding(t, testController, binding) if err != nil { @@ -2485,7 +2485,7 @@ func TestReconcileServiceBindingWithSecretParameters(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -2657,14 +2657,14 @@ func TestReconcileBindingWithSetOrphanMitigation(t *testing.T) { assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeServiceBrokerClient.Actions(), 0) if err := reconcileServiceBinding(t, testController, binding); tc.shouldReturnError && err == nil || !tc.shouldReturnError && err != nil { t.Fatalf("expected to return %v from reconciliation attempt, got %v", tc.shouldReturnError, err) } brokerActions := fakeServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -2745,7 +2745,7 @@ func TestReconcileBindingWithOrphanMitigationInProgress(t *testing.T) { assertDeleteSecretAction(t, fakeKubeClient.Actions(), binding.Spec.SecretName) brokerActions := fakeServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUnbind(t, brokerActions[0], &osb.UnbindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -2818,7 +2818,7 @@ func TestReconcileBindingWithOrphanMitigationReconciliationRetryTimeOut(t *testi assertDeleteSecretAction(t, fakeKubeClient.Actions(), binding.Spec.SecretName) brokerActions := fakeServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUnbind(t, brokerActions[0], &osb.UnbindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -2905,7 +2905,7 @@ func TestReconcileServiceBindingDeleteDuringOngoingOperation(t *testing.T) { assertDeleteSecretAction(t, fakeKubeClient.Actions(), binding.Spec.SecretName) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err := reconcileServiceBinding(t, testController, binding) if err != nil { @@ -2913,7 +2913,7 @@ func TestReconcileServiceBindingDeleteDuringOngoingOperation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUnbind(t, brokerActions[0], &osb.UnbindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -3001,7 +3001,7 @@ func TestReconcileServiceBindingDeleteDuringOrphanMitigation(t *testing.T) { assertDeleteSecretAction(t, fakeKubeClient.Actions(), binding.Spec.SecretName) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) err := reconcileServiceBinding(t, testController, binding) if err != nil { @@ -3009,7 +3009,7 @@ func TestReconcileServiceBindingDeleteDuringOrphanMitigation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUnbind(t, brokerActions[0], &osb.UnbindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -3078,7 +3078,7 @@ func TestReconcileServiceBindingAsynchronousBind(t *testing.T) { assertGetNamespaceAction(t, fakeKubeClient.Actions()) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeServiceBrokerClient.Actions(), 0) if err := reconcileServiceBinding(t, testController, binding); err != nil { t.Fatalf("a valid binding should not fail: %v", err) @@ -3090,7 +3090,7 @@ func TestReconcileServiceBindingAsynchronousBind(t *testing.T) { // Broker actions brokerActions := fakeServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertBind(t, brokerActions[0], &osb.BindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -3171,7 +3171,7 @@ func TestReconcileServiceBindingAsynchronousUnbind(t *testing.T) { assertDeleteSecretAction(t, fakeKubeClient.Actions(), binding.Spec.SecretName) fakeKubeClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeServiceBrokerClient.Actions(), 0) if err := reconcileServiceBinding(t, testController, binding); err != nil { t.Fatalf("a valid binding should not fail: %v", err) @@ -3183,7 +3183,7 @@ func TestReconcileServiceBindingAsynchronousUnbind(t *testing.T) { // Broker actions brokerActions := fakeServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUnbind(t, brokerActions[0], &osb.UnbindRequest{ BindingID: testServiceBindingGUID, InstanceID: testServiceInstanceGUID, @@ -3221,7 +3221,7 @@ func TestPollServiceBinding(t *testing.T) { } validatePollBindingLastOperationAction := func(t *testing.T, actions []fakeosb.Action) { - assertNumberOfClusterServiceBrokerActions(t, actions, 1) + assertNumberOfBrokerActions(t, actions, 1) operationKey := osb.OperationKey(testOperation) assertPollBindingLastOperation(t, actions[0], &osb.BindingLastOperationRequest{ @@ -3234,7 +3234,7 @@ func TestPollServiceBinding(t *testing.T) { } validatePollBindingLastOperationAndGetBindingActions := func(t *testing.T, actions []fakeosb.Action) { - assertNumberOfClusterServiceBrokerActions(t, actions, 2) + assertNumberOfBrokerActions(t, actions, 2) operationKey := osb.OperationKey(testOperation) assertPollBindingLastOperation(t, actions[0], &osb.BindingLastOperationRequest{ @@ -3849,7 +3849,7 @@ func TestPollServiceBinding(t *testing.T) { if tc.validateBrokerActionsFunc != nil { tc.validateBrokerActionsFunc(t, brokerActions) } else { - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) } // Kube actions diff --git a/pkg/controller/controller_clusterservicebroker_test.go b/pkg/controller/controller_clusterservicebroker_test.go index 2675db1a309..61d0056283e 100644 --- a/pkg/controller/controller_clusterservicebroker_test.go +++ b/pkg/controller/controller_clusterservicebroker_test.go @@ -246,7 +246,7 @@ func TestReconcileClusterServiceBrokerExistingServiceClassAndServicePlan(t *test } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) listRestrictions := clientgotesting.ListRestrictions{ @@ -295,7 +295,7 @@ func TestReconcileClusterServiceBrokerRemovedClusterServiceClass(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) listRestrictions := clientgotesting.ListRestrictions{ @@ -363,7 +363,7 @@ func TestReconcileClusterServiceBrokerRemovedAndRestoredClusterServiceClass(t *t } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) listRestrictions := clientgotesting.ListRestrictions{ @@ -420,7 +420,7 @@ func TestReconcileClusterServiceBrokerRemovedClusterServicePlan(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) listRestrictions := clientgotesting.ListRestrictions{ @@ -461,7 +461,7 @@ func TestReconcileClusterServiceBrokerExistingClusterServiceClassDifferentBroker } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() @@ -513,7 +513,7 @@ func TestReconcileClusterServiceBrokerExistingClusterServicePlanDifferentClass(t } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() @@ -582,7 +582,7 @@ func TestReconcileClusterServiceBrokerDelete(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() @@ -642,7 +642,7 @@ func TestReconcileClusterServiceBrokerErrorFetchingCatalog(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() @@ -695,7 +695,7 @@ func TestReconcileClusterServiceBrokerZeroServices(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) // Verify no core kube actions occurred @@ -842,10 +842,10 @@ func testReconcileClusterServiceBrokerWithAuth(t *testing.T, authInfo *v1beta1.C brokerActions := fakeClusterServiceBrokerClient.Actions() if shouldSucceed { // GetCatalog - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) } else { - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) } actions := fakeCatalogClient.Actions() @@ -903,7 +903,7 @@ func TestReconcileClusterServiceBrokerWithReconcileError(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() @@ -960,7 +960,7 @@ func TestReconcileClusterServiceBrokerSuccessOnFinalRetry(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() @@ -1007,7 +1007,7 @@ func TestReconcileClusterServiceBrokerFailureOnFinalRetry(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() @@ -1058,7 +1058,7 @@ func TestReconcileClusterServiceBrokerWithStatusUpdateError(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertGetCatalog(t, brokerActions[0]) actions := fakeCatalogClient.Actions() diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index ff6e716b66d..2a6f2d6a5f1 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -74,7 +74,7 @@ func TestReconcileServiceInstanceNonExistentClusterServiceClass(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 2) @@ -124,7 +124,7 @@ func TestReconcileServiceInstanceNonExistentClusterServiceClassWithK8SName(t *te } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -158,7 +158,7 @@ func TestReconcileServiceInstanceNonExistentClusterServiceBroker(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -208,7 +208,7 @@ func TestReconcileServiceInstanceWithAuthError(t *testing.T) { // verify that no broker actions occurred brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // verify that one catalog client action occurred actions := fakeCatalogClient.Actions() @@ -268,7 +268,7 @@ func TestReconcileServiceInstanceNonExistentClusterServicePlan(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // ensure that there are two actions, one to list plans and an action // to set the condition on the instance to indicate that the service @@ -330,7 +330,7 @@ func TestReconcileServiceInstanceNonExistentClusterServicePlanK8SName(t *testing } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // ensure that there is only one action to to set the condition on the // instance to indicate that the service plan doesn't exist @@ -611,7 +611,7 @@ func TestReconcileServiceInstanceWithParameters(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) expectedKubeActions := []kubeClientAction{ {verb: "get", resourceName: "namespaces", checkType: checkGetActionType}, } @@ -669,7 +669,7 @@ func TestReconcileServiceInstanceWithParameters(t *testing.T) { } brokerActions = fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -740,7 +740,7 @@ func TestReconcileServiceInstanceResolvesReferences(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // We should get the following actions: // list call for ClusterServiceClass @@ -816,7 +816,7 @@ func TestReconcileServiceInstanceResolvesReferencesClusterServiceClassRefAlready } brokerActions := fakeServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // We should get the following actions: // list call for ClusterServicePlan @@ -876,7 +876,7 @@ func TestReconcileServiceInstanceWithProvisionCallFailure(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -939,7 +939,7 @@ func TestReconcileServiceInstanceWithTemporaryProvisionFailure(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) expectedKubeActions := []kubeClientAction{ {verb: "get", resourceName: "namespaces", checkType: checkGetActionType}, @@ -975,7 +975,7 @@ func TestReconcileServiceInstanceWithTemporaryProvisionFailure(t *testing.T) { } brokerActions = fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1058,7 +1058,7 @@ func TestReconcileServiceInstanceWithTerminalProvisionFailure(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1128,7 +1128,7 @@ func TestReconcileServiceInstance(t *testing.T) { instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) fakeCatalogClient.ClearActions() - assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) + assertNumberOfBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) fakeKubeClient.ClearActions() if err := reconcileServiceInstance(t, testController, instance); err != nil { @@ -1136,7 +1136,7 @@ func TestReconcileServiceInstance(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1185,7 +1185,7 @@ func TestReconcileServiceInstanceNamespacedRefs(t *testing.T) { } defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) - fakeKubeClient, fakeCatalogClient, _, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ ProvisionReaction: &fakeosb.ProvisionReaction{ Response: &osb.ProvisionResponse{ DashboardURL: &testDashboardURL, @@ -1208,53 +1208,53 @@ func TestReconcileServiceInstanceNamespacedRefs(t *testing.T) { instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) fakeCatalogClient.ClearActions() - // assertNumberOfClusterServiceBrokerActions(t, fakeClusterServiceBrokerClient.Actions(), 0) - // fakeKubeClient.ClearActions() - - // if err := reconcileServiceInstance(t, testController, instance); err != nil { - // t.Fatalf("This should not fail : %v", err) - // } - - // brokerActions := fakeClusterServiceBrokerClient.Actions() - // assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) - // assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ - // AcceptsIncomplete: true, - // InstanceID: testServiceInstanceGUID, - // ServiceID: testClusterServiceClassGUID, - // PlanID: testClusterServicePlanGUID, - // OrganizationGUID: testNamespaceGUID, - // SpaceGUID: testNamespaceGUID, - // Context: testContext}) - - // instanceKey := testNamespace + "/" + testServiceInstanceName - - // // Since synchronous operation, must not make it into the polling queue. - // if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { - // t.Fatalf("Expected polling queue to not have any record of test 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 err := checkKubeClientActions(kubeActions, []kubeClientAction{ - // {verb: "get", resourceName: "namespaces", checkType: checkGetActionType}, - // }); err != nil { - // t.Fatal(err) - // } - - // updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) - // assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testClusterServicePlanName, testClusterServicePlanGUID, instance) - // assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) - - // events := getRecordedEvents(testController) - - // expectedEvent := normalEventBuilder(successProvisionReason).msg(successProvisionMessage) - // if err := checkEvents(events, expectedEvent.stringArr()); err != nil { - // t.Fatal(err) - // } + assertNumberOfBrokerActions(t, fakeBrokerClient.Actions(), 0) + fakeKubeClient.ClearActions() + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("This should not fail : %v", err) + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ + AcceptsIncomplete: true, + InstanceID: testServiceInstanceGUID, + ServiceID: testServiceClassGUID, + PlanID: testServicePlanGUID, + OrganizationGUID: testNamespaceGUID, + SpaceGUID: testNamespaceGUID, + Context: testContext}) + + instanceKey := testNamespace + "/" + testServiceInstanceName + + // Since synchronous operation, must not make it into the polling queue. + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test 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 err := checkKubeClientActions(kubeActions, []kubeClientAction{ + {verb: "get", resourceName: "namespaces", checkType: checkGetActionType}, + }); err != nil { + t.Fatal(err) + } + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testServicePlanName, testServicePlanGUID, instance) + assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) + + events := getRecordedEvents(testController) + + expectedEvent := normalEventBuilder(successProvisionReason).msg(successProvisionMessage) + if err := checkEvents(events, expectedEvent.stringArr()); err != nil { + t.Fatal(err) + } } // TestReconcileServiceInstanceFailsWithDeletedPlan tests that a ServiceInstance is not @@ -1277,7 +1277,7 @@ func TestReconcileServiceInstanceFailsWithDeletedPlan(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) instanceKey := testNamespace + "/" + testServiceInstanceName @@ -1327,7 +1327,7 @@ func TestReconcileServiceInstanceFailsWithDeletedClass(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) instanceKey := testNamespace + "/" + testServiceInstanceName @@ -1411,7 +1411,7 @@ func TestReconcileServiceInstanceSuccessWithK8SNames(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1495,7 +1495,7 @@ func TestReconcileServiceInstanceAsynchronous(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1565,7 +1565,7 @@ func TestReconcileServiceInstanceAsynchronousNoOperation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1614,7 +1614,7 @@ func TestReconcileServiceInstanceNamespaceError(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // verify no kube resources created. // One single action comes from getting namespace uid @@ -1686,7 +1686,7 @@ func TestReconcileServiceInstanceDelete(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1753,7 +1753,7 @@ func TestReconcileServiceInstanceDeleteBlockedByCredentials(t *testing.T) { } brokerActions := fakeBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() @@ -1794,7 +1794,7 @@ func TestReconcileServiceInstanceDeleteBlockedByCredentials(t *testing.T) { } brokerActions = fakeBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1883,7 +1883,7 @@ func TestReconcileServiceInstanceDeleteAsynchronous(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -1955,7 +1955,7 @@ func TestReconcileServiceInstanceDeleteFailedProvisionWithRequest(t *testing.T) } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -2058,7 +2058,7 @@ func TestReconsileServiceInstanceDeleteWithParameters(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() @@ -2108,7 +2108,7 @@ func TestReconcileServiceInstanceDeleteWhenAlreadyDeprovisionedUnsuccessfully(t } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() @@ -2166,7 +2166,7 @@ func TestReconcileServiceInstanceDeleteFailedUpdate(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -2223,7 +2223,7 @@ func TestReconcileServiceInstanceDeleteDoesNotInvokeClusterServiceBroker(t *test } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() @@ -2283,7 +2283,7 @@ func TestFinalizerClearedWhen409ConflictEncounteredOnStatusUpdate(t *testing.T) } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // Verify no core kube actions occurred kubeActions := fakeKubeClient.Actions() @@ -2333,7 +2333,7 @@ func TestReconcileServiceInstanceWithFailedCondition(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -2406,7 +2406,7 @@ func TestPollServiceInstanceInProgressProvisioningWithOperation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2463,7 +2463,7 @@ func TestPollServiceInstanceSuccessProvisioningWithOperation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2517,7 +2517,7 @@ func TestPollServiceInstanceFailureProvisioningWithOperation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2601,7 +2601,7 @@ func TestPollServiceInstanceInProgressDeprovisioningWithOperationNoFinalizer(t * } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2660,7 +2660,7 @@ func TestPollServiceInstanceSuccessDeprovisioningWithOperationNoFinalizer(t *tes } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2721,7 +2721,7 @@ func TestPollServiceInstanceFailureDeprovisioning(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2793,7 +2793,7 @@ func TestPollServiceInstanceFailureDeprovisioningWithReconciliationTimeout(t *te } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2864,7 +2864,7 @@ func TestPollServiceInstanceStatusGoneDeprovisioningWithOperationNoFinalizer(t * } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2925,7 +2925,7 @@ func TestPollServiceInstanceClusterServiceBrokerError(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -2990,7 +2990,7 @@ func TestPollServiceInstanceSuccessDeprovisioningWithOperationWithFinalizer(t *t } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -3047,7 +3047,7 @@ func TestReconcileServiceInstanceSuccessOnFinalRetry(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -3129,7 +3129,7 @@ func TestReconcileServiceInstanceUpdateInProgressPropertiesOnRetry(t *testing.T) // No OSB requests expected brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) // InProgressProperties fields should be updated expectedParameters := map[string]interface{}{ @@ -3188,7 +3188,7 @@ func TestReconcileServiceInstanceFailureOnFinalRetry(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -3264,7 +3264,7 @@ func TestPollServiceInstanceSuccessOnFinalRetry(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -3319,7 +3319,7 @@ func TestPollServiceInstanceFailureOnFinalRetry(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -3376,7 +3376,7 @@ func TestReconcileServiceInstanceWithStatusUpdateError(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) @@ -3642,7 +3642,7 @@ func TestUpdateServiceInstanceCondition(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) if !reflect.DeepEqual(tc.input, inputClone) { t.Errorf("%v: updating broker condition mutated input: %s", tc.name, expectedGot(inputClone, tc.input)) @@ -3739,7 +3739,7 @@ func TestReconcileInstanceUsingOriginatingIdentity(t *testing.T) { } brokerActions := fakeBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) actualRequest, ok := brokerActions[0].Request.(*osb.ProvisionRequest) if !ok { t.Errorf("%v: unexpected request type; expected %T, got %T", tc.name, &osb.ProvisionRequest{}, actualRequest) @@ -3807,7 +3807,7 @@ func TestReconcileInstanceDeleteUsingOriginatingIdentity(t *testing.T) { } brokerActions := fakeBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) actualRequest, ok := brokerActions[0].Request.(*osb.DeprovisionRequest) if !ok { t.Errorf("%v: unexpected request type; expected %T, got %T", tc.name, &osb.DeprovisionRequest{}, actualRequest) @@ -3854,7 +3854,7 @@ func TestPollInstanceUsingOriginatingIdentity(t *testing.T) { } brokerActions := fakeBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) actualRequest, ok := brokerActions[0].Request.(*osb.LastOperationRequest) if !ok { t.Errorf("%v: unexpected request type; expected %T, got %T", tc.name, &osb.LastOperationRequest{}, actualRequest) @@ -4349,7 +4349,7 @@ func TestReconcileServiceInstanceWithSecretParameters(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -4554,7 +4554,7 @@ func TestReconcileServiceInstanceUpdateParameters(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUpdateInstance(t, brokerActions[0], &osb.UpdateInstanceRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -4657,7 +4657,7 @@ func TestReconcileServiceInstanceDeleteParameters(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertUpdateInstance(t, brokerActions[0], &osb.UpdateInstanceRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -4879,7 +4879,7 @@ func TestReconcileServiceInstanceUpdateDashboardURLResponse(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) expectedPlanID := testClusterServicePlanGUID assertUpdateInstance(t, brokerActions[0], &osb.UpdateInstanceRequest{ AcceptsIncomplete: true, @@ -4990,7 +4990,7 @@ func TestReconcileServiceInstanceUpdatePlan(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) expectedPlanID := testClusterServicePlanGUID assertUpdateInstance(t, brokerActions[0], &osb.UpdateInstanceRequest{ AcceptsIncomplete: true, @@ -5062,7 +5062,7 @@ func TestReconcileServiceInstanceWithUpdateCallFailure(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) expectedPlanID := testClusterServicePlanGUID assertUpdateInstance(t, brokerActions[0], &osb.UpdateInstanceRequest{ AcceptsIncomplete: true, @@ -5126,7 +5126,7 @@ func TestReconcileServiceInstanceWithUpdateFailure(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) expectedPlanID := testClusterServicePlanGUID assertUpdateInstance(t, brokerActions[0], &osb.UpdateInstanceRequest{ AcceptsIncomplete: true, @@ -5394,7 +5394,7 @@ func TestReconcileServiceInstanceUpdateAsynchronous(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) expectedPlanID := testClusterServicePlanGUID assertUpdateInstance(t, brokerActions[0], &osb.UpdateInstanceRequest{ AcceptsIncomplete: true, @@ -5455,7 +5455,7 @@ func TestPollServiceInstanceAsyncInProgressUpdating(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -5512,7 +5512,7 @@ func TestPollServiceInstanceAsyncSuccessUpdating(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -5566,7 +5566,7 @@ func TestPollServiceInstanceAsyncFailureUpdating(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) operationKey := osb.OperationKey(testOperation) assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ InstanceID: testServiceInstanceGUID, @@ -5669,7 +5669,7 @@ func TestCheckClassAndPlanForDeletion(t *testing.T) { // no actions ever assertNumberOfActions(t, fakeKubeClient.Actions(), 0) brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 0) } @@ -5729,7 +5729,7 @@ func TestReconcileServiceInstanceDeleteDuringOngoingOperation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -5813,7 +5813,7 @@ func TestReconcileServiceInstanceDeleteWithOngoingOperation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -5886,7 +5886,7 @@ func TestReconcileServiceInstanceDeleteWithNonExistentPlan(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 1) + assertNumberOfBrokerActions(t, brokerActions, 1) assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ AcceptsIncomplete: true, InstanceID: testServiceInstanceGUID, @@ -5944,7 +5944,7 @@ func TestReconcileServiceInstanceUpdateMissingObservedGeneration(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) @@ -5992,7 +5992,7 @@ func TestReconcileServiceInstanceUpdateMissingOrphanMitigation(t *testing.T) { } brokerActions := fakeClusterServiceBrokerClient.Actions() - assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + assertNumberOfBrokerActions(t, brokerActions, 0) actions := fakeCatalogClient.Actions() assertNumberOfActions(t, actions, 1) diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 8ede2841cb8..fbbcc75ddbb 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -2844,7 +2844,7 @@ func assertServiceInstanceInProgressPropertiesPlan(t *testing.T, obj runtime.Obj if !ok { fatalf(t, "Couldn't convert object %+v into a *v1beta1.ServiceInstance", obj) } - assertServiceInstancePropertiesStatePlan(t, "in-progress", instance, planName, planID) + assertServiceInstancePropertiesStatePlan(t, "in-progress", instance, instance.Status.InProgressProperties, planName, planID) } func assertServiceInstanceInProgressPropertiesParameters(t *testing.T, obj runtime.Object, params map[string]interface{}, checksum string) { @@ -2883,7 +2883,7 @@ func assertServiceInstanceExternalPropertiesPlan(t *testing.T, obj runtime.Objec if !ok { fatalf(t, "Couldn't convert object %+v into a *v1beta1.ServiceInstance", obj) } - assertServiceInstancePropertiesStatePlan(t, "external", instance, planName, planID) + assertServiceInstancePropertiesStatePlan(t, "external", instance, instance.Status.ExternalProperties, planName, planID) } func assertServiceInstanceExternalPropertiesParameters(t *testing.T, obj runtime.Object, params map[string]interface{}, checksum string) { @@ -2906,8 +2906,7 @@ func assertServiceInstanceExternalPropertiesUnchanged(t *testing.T, obj runtime. } } -func assertServiceInstancePropertiesStatePlan(t *testing.T, propsLabel string, instance *v1beta1.ServiceInstance, expectedPlanName string, expectedPlanID string) { - actualProps := instance.Status.InProgressProperties +func assertServiceInstancePropertiesStatePlan(t *testing.T, propsLabel string, instance *v1beta1.ServiceInstance, actualProps *v1beta1.ServiceInstancePropertiesState, expectedPlanName string, expectedPlanID string) { if actualProps == nil { fatalf(t, "expected %v properties to not be nil", propsLabel) } @@ -3501,8 +3500,8 @@ func testCatalogFinalizerExists(t *testing.T, name string, f failfFunc, obj runt return true } -func assertNumberOfClusterServiceBrokerActions(t *testing.T, actions []fakeosb.Action, number int) { - testNumberOfClusterServiceBrokerActions(t, "" /* name */, fatalf, actions, number) +func assertNumberOfBrokerActions(t *testing.T, actions []fakeosb.Action, number int) { + testNumberOfBrokerActions(t, "" /* name */, fatalf, actions, number) } // assertListRestrictions compares expected Fields / Labels on a list options. @@ -3516,10 +3515,10 @@ func assertListRestrictions(t *testing.T, e, a clientgotesting.ListRestrictions) } func expectNumberOfClusterServiceBrokerActions(t *testing.T, name string, actions []fakeosb.Action, number int) bool { - return testNumberOfClusterServiceBrokerActions(t, name, errorf, actions, number) + return testNumberOfBrokerActions(t, name, errorf, actions, number) } -func testNumberOfClusterServiceBrokerActions(t *testing.T, name string, f failfFunc, actions []fakeosb.Action, number int) bool { +func testNumberOfBrokerActions(t *testing.T, name string, f failfFunc, actions []fakeosb.Action, number int) bool { logContext := "" if len(name) > 0 { logContext = name + ": " From e03feea5d0cbd2ce1ea992a43dcea29c64c0eda2 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 16:05:37 -0400 Subject: [PATCH 09/24] Add reconcile async with namespaced refs --- pkg/controller/controller_instance_test.go | 80 ++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index 2a6f2d6a5f1..572a6741530 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -1525,6 +1525,86 @@ func TestReconcileServiceInstanceAsynchronous(t *testing.T) { } } +// TestReconcileServiceInstanceAsynchronousNamespacedRefs tests provisioning +// a new service from namespaced classes and plans, where the request results +// in a async response. Resulting status will indicate not ready and polling +// in progress. +func TestReconcileServiceInstanceAsynchronousNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + key := osb.OperationKey(testOperation) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + ProvisionReaction: &fakeosb.ProvisionReaction{ + Response: &osb.ProvisionResponse{ + Async: true, + DashboardURL: &testDashboardURL, + OperationKey: &key, + }, + }, + }) + + addGetNamespaceReaction(fakeKubeClient) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceWithNamespacedRefs() + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) + fakeCatalogClient.ClearActions() + fakeKubeClient.ClearActions() + + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("This should not fail : %v", err) + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ + AcceptsIncomplete: true, + InstanceID: testServiceInstanceGUID, + ServiceID: testServiceClassGUID, + PlanID: testServicePlanGUID, + OrganizationGUID: testNamespaceGUID, + SpaceGUID: testNamespaceGUID, + Context: testContext, + }) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testOperation, testServicePlanName, testServicePlanGUID, instance) + assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) + + // 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) + } + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 1 { + t.Fatalf("Expected polling queue to have a record of seeing test instance once") + } +} + // TestReconcileServiceInstanceAsynchronousNoOperation tests an async provision // scenario. This differs from TestReconcileServiceInstanceAsynchronous() as // there is no operation key returned by OSB. From 5887ad57d0cfe3ec75e27766959ff5995bd860fa Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 16:57:11 -0400 Subject: [PATCH 10/24] Give instance tests their own file --- pkg/controller/controller_instance_ns_test.go | 191 ++++++++++++++++++ pkg/controller/controller_instance_test.go | 160 --------------- 2 files changed, 191 insertions(+), 160 deletions(-) create mode 100644 pkg/controller/controller_instance_ns_test.go diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go new file mode 100644 index 00000000000..9a97dadea26 --- /dev/null +++ b/pkg/controller/controller_instance_ns_test.go @@ -0,0 +1,191 @@ +/* +Copyright 2017 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 + +import ( + "fmt" + "testing" + + osb "github.com/pmorie/go-open-service-broker-client/v2" + fakeosb "github.com/pmorie/go-open-service-broker-client/v2/fake" + + "github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1" + + utilfeature "k8s.io/apiserver/pkg/util/feature" + + scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" +) + +// TestReconcileServiceInstanceNamespacedRefs tests synchronously provisioning a new service +func TestReconcileServiceInstanceNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + ProvisionReaction: &fakeosb.ProvisionReaction{ + Response: &osb.ProvisionResponse{ + DashboardURL: &testDashboardURL, + }, + }, + }) + + addGetNamespaceReaction(fakeKubeClient) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceWithNamespacedRefs() + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) + fakeCatalogClient.ClearActions() + + assertNumberOfBrokerActions(t, fakeBrokerClient.Actions(), 0) + fakeKubeClient.ClearActions() + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("This should not fail : %v", err) + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ + AcceptsIncomplete: true, + InstanceID: testServiceInstanceGUID, + ServiceID: testServiceClassGUID, + PlanID: testServicePlanGUID, + OrganizationGUID: testNamespaceGUID, + SpaceGUID: testNamespaceGUID, + Context: testContext}) + + instanceKey := testNamespace + "/" + testServiceInstanceName + + // Since synchronous operation, must not make it into the polling queue. + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test 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 err := checkKubeClientActions(kubeActions, []kubeClientAction{ + {verb: "get", resourceName: "namespaces", checkType: checkGetActionType}, + }); err != nil { + t.Fatal(err) + } + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testServicePlanName, testServicePlanGUID, instance) + assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) + + events := getRecordedEvents(testController) + + expectedEvent := normalEventBuilder(successProvisionReason).msg(successProvisionMessage) + if err := checkEvents(events, expectedEvent.stringArr()); err != nil { + t.Fatal(err) + } +} + +// TestReconcileServiceInstanceAsynchronousNamespacedRefs tests provisioning +// a new service from namespaced classes and plans, where the request results +// in a async response. Resulting status will indicate not ready and polling +// in progress. +func TestReconcileServiceInstanceAsynchronousNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + key := osb.OperationKey(testOperation) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + ProvisionReaction: &fakeosb.ProvisionReaction{ + Response: &osb.ProvisionResponse{ + Async: true, + DashboardURL: &testDashboardURL, + OperationKey: &key, + }, + }, + }) + + addGetNamespaceReaction(fakeKubeClient) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceWithNamespacedRefs() + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("unexpected error: %v", err) + } + + instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) + fakeCatalogClient.ClearActions() + fakeKubeClient.ClearActions() + + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("This should not fail : %v", err) + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ + AcceptsIncomplete: true, + InstanceID: testServiceInstanceGUID, + ServiceID: testServiceClassGUID, + PlanID: testServicePlanGUID, + OrganizationGUID: testNamespaceGUID, + SpaceGUID: testNamespaceGUID, + Context: testContext, + }) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testOperation, testServicePlanName, testServicePlanGUID, instance) + assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) + + // 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) + } + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 1 { + t.Fatalf("Expected polling queue to have a record of seeing test instance once") + } +} diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index 572a6741530..98babeb686e 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -1177,86 +1177,6 @@ func TestReconcileServiceInstance(t *testing.T) { } } -// TestReconcileServiceInstanceNamespacedRefs tests synchronously provisioning a new service -func TestReconcileServiceInstanceNamespacedRefs(t *testing.T) { - err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) - if err != nil { - t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") - } - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) - - fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ - ProvisionReaction: &fakeosb.ProvisionReaction{ - Response: &osb.ProvisionResponse{ - DashboardURL: &testDashboardURL, - }, - }, - }) - - addGetNamespaceReaction(fakeKubeClient) - - sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) - sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) - sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) - - instance := getTestServiceInstanceWithNamespacedRefs() - - if err := reconcileServiceInstance(t, testController, instance); err != nil { - t.Fatalf("unexpected error: %v", err) - } - - instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) - fakeCatalogClient.ClearActions() - - assertNumberOfBrokerActions(t, fakeBrokerClient.Actions(), 0) - fakeKubeClient.ClearActions() - - if err := reconcileServiceInstance(t, testController, instance); err != nil { - t.Fatalf("This should not fail : %v", err) - } - - brokerActions := fakeBrokerClient.Actions() - assertNumberOfBrokerActions(t, brokerActions, 1) - assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ - AcceptsIncomplete: true, - InstanceID: testServiceInstanceGUID, - ServiceID: testServiceClassGUID, - PlanID: testServicePlanGUID, - OrganizationGUID: testNamespaceGUID, - SpaceGUID: testNamespaceGUID, - Context: testContext}) - - instanceKey := testNamespace + "/" + testServiceInstanceName - - // Since synchronous operation, must not make it into the polling queue. - if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { - t.Fatalf("Expected polling queue to not have any record of test 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 err := checkKubeClientActions(kubeActions, []kubeClientAction{ - {verb: "get", resourceName: "namespaces", checkType: checkGetActionType}, - }); err != nil { - t.Fatal(err) - } - - updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) - assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testServicePlanName, testServicePlanGUID, instance) - assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) - - events := getRecordedEvents(testController) - - expectedEvent := normalEventBuilder(successProvisionReason).msg(successProvisionMessage) - if err := checkEvents(events, expectedEvent.stringArr()); err != nil { - t.Fatal(err) - } -} - // TestReconcileServiceInstanceFailsWithDeletedPlan tests that a ServiceInstance is not // created if the ServicePlan specified is marked as RemovedFromCatalog. func TestReconcileServiceInstanceFailsWithDeletedPlan(t *testing.T) { @@ -1525,86 +1445,6 @@ func TestReconcileServiceInstanceAsynchronous(t *testing.T) { } } -// TestReconcileServiceInstanceAsynchronousNamespacedRefs tests provisioning -// a new service from namespaced classes and plans, where the request results -// in a async response. Resulting status will indicate not ready and polling -// in progress. -func TestReconcileServiceInstanceAsynchronousNamespacedRefs(t *testing.T) { - err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) - if err != nil { - t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") - } - defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) - - key := osb.OperationKey(testOperation) - - fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ - ProvisionReaction: &fakeosb.ProvisionReaction{ - Response: &osb.ProvisionResponse{ - Async: true, - DashboardURL: &testDashboardURL, - OperationKey: &key, - }, - }, - }) - - addGetNamespaceReaction(fakeKubeClient) - - sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) - sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) - sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) - - instance := getTestServiceInstanceWithNamespacedRefs() - - if err := reconcileServiceInstance(t, testController, instance); err != nil { - t.Fatalf("unexpected error: %v", err) - } - - instance = assertServiceInstanceProvisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) - fakeCatalogClient.ClearActions() - fakeKubeClient.ClearActions() - - instanceKey := testNamespace + "/" + testServiceInstanceName - - if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { - t.Fatalf("Expected polling queue to not have any record of test instance") - } - - if err := reconcileServiceInstance(t, testController, instance); err != nil { - t.Fatalf("This should not fail : %v", err) - } - - brokerActions := fakeBrokerClient.Actions() - assertNumberOfBrokerActions(t, brokerActions, 1) - assertProvision(t, brokerActions[0], &osb.ProvisionRequest{ - AcceptsIncomplete: true, - InstanceID: testServiceInstanceGUID, - ServiceID: testServiceClassGUID, - PlanID: testServicePlanGUID, - OrganizationGUID: testNamespaceGUID, - SpaceGUID: testNamespaceGUID, - Context: testContext, - }) - - actions := fakeCatalogClient.Actions() - assertNumberOfActions(t, actions, 1) - - updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) - assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testOperation, testServicePlanName, testServicePlanGUID, instance) - assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) - - // 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) - } - - if testController.instancePollingQueue.NumRequeues(instanceKey) != 1 { - t.Fatalf("Expected polling queue to have a record of seeing test instance once") - } -} - // TestReconcileServiceInstanceAsynchronousNoOperation tests an async provision // scenario. This differs from TestReconcileServiceInstanceAsynchronous() as // there is no operation key returned by OSB. From 221efbf9be7e22272eb06d8ed68da5a33cf56201 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 17:20:38 -0400 Subject: [PATCH 11/24] Add ResolveNamespacedReferences test --- pkg/controller/controller_instance_ns_test.go | 73 +++++++++++++++++++ pkg/controller/controller_ns_test.go | 20 +++++ 2 files changed, 93 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index 9a97dadea26..d5ed1001a00 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -28,6 +28,10 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" + "k8s.io/apimachinery/pkg/fields" + "k8s.io/apimachinery/pkg/labels" + "k8s.io/apimachinery/pkg/runtime" + clientgotesting "k8s.io/client-go/testing" ) // TestReconcileServiceInstanceNamespacedRefs tests synchronously provisioning a new service @@ -189,3 +193,72 @@ func TestReconcileServiceInstanceAsynchronousNamespacedRefs(t *testing.T) { t.Fatalf("Expected polling queue to have a record of seeing test instance once") } } + +// TestResolveNamespacedReferences tests that resolveReferences works +// correctly and resolves references when the references are of namespaced. +func TestResolveNamespacedReferencesWorks(t *testing.T) { + fakeKubeClient, fakeCatalogClient, _, testController, _ := newTestController(t, noFakeActions()) + + instance := getTestServiceInstanceWithNamespacedPlanReference() + + sc := getTestServiceClass() + var scItems []v1beta1.ServiceClass + scItems = append(scItems, *sc) + fakeCatalogClient.AddReactor("list", "serviceclasses", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1beta1.ServiceClassList{Items: scItems}, nil + }) + sp := getTestServicePlan() + var spItems []v1beta1.ServicePlan + spItems = append(spItems, *sp) + fakeCatalogClient.AddReactor("list", "serviceplans", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, &v1beta1.ServicePlanList{Items: spItems}, nil + }) + + modified, err := testController.resolveReferences(instance) + if err != nil { + t.Fatalf("Should not have failed, but failed with: %q", err) + } + + if !modified { + t.Fatalf("Should have returned true") + } + + // We should get the following actions: + // list call for ServiceClass + // list call for ServicePlan + // updating references + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 3) + + listRestrictions := clientgotesting.ListRestrictions{ + Labels: labels.Everything(), + Fields: fields.OneTermEqualSelector("spec.externalName", instance.Spec.ServiceClassExternalName), + } + assertList(t, actions[0], &v1beta1.ServiceClass{}, listRestrictions) + + listRestrictions = clientgotesting.ListRestrictions{ + Labels: labels.Everything(), + Fields: fields.ParseSelectorOrDie("spec.externalName=test-serviceplan,spec.serviceBrokerName=test-servicebroker,spec.serviceClassRef.name=SCGUID"), + } + assertList(t, actions[1], &v1beta1.ServicePlan{}, listRestrictions) + + updatedServiceInstance := assertUpdateReference(t, actions[2], instance) + updateObject, ok := updatedServiceInstance.(*v1beta1.ServiceInstance) + if !ok { + t.Fatalf("couldn't convert to *v1beta1.ServiceInstance") + } + if updateObject.Spec.ServiceClassRef == nil || updateObject.Spec.ServiceClassRef.Name != testServiceClassGUID { + t.Fatalf("ServiceClassRef was not resolved correctly during reconcile") + } + if updateObject.Spec.ServicePlanRef == nil || updateObject.Spec.ServicePlanRef.Name != testServicePlanGUID { + t.Fatalf("ServicePlanRef was not resolved correctly during reconcile") + } + + // verify no kube resources created + // One single action comes from getting namespace uid + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + + events := getRecordedEvents(testController) + assertNumEvents(t, events, 0) +} diff --git a/pkg/controller/controller_ns_test.go b/pkg/controller/controller_ns_test.go index 0e0b7ac1883..9b3e1d6f50e 100644 --- a/pkg/controller/controller_ns_test.go +++ b/pkg/controller/controller_ns_test.go @@ -97,3 +97,23 @@ func getTestServicePlan() *v1beta1.ServicePlan { Status: v1beta1.ServicePlanStatus{}, } } + +func getTestServiceInstanceWithNamespacedPlanReference() *v1beta1.ServiceInstance { + return &v1beta1.ServiceInstance{ + ObjectMeta: metav1.ObjectMeta{ + Name: testServiceInstanceName, + Namespace: testNamespace, + Generation: 1, + }, + Spec: v1beta1.ServiceInstanceSpec{ + PlanReference: v1beta1.PlanReference{ + ServiceClassExternalName: testServiceClassName, + ServicePlanExternalName: testServicePlanName, + }, + ExternalID: testServiceInstanceGUID, + }, + Status: v1beta1.ServiceInstanceStatus{ + DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, + }, + } +} From e317ba17cbe1e70116839b5cf61a3c200a73c604 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 17:58:56 -0400 Subject: [PATCH 12/24] Add instance DeleteWithNamespacedRefs sync test --- pkg/controller/controller_instance_ns_test.go | 78 +++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index d5ed1001a00..e28bd970267 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -28,6 +28,7 @@ import ( utilfeature "k8s.io/apiserver/pkg/util/feature" scfeatures "github.com/kubernetes-incubator/service-catalog/pkg/features" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/fields" "k8s.io/apimachinery/pkg/labels" "k8s.io/apimachinery/pkg/runtime" @@ -194,6 +195,83 @@ func TestReconcileServiceInstanceAsynchronousNamespacedRefs(t *testing.T) { } } +// TestReconcileServiceInstanceDeleteWithNamespacedRefs tests +// deletingdeprovisioning an instance with namespaced refs +func TestReconcileServiceInstanceDeleteWithNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + DeprovisionReaction: &fakeosb.DeprovisionReaction{ + Response: &osb.DeprovisionResponse{}, + }, + }) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceWithNamespacedRefs() + instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} + instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} + // we only invoke the broker client to deprovision if we have a reconciled generation set + // as that implies a previous success. + instance.Generation = 2 + instance.Status.ReconciledGeneration = 1 + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned + instance.Status.ExternalProperties = &v1beta1.ServiceInstancePropertiesState{ + ServicePlanExternalName: testServicePlanName, + ServicePlanExternalID: testServicePlanGUID, + } + instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired + + fakeCatalogClient.AddReactor("get", "serviceinstances", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, instance, nil + }) + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("unexpected error: %v", err) + } + instance = assertServiceInstanceDeprovisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) + fakeCatalogClient.ClearActions() + fakeKubeClient.ClearActions() + + err = reconcileServiceInstance(t, testController, instance) + if err != nil { + t.Fatalf("This should not fail") + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ + AcceptsIncomplete: true, + InstanceID: testServiceInstanceGUID, + ServiceID: testServiceClassGUID, + PlanID: testServicePlanGUID, + }) + + // Verify no core kube actions occurred + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testClusterServicePlanName, testClusterServicePlanGUID, instance) + + events := getRecordedEvents(testController) + + expectedEvent := normalEventBuilder(successDeprovisionReason).msg("The instance was deprovisioned successfully") + if err := checkEvents(events, expectedEvent.stringArr()); err != nil { + t.Fatal(err) + } +} + // TestResolveNamespacedReferences tests that resolveReferences works // correctly and resolves references when the references are of namespaced. func TestResolveNamespacedReferencesWorks(t *testing.T) { From 5b327cd59d149650b8afa95d3ddb37fcd32b71e6 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 18:16:05 -0400 Subject: [PATCH 13/24] Add instance DeleteAsyncWithNamespaced refs test --- pkg/controller/controller_instance_ns_test.go | 91 +++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index e28bd970267..41cc6dc2dd2 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -272,6 +272,97 @@ func TestReconcileServiceInstanceDeleteWithNamespacedRefs(t *testing.T) { } } +func TestReconcileServiceInstanceDeleteAsynchronousWithNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + key := osb.OperationKey(testOperation) + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + DeprovisionReaction: &fakeosb.DeprovisionReaction{ + Response: &osb.DeprovisionResponse{ + Async: true, + OperationKey: &key, + }, + }, + }) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceWithNamespacedRefs() + instance.ObjectMeta.DeletionTimestamp = &metav1.Time{} + instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} + // we only invoke the broker client to deprovision if we have a reconciled generation set + // as that implies a previous success. + instance.Generation = 2 + instance.Status.ReconciledGeneration = 1 + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned + instance.Status.ExternalProperties = &v1beta1.ServiceInstancePropertiesState{ + ServicePlanExternalName: testServicePlanName, + ServicePlanExternalID: testServicePlanGUID, + } + instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired + + fakeCatalogClient.AddReactor("get", "serviceinstances", func(action clientgotesting.Action) (bool, runtime.Object, error) { + return true, instance, nil + }) + + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + if err := reconcileServiceInstance(t, testController, instance); err != nil { + t.Fatalf("unexpected error: %v", err) + } + instance = assertServiceInstanceDeprovisionInProgressIsTheOnlyCatalogClientAction(t, fakeCatalogClient, instance) + fakeCatalogClient.ClearActions() + fakeKubeClient.ClearActions() + + err = reconcileServiceInstance(t, testController, instance) + if err != nil { + t.Fatalf("This should not fail : %v", err) + } + + // The item should've been added to the instancePollingQueue for later processing + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 1 { + t.Fatalf("Expected polling queue to have a record of seeing test instance once") + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + assertDeprovision(t, brokerActions[0], &osb.DeprovisionRequest{ + AcceptsIncomplete: true, + InstanceID: testServiceInstanceGUID, + ServiceID: testServiceClassGUID, + PlanID: testServicePlanGUID, + }) + + // Verify no core kube actions occurred + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testOperation, testServicePlanName, testServicePlanGUID, instance) + + events := getRecordedEvents(testController) + + expectedEvent := normalEventBuilder(asyncDeprovisioningReason).msg("The instance is being deprovisioned asynchronously") + if err := checkEvents(events, expectedEvent.stringArr()); err != nil { + t.Fatal(err) + } +} + // TestResolveNamespacedReferences tests that resolveReferences works // correctly and resolves references when the references are of namespaced. func TestResolveNamespacedReferencesWorks(t *testing.T) { From 6754adf7df2d502069b8a2a793ed8b4eb3c5f094 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Mon, 18 Jun 2018 10:47:24 -0400 Subject: [PATCH 14/24] Buildable prepareUpdateInstanceRequest --- pkg/controller/controller_instance.go | 197 +++++++++++++++------ pkg/controller/controller_instance_test.go | 2 +- 2 files changed, 146 insertions(+), 53 deletions(-) diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index 6a879a2bc4b..b0226e55eb8 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -481,42 +481,88 @@ func (c *controller) reconcileServiceInstanceUpdate(instance *v1beta1.ServiceIns glog.V(4).Info(pcb.Message("Processing updating event")) - serviceClass, servicePlan, brokerName, brokerClient, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) - if err != nil { - return c.handleServiceInstanceReconciliationError(instance, err) - } + var brokerClient osb.Client + var request *osb.UpdateInstanceRequest - // Check if the ServiceClass or ServicePlan has been deleted. If so, do - // not allow plan upgrades, but do allow parameter changes. - if err := c.checkForRemovedClusterClassAndPlan(instance, serviceClass, servicePlan); err != nil { - return c.handleServiceInstanceReconciliationError(instance, err) - } + if instance.Spec.ClusterServiceClassSpecified() { - request, inProgressProperties, err := c.prepareUpdateInstanceRequest(instance, serviceClass, servicePlan) - if err != nil { - return c.handleServiceInstanceReconciliationError(instance, err) - } + serviceClass, servicePlan, brokerName, bClient, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) + if err != nil { + return c.handleServiceInstanceReconciliationError(instance, err) + } - if instance.Status.CurrentOperation == "" || !isServiceInstancePropertiesStateEqual(instance.Status.InProgressProperties, inProgressProperties) { - instance, err = c.recordStartOfServiceInstanceOperation(instance, v1beta1.ServiceInstanceOperationUpdate, inProgressProperties) + brokerClient = bClient + + // Check if the ServiceClass or ServicePlan has been deleted. If so, do + // not allow plan upgrades, but do allow parameter changes. + if err := c.checkForRemovedClusterClassAndPlan(instance, serviceClass, servicePlan); err != nil { + return c.handleServiceInstanceReconciliationError(instance, err) + } + + req, inProgressProperties, err := c.prepareUpdateInstanceRequest(instance) if err != nil { - // There has been an update to the instance. Start reconciliation - // over with a fresh view of the instance. - return err + return c.handleServiceInstanceReconciliationError(instance, err) } - // recordStartOfServiceInstanceOperation has updated the instance, so we need to continue in the next iteration - return nil - } + request = req - glog.V(4).Info(pcb.Messagef( - "Updating ServiceInstance of %s at ClusterServiceBroker %q", - pretty.ClusterServiceClassName(serviceClass), brokerName, - )) + if instance.Status.CurrentOperation == "" || !isServiceInstancePropertiesStateEqual(instance.Status.InProgressProperties, inProgressProperties) { + instance, err = c.recordStartOfServiceInstanceOperation(instance, v1beta1.ServiceInstanceOperationUpdate, inProgressProperties) + if err != nil { + // There has been an update to the instance. Start reconciliation + // over with a fresh view of the instance. + return err + } + // recordStartOfServiceInstanceOperation has updated the instance, so we need to continue in the next iteration + return nil + } + + glog.V(4).Info(pcb.Messagef( + "Updating ServiceInstance of %s at ClusterServiceBroker %q", + pretty.ClusterServiceClassName(serviceClass), brokerName, + )) + + } else if instance.Spec.ServiceClassSpecified() { + + serviceClass, servicePlan, brokerName, bClient, err := c.getServiceClassPlanAndServiceBroker(instance) + if err != nil { + return c.handleServiceInstanceReconciliationError(instance, err) + } + + brokerClient = bClient + + // Check if the ServiceClass or ServicePlan has been deleted. If so, do + // not allow plan upgrades, but do allow parameter changes. + if err := c.checkForRemovedClassAndPlan(instance, serviceClass, servicePlan); err != nil { + return c.handleServiceInstanceReconciliationError(instance, err) + } + + req, inProgressProperties, err := c.prepareUpdateInstanceRequest(instance) + if err != nil { + return c.handleServiceInstanceReconciliationError(instance, err) + } + request = req + + if instance.Status.CurrentOperation == "" || !isServiceInstancePropertiesStateEqual(instance.Status.InProgressProperties, inProgressProperties) { + instance, err = c.recordStartOfServiceInstanceOperation(instance, v1beta1.ServiceInstanceOperationUpdate, inProgressProperties) + if err != nil { + // There has been an update to the instance. Start reconciliation + // over with a fresh view of the instance. + return err + } + // recordStartOfServiceInstanceOperation has updated the instance, so we need to continue in the next iteration + return nil + } + + glog.V(4).Info(pcb.Messagef( + "Updating ServiceInstance of %s at ServiceBroker %q", + pretty.ServiceClassName(serviceClass), brokerName, + )) + } response, err := brokerClient.UpdateInstance(request) if err != nil { if httpErr, ok := osb.IsHTTPError(err); ok { - msg := fmt.Sprintf("ClusterServiceBroker returned a failure for update call; update will not be retried: %v", httpErr) + msg := fmt.Sprintf("ServiceBroker returned a failure for update call; update will not be retried: %v", httpErr) readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, errorUpdateInstanceCallFailedReason, msg) if isRetriableHTTPStatus(httpErr.StatusCode) { @@ -531,7 +577,7 @@ func (c *controller) reconcileServiceInstanceUpdate(instance *v1beta1.ServiceIns reason := errorErrorCallingUpdateInstanceReason if urlErr, ok := err.(*url.Error); ok && urlErr.Timeout() { - msg := fmt.Sprintf("Communication with the ClusterServiceBroker timed out; update will be retried: %v", urlErr) + msg := fmt.Sprintf("Communication with the ServiceBroker timed out; update will be retried: %v", urlErr) readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, reason, msg) return c.processTemporaryUpdateServiceInstanceFailure(instance, readyCond) } @@ -1865,34 +1911,81 @@ func (c *controller) innerPrepareProvisionRequest(instance *v1beta1.ServiceInsta // prepareUpdateInstanceRequest creates an update instance request object to be // passed to the broker client to update the given instance. -func (c *controller) prepareUpdateInstanceRequest(instance *v1beta1.ServiceInstance, serviceClass *v1beta1.ClusterServiceClass, servicePlan *v1beta1.ClusterServicePlan) (*osb.UpdateInstanceRequest, *v1beta1.ServiceInstancePropertiesState, error) { - rh, err := c.prepareRequestHelper(instance, servicePlan.Spec.ExternalName, servicePlan.Spec.ExternalID, true) - if err != nil { - return nil, nil, err - } +func (c *controller) prepareUpdateInstanceRequest(instance *v1beta1.ServiceInstance) (*osb.UpdateInstanceRequest, *v1beta1.ServiceInstancePropertiesState, error) { - request := &osb.UpdateInstanceRequest{ - AcceptsIncomplete: true, - InstanceID: instance.Spec.ExternalID, - ServiceID: serviceClass.Spec.ExternalID, - Context: rh.requestContext, - OriginatingIdentity: rh.originatingIdentity, - } + var rh *requestHelper + var request *osb.UpdateInstanceRequest - // Only send the plan ID if the plan ID has changed from what the Broker has - if instance.Status.ExternalProperties == nil || - servicePlan.Spec.ExternalID != instance.Status.ExternalProperties.ClusterServicePlanExternalID { - planID := servicePlan.Spec.ExternalID - request.PlanID = &planID - } - // Only send the parameters if they have changed from what the Broker has - if instance.Status.ExternalProperties == nil || - rh.inProgressProperties.ParametersChecksum != instance.Status.ExternalProperties.ParametersChecksum { - if rh.parameters != nil { - request.Parameters = rh.parameters - } else { - request.Parameters = make(map[string]interface{}) + if instance.Spec.ClusterServiceClassSpecified() { + serviceClass, servicePlan, _, _, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) + if err != nil { + return nil, nil, c.handleServiceInstanceReconciliationError(instance, err) + } + + rh, err = c.prepareRequestHelper(instance, servicePlan.Spec.ExternalName, servicePlan.Spec.ExternalID, true) + if err != nil { + return nil, nil, err + } + + request = &osb.UpdateInstanceRequest{ + AcceptsIncomplete: true, + InstanceID: instance.Spec.ExternalID, + ServiceID: serviceClass.Spec.ExternalID, + Context: rh.requestContext, + OriginatingIdentity: rh.originatingIdentity, + } + + // Only send the plan ID if the plan ID has changed from what the Broker has + if instance.Status.ExternalProperties == nil || + servicePlan.Spec.ExternalID != instance.Status.ExternalProperties.ClusterServicePlanExternalID { + planID := servicePlan.Spec.ExternalID + request.PlanID = &planID + } + // Only send the parameters if they have changed from what the Broker has + if instance.Status.ExternalProperties == nil || + rh.inProgressProperties.ParametersChecksum != instance.Status.ExternalProperties.ParametersChecksum { + if rh.parameters != nil { + request.Parameters = rh.parameters + } else { + request.Parameters = make(map[string]interface{}) + } } + + } else if instance.Spec.ServiceClassSpecified() { + serviceClass, servicePlan, _, _, err := c.getServiceClassPlanAndServiceBroker(instance) + if err != nil { + return nil, nil, c.handleServiceInstanceReconciliationError(instance, err) + } + + rh, err = c.prepareRequestHelper(instance, servicePlan.Spec.ExternalName, servicePlan.Spec.ExternalID, true) + if err != nil { + return nil, nil, err + } + + request = &osb.UpdateInstanceRequest{ + AcceptsIncomplete: true, + InstanceID: instance.Spec.ExternalID, + ServiceID: serviceClass.Spec.ExternalID, + Context: rh.requestContext, + OriginatingIdentity: rh.originatingIdentity, + } + + // Only send the plan ID if the plan ID has changed from what the Broker has + if instance.Status.ExternalProperties == nil || + servicePlan.Spec.ExternalID != instance.Status.ExternalProperties.ServicePlanExternalID { + planID := servicePlan.Spec.ExternalID + request.PlanID = &planID + } + // Only send the parameters if they have changed from what the Broker has + if instance.Status.ExternalProperties == nil || + rh.inProgressProperties.ParametersChecksum != instance.Status.ExternalProperties.ParametersChecksum { + if rh.parameters != nil { + request.Parameters = rh.parameters + } else { + request.Parameters = make(map[string]interface{}) + } + } + } return request, rh.inProgressProperties, nil diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index 98babeb686e..472ec6675e3 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -5071,7 +5071,7 @@ func TestReconcileServiceInstanceWithUpdateFailure(t *testing.T) { events := getRecordedEvents(testController) - expectedEvent := warningEventBuilder(errorUpdateInstanceCallFailedReason).msg("ClusterServiceBroker returned a failure for update call; update will not be retried:").msg("Status: 409; ErrorMessage: OutOfQuota; Description: You're out of quota!; ResponseError: ") + expectedEvent := warningEventBuilder(errorUpdateInstanceCallFailedReason).msg("ServiceBroker returned a failure for update call; update will not be retried:").msg("Status: 409; ErrorMessage: OutOfQuota; Description: You're out of quota!; ResponseError: ") if err := checkEvents(events, expectedEvent.stringArr()); err != nil { t.Fatal(err) } From 334c3b540ea0cdaa57d0464f0b1b6fe430e50780 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 21:45:23 -0400 Subject: [PATCH 15/24] Fix in progress polling for instances --- pkg/controller/controller_instance.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index b0226e55eb8..7e2497df8c3 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -755,7 +755,13 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro instance = instance.DeepCopy() - _, _, _, brokerClient, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) + var brokerClient osb.Client + var err error + if instance.Spec.ClusterServiceClassSpecified() { + _, _, _, brokerClient, err = c.getClusterServiceClassPlanAndClusterServiceBroker(instance) + } else { + _, _, _, brokerClient, err = c.getServiceClassPlanAndServiceBroker(instance) + } if err != nil { return c.handleServiceInstanceReconciliationError(instance, err) } @@ -2056,6 +2062,7 @@ func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta var rh *requestHelper var scExternalID string + var spExternalID string if instance.Spec.ClusterServiceClassSpecified() { serviceClass, servicePlan, _, _, err := c.getClusterServiceClassPlanAndClusterServiceBroker(instance) @@ -2065,9 +2072,8 @@ func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta scExternalID = serviceClass.Spec.ExternalID - // Sometimes the servicePlan is nil + // Sometimes the servicePlan is nil (deprovision) var spExternalName string - var spExternalID string if servicePlan != nil { spExternalName = servicePlan.Spec.ExternalName spExternalID = servicePlan.Spec.ExternalID @@ -2085,9 +2091,8 @@ func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta scExternalID = serviceClass.Spec.ExternalID - // Sometimes the servicePlan is nil + // Sometimes the servicePlan is nil (deprovision) var spExternalName string - var spExternalID string if servicePlan != nil { spExternalName = servicePlan.Spec.ExternalName spExternalID = servicePlan.Spec.ExternalID @@ -2109,7 +2114,7 @@ func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta request := &osb.LastOperationRequest{ InstanceID: instance.Spec.ExternalID, ServiceID: &scExternalID, - PlanID: &instance.Status.InProgressProperties.ClusterServicePlanExternalID, + PlanID: &spExternalID, OriginatingIdentity: rh.originatingIdentity, } if instance.Status.LastOperation != nil && *instance.Status.LastOperation != "" { From 732ae375ec4f87237615e3c244ce47dd820b1e1d Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 21:45:37 -0400 Subject: [PATCH 16/24] Add inprogress provision polling test --- pkg/controller/controller_instance_ns_test.go | 64 +++++++++++++++++++ pkg/controller/controller_ns_test.go | 28 ++++++++ 2 files changed, 92 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index 41cc6dc2dd2..cf5fb4bc65d 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -195,6 +195,70 @@ func TestReconcileServiceInstanceAsynchronousNamespacedRefs(t *testing.T) { } } +// TestPollServiceInstanceInProgressProvisioningWithOperationNamespacedRefs +// tests polling an instance that is already in process of provisioning +// (background/asynchronously) and is still in progress (should be re-polled) +// The instance being provisioned here refers to namespaced classes and plans. +func TestPollServiceInstanceInProgressProvisioningWithOperationNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + PollLastOperationReaction: &fakeosb.PollLastOperationReaction{ + Response: &osb.LastOperationResponse{ + State: osb.StateInProgress, + Description: strPtr(lastOperationDescription), + }, + }, + }) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceAsyncProvisioningWithNamespacedRefs(testOperation) + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + err = testController.pollServiceInstance(instance) + if err != nil { + t.Fatalf("pollServiceInstance failed: %s", err) + } + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 1 { + t.Fatalf("Expected polling queue to have record of seeing test instance once") + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + operationKey := osb.OperationKey(testOperation) + assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ + InstanceID: testServiceInstanceGUID, + ServiceID: strPtr(testServiceClassGUID), + PlanID: strPtr(testServicePlanGUID), + OperationKey: &operationKey, + }) + + // there should have been 1 action to update the status with the last operation description + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testOperation, testServicePlanName, testServicePlanGUID, instance) + assertServiceInstanceConditionHasLastOperationDescription(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, lastOperationDescription) + + // verify no kube resources created. + // No actions + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) +} + // TestReconcileServiceInstanceDeleteWithNamespacedRefs tests // deletingdeprovisioning an instance with namespaced refs func TestReconcileServiceInstanceDeleteWithNamespacedRefs(t *testing.T) { diff --git a/pkg/controller/controller_ns_test.go b/pkg/controller/controller_ns_test.go index 9b3e1d6f50e..0229bff8953 100644 --- a/pkg/controller/controller_ns_test.go +++ b/pkg/controller/controller_ns_test.go @@ -117,3 +117,31 @@ func getTestServiceInstanceWithNamespacedPlanReference() *v1beta1.ServiceInstanc }, } } + +func getTestServiceInstanceAsyncProvisioningWithNamespacedRefs(operation string) *v1beta1.ServiceInstance { + instance := getTestServiceInstanceWithNamespacedRefs() + + operationStartTime := metav1.NewTime(time.Now().Add(-1 * time.Hour)) + instance.Status = v1beta1.ServiceInstanceStatus{ + Conditions: []v1beta1.ServiceInstanceCondition{{ + Type: v1beta1.ServiceInstanceConditionReady, + Status: v1beta1.ConditionFalse, + Message: "Provisioning", + LastTransitionTime: metav1.NewTime(time.Now().Add(-5 * time.Minute)), + }}, + AsyncOpInProgress: true, + OperationStartTime: &operationStartTime, + CurrentOperation: v1beta1.ServiceInstanceOperationProvision, + InProgressProperties: &v1beta1.ServiceInstancePropertiesState{ + ServicePlanExternalName: testServicePlanName, + ServicePlanExternalID: testServicePlanGUID, + }, + ObservedGeneration: instance.Generation, + DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, + } + if operation != "" { + instance.Status.LastOperation = &operation + } + + return instance +} From 3916378759d18acd2afb057ff298631bd6a499bc Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 22:18:17 -0400 Subject: [PATCH 17/24] Cover the nil plan case for last op reqs --- pkg/controller/controller_instance.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index 7e2497df8c3..3da31004dfe 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -2059,7 +2059,6 @@ func (c *controller) prepareDeprovisionRequest(instance *v1beta1.ServiceInstance // preparePollServiceInstanceRequest creates a request object to be passed to // the broker client to query the given instance's last operation endpoint. func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta1.ServiceInstance) (*osb.LastOperationRequest, error) { - var rh *requestHelper var scExternalID string var spExternalID string @@ -2072,11 +2071,13 @@ func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta scExternalID = serviceClass.Spec.ExternalID - // Sometimes the servicePlan is nil (deprovision) var spExternalName string if servicePlan != nil { spExternalName = servicePlan.Spec.ExternalName spExternalID = servicePlan.Spec.ExternalID + } else { + // If the ServicePlan is nil, pull from the InProgressProperties + spExternalID = instance.Status.InProgressProperties.ClusterServicePlanExternalID } rh, err = c.prepareRequestHelper(instance, spExternalName, spExternalID, false) @@ -2096,6 +2097,9 @@ func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta if servicePlan != nil { spExternalName = servicePlan.Spec.ExternalName spExternalID = servicePlan.Spec.ExternalID + } else { + // If the ServicePlan is nil, pull from the InProgressProperties + spExternalID = instance.Status.InProgressProperties.ServicePlanExternalID } rh, err = c.prepareRequestHelper(instance, spExternalName, spExternalID, false) From 35cc9edbb996e874589599d1076b8d41f9b05775 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Mon, 18 Jun 2018 22:34:00 -0400 Subject: [PATCH 18/24] Add success async provision ns type test --- pkg/controller/controller_instance_ns_test.go | 61 +++++++++++++++++++ 1 file changed, 61 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index cf5fb4bc65d..b0062e5d3ef 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -259,6 +259,67 @@ func TestPollServiceInstanceInProgressProvisioningWithOperationNamespacedRefs(t assertNumberOfActions(t, kubeActions, 0) } +// TestPollServiceInstanceSuccessProvisioningWithOperationNamespacedRefs tests +// polling an instance that is already in process of provisioning (background/ +// asynchronously) and is found to be ready. +func TestPollServiceInstanceSuccessProvisioningWithOperationNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + PollLastOperationReaction: &fakeosb.PollLastOperationReaction{ + Response: &osb.LastOperationResponse{ + State: osb.StateSucceeded, + Description: strPtr(lastOperationDescription), + }, + }, + }) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceAsyncProvisioningWithNamespacedRefs(testOperation) + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + err = testController.pollServiceInstance(instance) + if err != nil { + t.Fatalf("pollServiceInstance failed: %s", err) + } + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have requeues of test instance after polling have completed with a 'success' state") + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + operationKey := osb.OperationKey(testOperation) + assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ + InstanceID: testServiceInstanceGUID, + ServiceID: strPtr(testServiceClassGUID), + PlanID: strPtr(testServicePlanGUID), + OperationKey: &operationKey, + }) + + // verify no kube resources created. + // No actions + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testServicePlanName, testServicePlanGUID, instance) +} + // TestReconcileServiceInstanceDeleteWithNamespacedRefs tests // deletingdeprovisioning an instance with namespaced refs func TestReconcileServiceInstanceDeleteWithNamespacedRefs(t *testing.T) { From d0c11dd009ee4079442887ef05449319b46b6da7 Mon Sep 17 00:00:00 2001 From: "jesus m. rodriguez" Date: Mon, 18 Jun 2018 22:35:27 -0400 Subject: [PATCH 19/24] check properties for nil early --- pkg/controller/controller_instance.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index 3da31004dfe..ce6e1188301 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -2059,6 +2059,14 @@ func (c *controller) prepareDeprovisionRequest(instance *v1beta1.ServiceInstance // preparePollServiceInstanceRequest creates a request object to be passed to // the broker client to query the given instance's last operation endpoint. func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta1.ServiceInstance) (*osb.LastOperationRequest, error) { + + if instance.Status.InProgressProperties == nil { + pcb := pretty.NewInstanceContextBuilder(instance) + err := stderrors.New("Instance.Status.InProgressProperties can not be nil") + glog.Errorf(pcb.Message(err.Error())) + return nil, err + } + var rh *requestHelper var scExternalID string var spExternalID string @@ -2108,13 +2116,6 @@ func (c *controller) prepareServiceInstanceLastOperationRequest(instance *v1beta } } - if instance.Status.InProgressProperties == nil { - pcb := pretty.NewInstanceContextBuilder(instance) - err := stderrors.New("Instance.Status.InProgressProperties can not be nil") - glog.Errorf(pcb.Message(err.Error())) - return nil, err - } - request := &osb.LastOperationRequest{ InstanceID: instance.Spec.ExternalID, ServiceID: &scExternalID, From c05ab181b95dbe714be6b5483bf5260291df41d5 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Tue, 19 Jun 2018 10:00:22 -0400 Subject: [PATCH 20/24] Add inprogress provision async failure ns test --- pkg/controller/controller_instance_ns_test.go | 68 +++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index b0062e5d3ef..7333ed26e55 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -320,6 +320,74 @@ func TestPollServiceInstanceSuccessProvisioningWithOperationNamespacedRefs(t *te assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testServicePlanName, testServicePlanGUID, instance) } +// TestPollServiceInstanceFailureProvisioningWithOperationNamespacedRefs tests +// polling an instance where provision was in process asynchronously but has an +// updated status of failed to provision. +func TestPollServiceInstanceFailureProvisioningWithOperationNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + PollLastOperationReaction: &fakeosb.PollLastOperationReaction{ + Response: &osb.LastOperationResponse{ + State: osb.StateFailed, + }, + }, + }) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceAsyncProvisioningWithNamespacedRefs(testOperation) + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + err = testController.pollServiceInstance(instance) + if err != nil { + t.Fatalf("pollServiceInstance failed: %s", err) + } + + if testController.instancePollingQueue.NumRequeues(instanceKey) == 0 { + t.Fatalf("Expected polling queue to have a record of test instance as provisioning should have retried") + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + operationKey := osb.OperationKey(testOperation) + assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ + InstanceID: testServiceInstanceGUID, + ServiceID: strPtr(testServiceClassGUID), + PlanID: strPtr(testServicePlanGUID), + OperationKey: &operationKey, + }) + + // verify no kube resources created. + // No actions + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceRequestFailingErrorStartOrphanMitigation( + t, + updatedServiceInstance, + v1beta1.ServiceInstanceOperationProvision, + startingInstanceOrphanMitigationReason, + "", + errorProvisionCallFailedReason, + instance, + ) +} + // TestReconcileServiceInstanceDeleteWithNamespacedRefs tests // deletingdeprovisioning an instance with namespaced refs func TestReconcileServiceInstanceDeleteWithNamespacedRefs(t *testing.T) { From 1e9139f14e167469d5ebc28436d82eb13c5d98d0 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Tue, 19 Jun 2018 10:11:47 -0400 Subject: [PATCH 21/24] Add inprogress depro poll ns test --- pkg/controller/controller_instance_ns_test.go | 87 +++++++++++++++++++ pkg/controller/controller_ns_test.go | 39 +++++++++ 2 files changed, 126 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index 7333ed26e55..f38c4b53032 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -556,6 +556,93 @@ func TestReconcileServiceInstanceDeleteAsynchronousWithNamespacedRefs(t *testing } } +// TestPollServiceInstanceInProgressDeprovisioningWithOperationNoFinalizerNamespacedRefs tests +// polling an instance that was asynchronously being deprovisioned and is still +// in progress. +func TestPollServiceInstanceInProgressDeprovisioningWithOperationNoFinalizerNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + cases := []struct { + name string + setup func(instance *v1beta1.ServiceInstance) + }{ + { + // simulates deprovision after user changed plan to non-existing plan + name: "nil plan", + setup: func(instance *v1beta1.ServiceInstance) { + instance.Spec.ServicePlanExternalName = "plan-that-does-not-exist" + instance.Spec.ServicePlanRef = nil + }, + }, + { + name: "With plan", + setup: func(instance *v1beta1.ServiceInstance) {}, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + PollLastOperationReaction: &fakeosb.PollLastOperationReaction{ + Response: &osb.LastOperationResponse{ + State: osb.StateInProgress, + Description: strPtr(lastOperationDescription), + }, + }, + }) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceAsyncDeprovisioningWithNamespacedRefs(testOperation) + tc.setup(instance) + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + err = testController.pollServiceInstance(instance) + if err != nil { + t.Fatalf("pollServiceInstance failed: %s", err) + } + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 1 { + t.Fatalf("Expected polling queue to have record of seeing test instance once") + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + operationKey := osb.OperationKey(testOperation) + assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ + InstanceID: testServiceInstanceGUID, + ServiceID: strPtr(testServiceClassGUID), + PlanID: strPtr(testServicePlanGUID), + OperationKey: &operationKey, + }) + + // there should have been 1 action to update the instance status with the last operation + // description + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceAsyncStillInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testOperation, testServicePlanName, testServicePlanGUID, instance) + assertServiceInstanceConditionHasLastOperationDescription(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, lastOperationDescription) + + // verify no kube resources created. + // No actions + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + }) + } +} + // TestResolveNamespacedReferences tests that resolveReferences works // correctly and resolves references when the references are of namespaced. func TestResolveNamespacedReferencesWorks(t *testing.T) { diff --git a/pkg/controller/controller_ns_test.go b/pkg/controller/controller_ns_test.go index 0229bff8953..926e152116c 100644 --- a/pkg/controller/controller_ns_test.go +++ b/pkg/controller/controller_ns_test.go @@ -145,3 +145,42 @@ func getTestServiceInstanceAsyncProvisioningWithNamespacedRefs(operation string) return instance } + +func getTestServiceInstanceAsyncDeprovisioningWithNamespacedRefs(operation string) *v1beta1.ServiceInstance { + instance := getTestServiceInstanceWithNamespacedRefs() + instance.Generation = 2 + + operationStartTime := metav1.NewTime(time.Now().Add(-1 * time.Hour)) + instance.Status = v1beta1.ServiceInstanceStatus{ + Conditions: []v1beta1.ServiceInstanceCondition{{ + Type: v1beta1.ServiceInstanceConditionReady, + Status: v1beta1.ConditionFalse, + Message: "Deprovisioning", + LastTransitionTime: metav1.NewTime(time.Now().Add(-5 * time.Minute)), + }}, + AsyncOpInProgress: true, + OperationStartTime: &operationStartTime, + CurrentOperation: v1beta1.ServiceInstanceOperationDeprovision, + InProgressProperties: &v1beta1.ServiceInstancePropertiesState{ + ServicePlanExternalName: testServicePlanName, + ServicePlanExternalID: testServicePlanGUID, + }, + + ReconciledGeneration: 1, + ObservedGeneration: 2, + ExternalProperties: &v1beta1.ServiceInstancePropertiesState{ + ServicePlanExternalName: testServicePlanName, + ServicePlanExternalID: testServicePlanGUID, + }, + ProvisionStatus: v1beta1.ServiceInstanceProvisionStatusProvisioned, + DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, + } + if operation != "" { + instance.Status.LastOperation = &operation + } + + // Set the deleted timestamp to simulate deletion + ts := metav1.NewTime(time.Now().Add(-5 * time.Minute)) + instance.DeletionTimestamp = &ts + return instance +} From 4af7d593a970f27b0810445298c366e89b2d345c Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Tue, 19 Jun 2018 10:14:41 -0400 Subject: [PATCH 22/24] Add polling success deprovision ns test --- pkg/controller/controller_instance_ns_test.go | 67 +++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index f38c4b53032..3ba1a01f841 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -643,6 +643,73 @@ func TestPollServiceInstanceInProgressDeprovisioningWithOperationNoFinalizerName } } +// TestPollServiceInstanceSuccessDeprovisioningWithOperationNoFinalizerNamespacedRefs tests +// polling an instance that was asynchronously being deprovisioned and its +// current poll status succeeded. Verify instance is deprovisioned. +func TestPollServiceInstanceSuccessDeprovisioningWithOperationNoFinalizerNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + PollLastOperationReaction: &fakeosb.PollLastOperationReaction{ + Response: &osb.LastOperationResponse{ + State: osb.StateSucceeded, + }, + }, + }) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceAsyncDeprovisioningWithNamespacedRefs(testOperation) + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + err = testController.pollServiceInstance(instance) + if err != nil { + t.Fatalf("pollServiceInstance failed: %s", err) + } + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance as polling should have completed") + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + operationKey := osb.OperationKey(testOperation) + assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ + InstanceID: testServiceInstanceGUID, + ServiceID: strPtr(testServiceClassGUID), + PlanID: strPtr(testServicePlanGUID), + OperationKey: &operationKey, + }) + + // verify no kube resources created. + // No actions + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceOperationSuccess(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testServicePlanName, testServicePlanGUID, instance) + + events := getRecordedEvents(testController) + + expectedEvent := normalEventBuilder(successDeprovisionReason).msg("The instance was deprovisioned successfully") + if err := checkEvents(events, expectedEvent.stringArr()); err != nil { + t.Fatal(err) + } +} + // TestResolveNamespacedReferences tests that resolveReferences works // correctly and resolves references when the references are of namespaced. func TestResolveNamespacedReferencesWorks(t *testing.T) { From ad1f04b85a2fdec44a887a63287707f6b99dbf13 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Tue, 19 Jun 2018 10:19:24 -0400 Subject: [PATCH 23/24] Add failed polling deprovision ns test --- pkg/controller/controller_instance_ns_test.go | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index 3ba1a01f841..0306c575b7b 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -710,6 +710,81 @@ func TestPollServiceInstanceSuccessDeprovisioningWithOperationNoFinalizerNamespa } } +// TestPollServiceInstanceFailureDeprovisioningNamespacedRefs tests polling an +// instance that has a async deprovision in progress where the broker responds +// with Failed. +func TestPollServiceInstanceFailureDeprovisioningNamespacedRefs(t *testing.T) { + err := utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=true", scfeatures.NamespacedServiceBroker)) + if err != nil { + t.Fatalf("Could not enable NamespacedServiceBroker feature flag.") + } + defer utilfeature.DefaultFeatureGate.Set(fmt.Sprintf("%v=false", scfeatures.NamespacedServiceBroker)) + + fakeKubeClient, fakeCatalogClient, fakeBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + PollLastOperationReaction: &fakeosb.PollLastOperationReaction{ + Response: &osb.LastOperationResponse{ + State: osb.StateFailed, + }, + }, + }) + + sharedInformers.ServiceBrokers().Informer().GetStore().Add(getTestServiceBroker()) + sharedInformers.ServiceClasses().Informer().GetStore().Add(getTestServiceClass()) + sharedInformers.ServicePlans().Informer().GetStore().Add(getTestServicePlan()) + + instance := getTestServiceInstanceAsyncDeprovisioningWithNamespacedRefs(testOperation) + instanceKey := testNamespace + "/" + testServiceInstanceName + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance") + } + + err = testController.pollServiceInstance(instance) + if err == nil { + t.Fatalf("Expected pollServiceInstance to return an error but there was none") + } + + if testController.instancePollingQueue.NumRequeues(instanceKey) != 0 { + t.Fatalf("Expected polling queue to not have any record of test instance as polling should have completed") + } + + brokerActions := fakeBrokerClient.Actions() + assertNumberOfBrokerActions(t, brokerActions, 1) + operationKey := osb.OperationKey(testOperation) + assertPollLastOperation(t, brokerActions[0], &osb.LastOperationRequest{ + InstanceID: testServiceInstanceGUID, + ServiceID: strPtr(testServiceClassGUID), + PlanID: strPtr(testServicePlanGUID), + OperationKey: &operationKey, + }) + + // verify no kube resources created. + // No actions + kubeActions := fakeKubeClient.Actions() + assertNumberOfActions(t, kubeActions, 0) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) + assertServiceInstanceRequestRetriableError( + t, + updatedServiceInstance, + v1beta1.ServiceInstanceOperationDeprovision, + errorDeprovisionCalledReason, + testServicePlanName, + testServicePlanGUID, + instance, + ) + + events := getRecordedEvents(testController) + + expectedEvent := warningEventBuilder(errorDeprovisionCalledReason).msg("Deprovision call failed: (no description provided)") + if err := checkEvents(events, expectedEvent.stringArr()); err != nil { + t.Fatal(err) + } +} + // TestResolveNamespacedReferences tests that resolveReferences works // correctly and resolves references when the references are of namespaced. func TestResolveNamespacedReferencesWorks(t *testing.T) { From d4faebeb3c0195e074944ded9a0bd134539d7039 Mon Sep 17 00:00:00 2001 From: Erik Nelson Date: Thu, 21 Jun 2018 11:28:53 -0400 Subject: [PATCH 24/24] Update new files with 2018 copyright header --- pkg/controller/controller_instance_ns_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/controller/controller_instance_ns_test.go b/pkg/controller/controller_instance_ns_test.go index 0306c575b7b..b4a1b05ee23 100644 --- a/pkg/controller/controller_instance_ns_test.go +++ b/pkg/controller/controller_instance_ns_test.go @@ -1,5 +1,5 @@ /* -Copyright 2017 The Kubernetes Authors. +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.