diff --git a/pkg/apis/servicecatalog/types.go b/pkg/apis/servicecatalog/types.go index b1139d3ab52..55e8b46754a 100644 --- a/pkg/apis/servicecatalog/types.go +++ b/pkg/apis/servicecatalog/types.go @@ -537,8 +537,15 @@ type ServiceInstanceStatus struct { // ReconciledGeneration is the 'Generation' of the serviceInstanceSpec that // was last processed by the controller. The reconciled generation is updated // even if the controller failed to process the spec. + // Deprecated: use ObservedGeneration with conditions set to true to find + // whether generation was reconciled. ReconciledGeneration int64 + // ObservedGeneration is the 'Generation' of the serviceInstanceSpec that + // was last processed by the controller. The observed generation is updated + // whenever the status is updated regardless of operation result. + ObservedGeneration int64 + // OperationStartTime is the time at which the current operation began. OperationStartTime *metav1.Time @@ -551,6 +558,9 @@ type ServiceInstanceStatus struct { // broker knows about. ExternalProperties *ServiceInstancePropertiesState + // ProvisionStatus describes whether the instance is in the provisioned state. + ProvisionStatus ServiceInstanceProvisionStatus + // DeprovisionStatus describes what has been done to deprovision the // ServiceInstance. DeprovisionStatus ServiceInstanceDeprovisionStatus @@ -653,6 +663,19 @@ const ( ServiceInstanceDeprovisionStatusFailed ServiceInstanceDeprovisionStatus = "Failed" ) +// ServiceInstanceProvisionStatus is the status of provisioning a +// ServiceInstance +type ServiceInstanceProvisionStatus string + +const ( + // ServiceInstanceProvisionStatusProvisioned indicates that the instance + // was provisioned. + ServiceInstanceProvisionStatusProvisioned ServiceInstanceProvisionStatus = "Provisioned" + // ServiceInstanceProvisionStatusNotProvisioned indicates that the instance + // was not ever provisioned or was deprovisioned. + ServiceInstanceProvisionStatusNotProvisioned ServiceInstanceProvisionStatus = "NotProvisioned" +) + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // ServiceBindingList is a list of ServiceBindings. diff --git a/pkg/apis/servicecatalog/v1beta1/types.go b/pkg/apis/servicecatalog/v1beta1/types.go index d8479559632..c8ecc038589 100644 --- a/pkg/apis/servicecatalog/v1beta1/types.go +++ b/pkg/apis/servicecatalog/v1beta1/types.go @@ -601,8 +601,15 @@ type ServiceInstanceStatus struct { // ReconciledGeneration is the 'Generation' of the serviceInstanceSpec that // was last processed by the controller. The reconciled generation is updated // even if the controller failed to process the spec. + // Deprecated: use ObservedGeneration with conditions set to true to find + // whether generation was reconciled. ReconciledGeneration int64 `json:"reconciledGeneration"` + // ObservedGeneration is the 'Generation' of the serviceInstanceSpec that + // was last processed by the controller. The observed generation is updated + // whenever the status is updated regardless of operation result. + ObservedGeneration int64 `json:"observedGeneration"` + // OperationStartTime is the time at which the current operation began. OperationStartTime *metav1.Time `json:"operationStartTime,omitempty"` @@ -615,6 +622,9 @@ type ServiceInstanceStatus struct { // broker knows about. ExternalProperties *ServiceInstancePropertiesState `json:"externalProperties,omitempty"` + // ProvisionStatus describes whether the instance is in the provisioned state. + ProvisionStatus ServiceInstanceProvisionStatus `json:"provisionStatus"` + // DeprovisionStatus describes what has been done to deprovision the // ServiceInstance. DeprovisionStatus ServiceInstanceDeprovisionStatus `json:"deprovisionStatus"` @@ -717,6 +727,19 @@ const ( ServiceInstanceDeprovisionStatusFailed ServiceInstanceDeprovisionStatus = "Failed" ) +// ServiceInstanceProvisionStatus is the status of provisioning a +// ServiceInstance +type ServiceInstanceProvisionStatus string + +const ( + // ServiceInstanceProvisionStatusProvisioned indicates that the instance + // was provisioned. + ServiceInstanceProvisionStatusProvisioned ServiceInstanceProvisionStatus = "Provisioned" + // ServiceInstanceProvisionStatusNotProvisioned indicates that the instance + // was not ever provisioned or was deprovisioned. + ServiceInstanceProvisionStatusNotProvisioned ServiceInstanceProvisionStatus = "NotProvisioned" +) + // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object // ServiceBindingList is a list of ServiceBindings. diff --git a/pkg/apis/servicecatalog/v1beta1/zz_generated.conversion.go b/pkg/apis/servicecatalog/v1beta1/zz_generated.conversion.go index 486046797c3..da0cc079f69 100644 --- a/pkg/apis/servicecatalog/v1beta1/zz_generated.conversion.go +++ b/pkg/apis/servicecatalog/v1beta1/zz_generated.conversion.go @@ -1005,9 +1005,11 @@ func autoConvert_v1beta1_ServiceInstanceStatus_To_servicecatalog_ServiceInstance out.DashboardURL = (*string)(unsafe.Pointer(in.DashboardURL)) out.CurrentOperation = servicecatalog.ServiceInstanceOperation(in.CurrentOperation) out.ReconciledGeneration = in.ReconciledGeneration + out.ObservedGeneration = in.ObservedGeneration out.OperationStartTime = (*v1.Time)(unsafe.Pointer(in.OperationStartTime)) out.InProgressProperties = (*servicecatalog.ServiceInstancePropertiesState)(unsafe.Pointer(in.InProgressProperties)) out.ExternalProperties = (*servicecatalog.ServiceInstancePropertiesState)(unsafe.Pointer(in.ExternalProperties)) + out.ProvisionStatus = servicecatalog.ServiceInstanceProvisionStatus(in.ProvisionStatus) out.DeprovisionStatus = servicecatalog.ServiceInstanceDeprovisionStatus(in.DeprovisionStatus) return nil } @@ -1025,9 +1027,11 @@ func autoConvert_servicecatalog_ServiceInstanceStatus_To_v1beta1_ServiceInstance out.DashboardURL = (*string)(unsafe.Pointer(in.DashboardURL)) out.CurrentOperation = ServiceInstanceOperation(in.CurrentOperation) out.ReconciledGeneration = in.ReconciledGeneration + out.ObservedGeneration = in.ObservedGeneration out.OperationStartTime = (*v1.Time)(unsafe.Pointer(in.OperationStartTime)) out.InProgressProperties = (*ServiceInstancePropertiesState)(unsafe.Pointer(in.InProgressProperties)) out.ExternalProperties = (*ServiceInstancePropertiesState)(unsafe.Pointer(in.ExternalProperties)) + out.ProvisionStatus = ServiceInstanceProvisionStatus(in.ProvisionStatus) out.DeprovisionStatus = ServiceInstanceDeprovisionStatus(in.DeprovisionStatus) return nil } diff --git a/pkg/controller/controller.go b/pkg/controller/controller.go index 99e4a5e65a5..3af636ea1b7 100644 --- a/pkg/controller/controller.go +++ b/pkg/controller/controller.go @@ -590,11 +590,11 @@ func convertClusterServicePlans(plans []osb.Plan, serviceClassID string) ([]*v1b return servicePlans, nil } -// isServiceInstanceReady returns whether the given instance has a ready condition +// isServiceInstanceConditionTrue returns whether the given instance has a given condition // with status true. -func isServiceInstanceReady(instance *v1beta1.ServiceInstance) bool { +func isServiceInstanceConditionTrue(instance *v1beta1.ServiceInstance, conditionType v1beta1.ServiceInstanceConditionType) bool { for _, cond := range instance.Status.Conditions { - if cond.Type == v1beta1.ServiceInstanceConditionReady { + if cond.Type == conditionType { return cond.Status == v1beta1.ConditionTrue } } @@ -602,6 +602,18 @@ func isServiceInstanceReady(instance *v1beta1.ServiceInstance) bool { return false } +// isServiceInstanceReady returns whether the given instance has a ready condition +// with status true. +func isServiceInstanceReady(instance *v1beta1.ServiceInstance) bool { + return isServiceInstanceConditionTrue(instance, v1beta1.ServiceInstanceConditionReady) +} + +// isServiceInstanceFailed returns whether the instance has a failed condition with +// status true. +func isServiceInstanceFailed(instance *v1beta1.ServiceInstance) bool { + return isServiceInstanceConditionTrue(instance, v1beta1.ServiceInstanceConditionFailed) +} + // NewClientConfigurationForBroker creates a new ClientConfiguration for connecting // to the specified Broker func NewClientConfigurationForBroker(broker *v1beta1.ClusterServiceBroker, authConfig *osb.AuthConfig) *osb.ClientConfiguration { diff --git a/pkg/controller/controller_instance.go b/pkg/controller/controller_instance.go index fc21cfe4543..0d6bfba678a 100644 --- a/pkg/controller/controller_instance.go +++ b/pkg/controller/controller_instance.go @@ -86,6 +86,8 @@ const ( deprovisioningInFlightMessage string = "Deprovision request for ServiceInstance in-flight to Broker" startingInstanceOrphanMitigationReason string = "StartingInstanceOrphanMitigation" startingInstanceOrphanMitigationMessage string = "The instance provision call failed with an ambiguous error; attempting to deprovision the instance in order to mitigate an orphaned resource" + observedNewGenerationReason string = "ObservedNewGeneration" + observedNewGenerationMessage string = "Observed a new generation of the instance" ) // ServiceInstance handlers and control-loop @@ -196,9 +198,9 @@ func getReconciliationActionForServiceInstance(instance *v1beta1.ServiceInstance return reconcilePoll case instance.ObjectMeta.DeletionTimestamp != nil || instance.Status.OrphanMitigationInProgress: return reconcileDelete - case instance.Status.ReconciledGeneration != 0: + case instance.Status.ProvisionStatus == v1beta1.ServiceInstanceProvisionStatusProvisioned: return reconcileUpdate - default: // instance.Status.ReconciledGeneration == 0 + default: // instance.Status.ProvisionStatus == "NotProvisioned" return reconcileAdd } } @@ -228,6 +230,16 @@ func (c *controller) reconcileServiceInstanceKey(key string) error { // error is returned to indicate that the instance has not been fully // processed and should be resubmitted at a later time. func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance) error { + handled, err := c.initObservedGeneration(instance) + if err != nil { + return err + } + if handled { + // Update the status and finish the iteration + // The updated instance will be automatically added back to the queue + // and be processed again + return nil + } reconciliationAction := getReconciliationActionForServiceInstance(instance) switch reconciliationAction { case reconcileAdd: @@ -244,6 +256,31 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance) } } +// initObservedGeneration implements ObservedGeneration initialization based on +// ReconciledGeneration for status API migration. +// Returns true if the status was updated (i.e. the iteration has finished and no +// more processing needed). +func (c *controller) initObservedGeneration(instance *v1beta1.ServiceInstance) (bool, error) { + if instance.Status.ObservedGeneration == 0 && instance.Status.ReconciledGeneration != 0 { + instance.Status.ObservedGeneration = instance.Status.ReconciledGeneration + // Before we implement https://github.com/kubernetes-incubator/service-catalog/issues/1715 + // and switch to non-terminal errors, the "Failed":"True" is a sign that the provisioning failed + provisioned := !isServiceInstanceFailed(instance) + if provisioned { + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned + } else { + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusNotProvisioned + } + + _, err := c.updateServiceInstanceStatus(instance) + if err != nil { + return false, err + } + return true, nil + } + return false, nil +} + // reconcileServiceInstanceAdd is responsible for handling the provisioning // of new service instances. func (c *controller) reconcileServiceInstanceAdd(instance *v1beta1.ServiceInstance) error { @@ -254,12 +291,16 @@ func (c *controller) reconcileServiceInstanceAdd(instance *v1beta1.ServiceInstan return nil } - if instance.Status.ReconciledGeneration == instance.Generation { - glog.V(4).Info(pcb.Message("Not processing event because reconciled generation showed there is no work to do")) + if isServiceInstanceProcessedAlready(instance) { + glog.V(4).Info(pcb.Message("Not processing event because status showed there is no work to do")) return nil } instance = instance.DeepCopy() + // Any status updates from this point should have an updated observed generation + if instance.Status.ObservedGeneration != instance.Generation { + c.prepareObservedGeneration(instance) + } // Update references to ClusterServicePlan / ClusterServiceClass if necessary. // @@ -358,12 +399,16 @@ func (c *controller) reconcileServiceInstanceUpdate(instance *v1beta1.ServiceIns return nil } - if instance.Status.ReconciledGeneration == instance.Generation { - glog.V(4).Info(pcb.Message("Not processing event because reconciled generation showed there is no work to do")) + if isServiceInstanceProcessedAlready(instance) { + glog.V(4).Info(pcb.Message("Not processing event because status showed there is no work to do")) return nil } instance = instance.DeepCopy() + // Any status updates from this point should have an updated observed generation + if instance.Status.ObservedGeneration != instance.Generation { + c.prepareObservedGeneration(instance) + } // Update references to ClusterServicePlan / ClusterServiceClass if necessary. // @@ -410,7 +455,8 @@ func (c *controller) reconcileServiceInstanceUpdate(instance *v1beta1.ServiceIns if httpErr, ok := osb.IsHTTPError(err); ok { msg := fmt.Sprintf("ClusterServiceBroker returned a failure for update call; update will not be retried: %v", httpErr) readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, errorUpdateInstanceCallFailedReason, msg) - return c.processUpdateServiceInstanceFailure(instance, readyCond) + failedCond := newServiceInstanceFailedCondition(v1beta1.ConditionTrue, errorUpdateInstanceCallFailedReason, msg) + return c.processUpdateServiceInstanceFailure(instance, readyCond, failedCond) } reason := errorErrorCallingUpdateInstanceReason @@ -418,7 +464,8 @@ func (c *controller) reconcileServiceInstanceUpdate(instance *v1beta1.ServiceIns if urlErr, ok := err.(*url.Error); ok && urlErr.Timeout() { msg := fmt.Sprintf("Communication with the ClusterServiceBroker timed out; update will not be retried: %v", urlErr) readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, reason, msg) - return c.processUpdateServiceInstanceFailure(instance, readyCond) + failedCond := newServiceInstanceFailedCondition(v1beta1.ConditionTrue, reason, msg) + return c.processUpdateServiceInstanceFailure(instance, readyCond, failedCond) } msg := fmt.Sprintf("The update call failed and will be retried: Error communicating with broker for updating: %s", err) @@ -431,7 +478,8 @@ func (c *controller) reconcileServiceInstanceUpdate(instance *v1beta1.ServiceIns msg = "Stopping reconciliation retries because too much time has elapsed" readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, errorReconciliationRetryTimeoutReason, msg) - return c.processUpdateServiceInstanceFailure(instance, readyCond) + failedCond := newServiceInstanceFailedCondition(v1beta1.ConditionTrue, errorReconciliationRetryTimeoutReason, msg) + return c.processUpdateServiceInstanceFailure(instance, readyCond, failedCond) } readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, reason, msg) @@ -468,6 +516,12 @@ func (c *controller) reconcileServiceInstanceDelete(instance *v1beta1.ServiceIns glog.V(4).Info(pcb.Message("Processing deleting event")) instance = instance.DeepCopy() + // Any status updates from this point should have an updated observed generation + // except for the orphan mitigation (it is considered to be a continuation + // of the previously failed provisioning operation). + if !instance.Status.OrphanMitigationInProgress && instance.Status.ObservedGeneration != instance.Generation { + c.prepareObservedGeneration(instance) + } // If the deprovisioning succeeded or is not needed, then no need to // make a request to the broker. @@ -684,9 +738,11 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro failedCond := newServiceInstanceFailedCondition(v1beta1.ConditionTrue, reason, message) err = c.processProvisionFailure(instance, readyCond, failedCond, false) default: - msg := "Update call failed: " + description - readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, errorUpdateInstanceCallFailedReason, msg) - err = c.processUpdateServiceInstanceFailure(instance, readyCond) + reason := errorUpdateInstanceCallFailedReason + message := "Update call failed: " + description + readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, reason, message) + failedCond := newServiceInstanceFailedCondition(v1beta1.ConditionTrue, reason, message) + err = c.processUpdateServiceInstanceFailure(instance, readyCond, failedCond) } if err != nil { return c.handleServiceInstancePollingError(instance, err) @@ -704,16 +760,13 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro } } -// isServiceInstanceFailed returns whether the instance has a failed condition with -// status true. -func isServiceInstanceFailed(instance *v1beta1.ServiceInstance) bool { - for _, condition := range instance.Status.Conditions { - if condition.Type == v1beta1.ServiceInstanceConditionFailed && condition.Status == v1beta1.ConditionTrue { - return true - } - } - - return false +// isServiceInstanceProcessedAlready returns true if there is no further processing +// needed for the instance based on ObservedGeneration +func isServiceInstanceProcessedAlready(instance *v1beta1.ServiceInstance) bool { + // The observed generation is considered to be reconciled if either of the + // conditions is set to true and there is no orphan mitigation in progress + return instance.Status.ObservedGeneration >= instance.Generation && + (isServiceInstanceReady(instance) || isServiceInstanceFailed(instance)) && !instance.Status.OrphanMitigationInProgress } // processServiceInstancePollingFailureRetryTimeout marks the instance as having @@ -736,7 +789,7 @@ func (c *controller) processServiceInstancePollingFailureRetryTimeout(instance * return c.processProvisionFailure(instance, readyCond, failedCond, true) default: readyCond := newServiceInstanceReadyCondition(v1beta1.ConditionFalse, errorReconciliationRetryTimeoutReason, msg) - err = c.processUpdateServiceInstanceFailure(instance, readyCond) + err = c.processUpdateServiceInstanceFailure(instance, readyCond, failedCond) } if err != nil { return c.handleServiceInstancePollingError(instance, err) @@ -1027,10 +1080,14 @@ func setServiceInstanceConditionInternal(toUpdate *v1beta1.ServiceInstance, func (c *controller) updateServiceInstanceReferences(toUpdate *v1beta1.ServiceInstance) (*v1beta1.ServiceInstance, error) { pcb := pretty.NewContextBuilder(pretty.ServiceInstance, toUpdate.Namespace, toUpdate.Name) glog.V(4).Info(pcb.Message("Updating references")) + status := toUpdate.Status updatedInstance, err := c.serviceCatalogClient.ServiceInstances(toUpdate.Namespace).UpdateReferences(toUpdate) if err != nil { glog.Errorf(pcb.Messagef("Failed to update references: %v", err)) } + // The UpdateReferences method ignores status changes. + // Restore status that might have changed locally to be able to update it later. + updatedInstance.Status = status return updatedInstance, err } @@ -1071,6 +1128,30 @@ func (c *controller) updateServiceInstanceCondition( return err } +// prepareObservedGeneration sets the instance's observed generation +// and clears the conditions, preparing it for any status updates that can occur +// during the further processing. +// It doesn't send the update request to server. +func (c *controller) prepareObservedGeneration(toUpdate *v1beta1.ServiceInstance) { + toUpdate.Status.ObservedGeneration = toUpdate.Generation + reason := observedNewGenerationReason + message := observedNewGenerationMessage + setServiceInstanceCondition( + toUpdate, + v1beta1.ServiceInstanceConditionReady, + v1beta1.ConditionFalse, + reason, + message, + ) + setServiceInstanceCondition( + toUpdate, + v1beta1.ServiceInstanceConditionFailed, + v1beta1.ConditionFalse, + reason, + message, + ) +} + // recordStartOfServiceInstanceOperation updates the instance to indicate that // there is an operation being performed. If the instance was already // performing a different operation, that operation is replaced. The Status of @@ -1083,10 +1164,7 @@ func (c *controller) updateServiceInstanceCondition( // if there was an error // 2 - any error that occurred func (c *controller) recordStartOfServiceInstanceOperation(toUpdate *v1beta1.ServiceInstance, operation v1beta1.ServiceInstanceOperation, inProgressProperties *v1beta1.ServiceInstancePropertiesState) (*v1beta1.ServiceInstance, error) { - currentReconciledGeneration := toUpdate.Status.ReconciledGeneration clearServiceInstanceCurrentOperation(toUpdate) - - toUpdate.Status.ReconciledGeneration = currentReconciledGeneration toUpdate.Status.CurrentOperation = operation now := metav1.Now() toUpdate.Status.OperationStartTime = &now @@ -1128,10 +1206,7 @@ func (c *controller) checkForRemovedClassAndPlan(instance *v1beta1.ServiceInstan return nil } - isProvisioning := false - if instance.Status.ReconciledGeneration == 0 { - isProvisioning = true - } + isProvisioning := instance.Status.ProvisionStatus != v1beta1.ServiceInstanceProvisionStatusProvisioned // Regardless of what's been deleted, you can always update // parameters (ie, not change plans) @@ -1166,18 +1241,6 @@ func clearServiceInstanceCurrentOperation(toUpdate *v1beta1.ServiceInstance) { toUpdate.Status.OrphanMitigationInProgress = false toUpdate.Status.LastOperation = nil toUpdate.Status.InProgressProperties = nil - toUpdate.Status.ReconciledGeneration = toUpdate.Generation -} - -// rollbackReconciledGenerationOnDeletion resets the ReconciledGeneration if a -// deletion was performed while an async provision or update is running. -// TODO: rework saving off current generation as the start of the async -// operation, see PR 1708/Issue 1587. -func rollbackReconciledGenerationOnDeletion(instance *v1beta1.ServiceInstance, currentReconciledGeneration int64) { - if instance.DeletionTimestamp != nil { - glog.V(4).Infof("Not updating ReconciledGeneration after async operation because there is a deletion pending.") - instance.Status.ReconciledGeneration = currentReconciledGeneration - } } // serviceInstanceHasExistingBindings returns true if there are any existing @@ -1445,9 +1508,9 @@ func (c *controller) processProvisionSuccess(instance *v1beta1.ServiceInstance, setServiceInstanceDashboardURL(instance, dashboardURL) setServiceInstanceCondition(instance, v1beta1.ServiceInstanceConditionReady, v1beta1.ConditionTrue, successProvisionReason, successProvisionMessage) instance.Status.ExternalProperties = instance.Status.InProgressProperties - currentReconciledGeneration := instance.Status.ReconciledGeneration clearServiceInstanceCurrentOperation(instance) - rollbackReconciledGenerationOnDeletion(instance, currentReconciledGeneration) + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned + instance.Status.ReconciledGeneration = instance.Status.ObservedGeneration if _, err := c.updateServiceInstanceStatus(instance); err != nil { return err @@ -1460,7 +1523,6 @@ func (c *controller) processProvisionSuccess(instance *v1beta1.ServiceInstance, // processProvisionFailure handles the logging and updating of a // ServiceInstance that hit a terminal failure during provision reconciliation. func (c *controller) processProvisionFailure(instance *v1beta1.ServiceInstance, readyCond, failedCond *v1beta1.ServiceInstanceCondition, shouldMitigateOrphan bool) error { - currentReconciledGeneration := instance.Status.ReconciledGeneration if failedCond == nil { return fmt.Errorf("failedCond must not be nil") } @@ -1488,7 +1550,7 @@ func (c *controller) processProvisionFailure(instance *v1beta1.ServiceInstance, err = fmt.Errorf(failedCond.Message) } else { clearServiceInstanceCurrentOperation(instance) - rollbackReconciledGenerationOnDeletion(instance, currentReconciledGeneration) + instance.Status.ReconciledGeneration = instance.Status.ObservedGeneration } if _, err := c.updateServiceInstanceStatus(instance); err != nil { @@ -1520,9 +1582,8 @@ func (c *controller) processProvisionAsyncResponse(instance *v1beta1.ServiceInst func (c *controller) processUpdateServiceInstanceSuccess(instance *v1beta1.ServiceInstance) error { setServiceInstanceCondition(instance, v1beta1.ServiceInstanceConditionReady, v1beta1.ConditionTrue, successUpdateInstanceReason, successUpdateInstanceMessage) instance.Status.ExternalProperties = instance.Status.InProgressProperties - currentReconciledGeneration := instance.Status.ReconciledGeneration clearServiceInstanceCurrentOperation(instance) - rollbackReconciledGenerationOnDeletion(instance, currentReconciledGeneration) + instance.Status.ReconciledGeneration = instance.Status.ObservedGeneration if _, err := c.updateServiceInstanceStatus(instance); err != nil { return err @@ -1534,13 +1595,13 @@ func (c *controller) processUpdateServiceInstanceSuccess(instance *v1beta1.Servi // processUpdateServiceInstanceFailure handles the logging and updating of a // ServiceInstance that hit a terminal failure during update reconciliation. -func (c *controller) processUpdateServiceInstanceFailure(instance *v1beta1.ServiceInstance, readyCond *v1beta1.ServiceInstanceCondition) error { +func (c *controller) processUpdateServiceInstanceFailure(instance *v1beta1.ServiceInstance, readyCond, failedCond *v1beta1.ServiceInstanceCondition) error { c.recorder.Event(instance, corev1.EventTypeWarning, readyCond.Reason, readyCond.Message) - currentReconciledGeneration := instance.Status.ReconciledGeneration setServiceInstanceCondition(instance, v1beta1.ServiceInstanceConditionReady, readyCond.Status, readyCond.Reason, readyCond.Message) + setServiceInstanceCondition(instance, v1beta1.ServiceInstanceConditionFailed, failedCond.Status, failedCond.Reason, failedCond.Message) clearServiceInstanceCurrentOperation(instance) - rollbackReconciledGenerationOnDeletion(instance, currentReconciledGeneration) + instance.Status.ReconciledGeneration = instance.Status.ObservedGeneration if _, err := c.updateServiceInstanceStatus(instance); err != nil { return err @@ -1580,7 +1641,9 @@ func (c *controller) processDeprovisionSuccess(instance *v1beta1.ServiceInstance setServiceInstanceCondition(instance, v1beta1.ServiceInstanceConditionReady, v1beta1.ConditionFalse, reason, msg) clearServiceInstanceCurrentOperation(instance) instance.Status.ExternalProperties = nil + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusNotProvisioned instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusSucceeded + instance.Status.ReconciledGeneration = instance.Status.ObservedGeneration if mitigatingOrphan { if _, err := c.updateServiceInstanceStatus(instance); err != nil { @@ -1624,6 +1687,7 @@ func (c *controller) processDeprovisionFailure(instance *v1beta1.ServiceInstance } clearServiceInstanceCurrentOperation(instance) + instance.Status.ReconciledGeneration = instance.Status.ObservedGeneration instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusFailed if _, err := c.updateServiceInstanceStatus(instance); err != nil { diff --git a/pkg/controller/controller_instance_test.go b/pkg/controller/controller_instance_test.go index caa6a01bce0..56c9a821671 100644 --- a/pkg/controller/controller_instance_test.go +++ b/pkg/controller/controller_instance_test.go @@ -1345,7 +1345,7 @@ func TestReconcileServiceInstanceAsynchronous(t *testing.T) { assertServiceInstanceOperationInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testClusterServicePlanName, testClusterServicePlanGUID, instance) updatedServiceInstance = assertUpdateStatus(t, actions[1], instance) - assertServiceInstanceAsyncInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) // verify no kube resources created. @@ -1412,7 +1412,7 @@ func TestReconcileServiceInstanceAsynchronousNoOperation(t *testing.T) { assertServiceInstanceOperationInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testClusterServicePlanName, testClusterServicePlanGUID, instance) updatedServiceInstance = assertUpdateStatus(t, actions[1], instance) - assertServiceInstanceAsyncInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, "", testClusterServicePlanName, testClusterServicePlanGUID, instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, "", testClusterServicePlanName, testClusterServicePlanGUID, instance) assertServiceInstanceDashboardURL(t, updatedServiceInstance, testDashboardURL) // verify no kube resources created. @@ -1494,6 +1494,8 @@ func TestReconcileServiceInstanceDelete(t *testing.T) { // 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{ ClusterServicePlanExternalName: testClusterServicePlanName, ClusterServicePlanExternalID: testClusterServicePlanGUID, @@ -1563,6 +1565,8 @@ func TestReconcileServiceInstanceDeleteBlockedByCredentials(t *testing.T) { // 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{ ClusterServicePlanExternalName: testClusterServicePlanName, ClusterServicePlanExternalID: testClusterServicePlanGUID, @@ -1668,6 +1672,8 @@ func TestReconcileServiceInstanceDeleteAsynchronous(t *testing.T) { // 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{ ClusterServicePlanExternalName: testClusterServicePlanName, ClusterServicePlanExternalID: testClusterServicePlanGUID, @@ -1715,7 +1721,7 @@ func TestReconcileServiceInstanceDeleteAsynchronous(t *testing.T) { assertServiceInstanceOperationInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testClusterServicePlanName, testClusterServicePlanGUID, instance) updatedServiceInstance = assertUpdateStatus(t, actions[1], instance) - assertServiceInstanceAsyncInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) events := getRecordedEvents(testController) @@ -1747,6 +1753,8 @@ func TestReconcileServiceInstanceDeleteFailedProvisionWithRequest(t *testing.T) instance.Generation = 2 instance.Status.ReconciledGeneration = 1 + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusNotProvisioned fakeCatalogClient.AddReactor("get", "serviceinstances", func(action clientgotesting.Action) (bool, runtime.Object, error) { return true, instance, nil @@ -1791,44 +1799,44 @@ func TestReconcileServiceInstanceDeleteFailedProvisionWithRequest(t *testing.T) // an instance will be finalized under various conditions defined by test parameters func TestReconsileServiceInstanceDeleteWithParameters(t *testing.T) { cases := []struct { - name string - externalProperties *v1beta1.ServiceInstancePropertiesState - deprovisionStatus v1beta1.ServiceInstanceDeprovisionStatus - serviceBinding *v1beta1.ServiceBinding - generation int64 - reconceiledGeneration int64 + name string + externalProperties *v1beta1.ServiceInstancePropertiesState + deprovisionStatus v1beta1.ServiceInstanceDeprovisionStatus + serviceBinding *v1beta1.ServiceBinding + generation int64 + reconciledGeneration int64 }{ { - name: "With a failed to provision instance and without making a provision request", - externalProperties: &v1beta1.ServiceInstancePropertiesState{}, - deprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusNotRequired, - serviceBinding: nil, - generation: 1, - reconceiledGeneration: 0, + name: "With a failed to provision instance and without making a provision request", + externalProperties: &v1beta1.ServiceInstancePropertiesState{}, + deprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusNotRequired, + serviceBinding: nil, + generation: 1, + reconciledGeneration: 0, }, { - name: "With a failed to provision instance, with inactive binding, and without making a provision request", - externalProperties: &v1beta1.ServiceInstancePropertiesState{}, - deprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusNotRequired, - serviceBinding: getTestServiceInactiveBinding(), - generation: 1, - reconceiledGeneration: 0, + name: "With a failed to provision instance, with inactive binding, and without making a provision request", + externalProperties: &v1beta1.ServiceInstancePropertiesState{}, + deprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusNotRequired, + serviceBinding: getTestServiceInactiveBinding(), + generation: 1, + reconciledGeneration: 0, }, { - name: "With a deprovisioned instance and without making a deprovision request", - externalProperties: nil, - deprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusSucceeded, - serviceBinding: nil, - generation: 2, - reconceiledGeneration: 1, + name: "With a deprovisioned instance and without making a deprovision request", + externalProperties: nil, + deprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusSucceeded, + serviceBinding: nil, + generation: 2, + reconciledGeneration: 1, }, { - name: "With a deprovisioned instance, with inactive binding, and without making a deprovision request", - externalProperties: nil, - deprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusSucceeded, - serviceBinding: getTestServiceInactiveBinding(), - generation: 2, - reconceiledGeneration: 1, + name: "With a deprovisioned instance, with inactive binding, and without making a deprovision request", + externalProperties: nil, + deprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusSucceeded, + serviceBinding: getTestServiceInactiveBinding(), + generation: 2, + reconciledGeneration: 1, }, } @@ -1850,7 +1858,9 @@ func TestReconsileServiceInstanceDeleteWithParameters(t *testing.T) { instance.Status.DeprovisionStatus = tc.deprovisionStatus instance.Generation = tc.generation - instance.Status.ReconciledGeneration = tc.reconceiledGeneration + instance.Status.ReconciledGeneration = tc.reconciledGeneration + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusNotProvisioned fakeCatalogClient.AddReactor("get", "serviceinstances", func(action clientgotesting.Action) (bool, runtime.Object, error) { return true, instance, nil @@ -1899,6 +1909,8 @@ func TestReconcileServiceInstanceDeleteWhenAlreadyDeprovisionedUnsuccessfully(t instance.Generation = 2 instance.Status.ReconciledGeneration = 1 + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned fakeCatalogClient.AddReactor("get", "serviceinstances", func(action clientgotesting.Action) (bool, runtime.Object, error) { return true, instance, nil @@ -1947,6 +1959,8 @@ func TestReconcileServiceInstanceDeleteFailedUpdate(t *testing.T) { } instance.Generation = 2 instance.Status.ReconciledGeneration = 2 + instance.Status.ObservedGeneration = 2 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired fakeCatalogClient.AddReactor("get", "serviceinstances", func(action clientgotesting.Action) (bool, runtime.Object, error) { @@ -2006,6 +2020,8 @@ func TestReconcileServiceInstanceDeleteDoesNotInvokeClusterServiceBroker(t *test instance.ObjectMeta.Finalizers = []string{v1beta1.FinalizerServiceCatalog} instance.Generation = 1 instance.Status.ReconciledGeneration = 0 + instance.Status.ObservedGeneration = 0 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusNotProvisioned instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusNotRequired fakeCatalogClient.AddReactor("get", "serviceinstances", func(action clientgotesting.Action) (bool, runtime.Object, error) { @@ -2118,7 +2134,7 @@ func TestPollServiceInstanceInProgressProvisioningWithOperation(t *testing.T) { assertNumberOfActions(t, actions, 1) updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) - assertServiceInstanceAsyncInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) assertServiceInstanceConditionHasLastOperationDescription(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationProvision, lastOperationDescription) // verify no kube resources created. @@ -2292,7 +2308,7 @@ func TestPollServiceInstanceInProgressDeprovisioningWithOperationNoFinalizer(t * assertNumberOfActions(t, actions, 1) updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) - assertServiceInstanceAsyncInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) + assertServiceInstanceAsyncStillInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) assertServiceInstanceConditionHasLastOperationDescription(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationDeprovision, lastOperationDescription) // verify no kube resources created. @@ -2711,6 +2727,7 @@ func TestReconcileServiceInstanceSuccessOnFinalRetry(t *testing.T) { ClusterServicePlanExternalName: testClusterServicePlanName, ClusterServicePlanExternalID: testClusterServicePlanGUID, } + instance.Status.ObservedGeneration = instance.Generation startTime := metav1.NewTime(time.Now().Add(-7 * 24 * time.Hour)) instance.Status.OperationStartTime = &startTime @@ -2772,6 +2789,7 @@ func TestReconcileServiceInstanceFailureOnFinalRetry(t *testing.T) { instance.Status.CurrentOperation = v1beta1.ServiceInstanceOperationProvision startTime := metav1.NewTime(time.Now().Add(-7 * 24 * time.Hour)) instance.Status.OperationStartTime = &startTime + instance.Status.ObservedGeneration = instance.Generation if err := testController.reconcileServiceInstance(instance); err != nil { t.Fatalf("Should have returned no error because the retry duration has elapsed: %v", err) @@ -3366,6 +3384,8 @@ func TestReconcileInstanceDeleteUsingOriginatingIdentity(t *testing.T) { // ReconciledGeneration 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.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired if tc.includeUserInfo { instance.Spec.UserInfo = testUserInfo @@ -4028,6 +4048,8 @@ func TestReconcileServiceInstanceUpdateParameters(t *testing.T) { instance := getTestServiceInstanceWithRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired oldParameters := map[string]interface{}{ @@ -4155,6 +4177,8 @@ func TestReconcileServiceInstanceDeleteParameters(t *testing.T) { instance := getTestServiceInstanceWithRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired oldParameters := map[string]interface{}{ @@ -4328,6 +4352,8 @@ func TestReconcileServiceInstanceUpdatePlan(t *testing.T) { instance := getTestServiceInstanceWithRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired oldParameters := map[string]interface{}{ @@ -4535,7 +4561,7 @@ func TestReconcileServiceInstanceWithUpdateFailure(t *testing.T) { assertServiceInstanceOperationInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, testClusterServicePlanName, testClusterServicePlanGUID, instance) updatedServiceInstance = assertUpdateStatus(t, actions[1], instance) - assertServiceInstanceRequestFailingErrorNoOrphanMitigation(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, errorUpdateInstanceCallFailedReason, "ClusterServiceBrokerReturnedFailure", instance) + assertServiceInstanceRequestFailingErrorNoOrphanMitigation(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, errorUpdateInstanceCallFailedReason, errorUpdateInstanceCallFailedReason, instance) events := getRecordedEvents(testController) @@ -4766,6 +4792,8 @@ func TestReconcileServiceInstanceUpdateAsynchronous(t *testing.T) { instance := getTestServiceInstanceWithRefs() instance.Generation = 2 instance.Status.ReconciledGeneration = 1 + instance.Status.ObservedGeneration = 1 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired instance.Status.ExternalProperties = &v1beta1.ServiceInstancePropertiesState{ @@ -4803,7 +4831,7 @@ func TestReconcileServiceInstanceUpdateAsynchronous(t *testing.T) { assertServiceInstanceOperationInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, testClusterServicePlanName, testClusterServicePlanGUID, instance) updatedServiceInstance = assertUpdateStatus(t, actions[1], instance) - assertServiceInstanceAsyncInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) + assertServiceInstanceAsyncStartInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) // verify no kube resources created. // One single action comes from getting namespace uid @@ -4865,7 +4893,7 @@ func TestPollServiceInstanceAsyncInProgressUpdating(t *testing.T) { assertNumberOfActions(t, actions, 1) updatedServiceInstance := assertUpdateStatus(t, actions[0], instance) - assertServiceInstanceAsyncInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) + assertServiceInstanceAsyncStillInProgress(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, testOperation, testClusterServicePlanName, testClusterServicePlanGUID, instance) assertServiceInstanceConditionHasLastOperationDescription(t, updatedServiceInstance, v1beta1.ServiceInstanceOperationUpdate, lastOperationDescription) // verify no kube resources created. @@ -5233,6 +5261,8 @@ func TestReconcileServiceInstanceDeleteWithNonExistentPlan(t *testing.T) { // 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{ ClusterServicePlanExternalName: "old-plan-name", ClusterServicePlanExternalID: "old-plan-id", @@ -5278,3 +5308,49 @@ func TestReconcileServiceInstanceDeleteWithNonExistentPlan(t *testing.T) { t.Fatal(err) } } + +// TestReconcileServiceInstanceUpdateMissingObservedGeneration tests reconciling a +// ServiceInstance with ObservedGeneration missing (while Reconciled Generation set) +// i.e. API version migration testing +func TestReconcileServiceInstanceUpdateMissingObservedGeneration(t *testing.T) { + _, fakeCatalogClient, fakeClusterServiceBrokerClient, testController, sharedInformers := newTestController(t, fakeosb.FakeClientConfiguration{ + UpdateInstanceReaction: &fakeosb.UpdateInstanceReaction{ + Response: &osb.UpdateInstanceResponse{}, + }, + }) + + sharedInformers.ClusterServiceBrokers().Informer().GetStore().Add(getTestClusterServiceBroker()) + sharedInformers.ClusterServiceClasses().Informer().GetStore().Add(getTestClusterServiceClass()) + sharedInformers.ClusterServicePlans().Informer().GetStore().Add(getTestClusterServicePlan()) + + instance := getTestServiceInstanceWithRefs() + instance.Generation = 2 + instance.Status.ReconciledGeneration = 1 + // Missing ObservedGeneration and Provisioned after updating Service Catalog + instance.Status.ObservedGeneration = 0 + instance.Status.ProvisionStatus = v1beta1.ServiceInstanceProvisionStatusNotProvisioned + instance.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired + + instance.Status.ExternalProperties = &v1beta1.ServiceInstancePropertiesState{ + ClusterServicePlanExternalName: testClusterServicePlanName, + ClusterServicePlanExternalID: testClusterServicePlanGUID, + } + + if err := testController.reconcileServiceInstance(instance); err != nil { + t.Fatalf("This should not fail : %v", err) + } + + brokerActions := fakeClusterServiceBrokerClient.Actions() + assertNumberOfClusterServiceBrokerActions(t, brokerActions, 0) + + actions := fakeCatalogClient.Actions() + assertNumberOfActions(t, actions, 1) + + updatedServiceInstance := assertUpdateStatus(t, actions[0], instance).(*v1beta1.ServiceInstance) + if updatedServiceInstance.Status.ObservedGeneration == 0 || updatedServiceInstance.Status.ObservedGeneration != instance.Status.ReconciledGeneration { + t.Fatalf("Unexpected ObservedGeneration value: %d", updatedServiceInstance.Status.ObservedGeneration) + } + if instance.Status.ProvisionStatus != v1beta1.ServiceInstanceProvisionStatusProvisioned { + t.Fatalf("The instance was expected to be marked as Provisioned") + } +} diff --git a/pkg/controller/controller_test.go b/pkg/controller/controller_test.go index 38a788b74b2..afb287fd965 100644 --- a/pkg/controller/controller_test.go +++ b/pkg/controller/controller_test.go @@ -687,6 +687,8 @@ func getTestServiceInstanceUpdatingPlan() *v1beta1.ServiceInstance { }, // It's been provisioned successfully. ReconciledGeneration: 1, + ObservedGeneration: 1, + ProvisionStatus: v1beta1.ServiceInstanceProvisionStatusProvisioned, DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, } @@ -707,6 +709,8 @@ func getTestServiceInstanceUpdatingParameters() *v1beta1.ServiceInstance { }, // It's been provisioned successfully. ReconciledGeneration: 1, + ObservedGeneration: 1, + ProvisionStatus: v1beta1.ServiceInstanceProvisionStatusProvisioned, DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, } @@ -727,6 +731,8 @@ func getTestServiceInstanceUpdatingParametersOfDeletedPlan() *v1beta1.ServiceIns }, // It's been provisioned successfully. ReconciledGeneration: 1, + ObservedGeneration: 1, + ProvisionStatus: v1beta1.ServiceInstanceProvisionStatusProvisioned, DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, } @@ -752,7 +758,8 @@ func getTestServiceInstanceAsyncProvisioning(operation string) *v1beta1.ServiceI ClusterServicePlanExternalName: testClusterServicePlanName, ClusterServicePlanExternalID: testClusterServicePlanGUID, }, - DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, + ObservedGeneration: instance.Generation, + DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, } if operation != "" { instance.Status.LastOperation = &operation @@ -770,6 +777,7 @@ func getTestServiceInstanceAsyncUpdating(operation string) *v1beta1.ServiceInsta operationStartTime := metav1.NewTime(time.Now().Add(-1 * time.Hour)) instance.Status = v1beta1.ServiceInstanceStatus{ ReconciledGeneration: 1, + ObservedGeneration: 2, Conditions: []v1beta1.ServiceInstanceCondition{{ Type: v1beta1.ServiceInstanceConditionReady, Status: v1beta1.ConditionFalse, @@ -787,6 +795,7 @@ func getTestServiceInstanceAsyncUpdating(operation string) *v1beta1.ServiceInsta ClusterServicePlanExternalName: "old-plan-name", ClusterServicePlanExternalID: "old-plan-id", }, + ProvisionStatus: v1beta1.ServiceInstanceProvisionStatusProvisioned, DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, } if operation != "" { @@ -812,10 +821,12 @@ func getTestServiceInstanceAsyncDeprovisioning(operation string) *v1beta1.Servic OperationStartTime: &operationStartTime, CurrentOperation: v1beta1.ServiceInstanceOperationDeprovision, ReconciledGeneration: 1, + ObservedGeneration: 2, ExternalProperties: &v1beta1.ServiceInstancePropertiesState{ ClusterServicePlanExternalName: testClusterServicePlanName, ClusterServicePlanExternalID: testClusterServicePlanGUID, }, + ProvisionStatus: v1beta1.ServiceInstanceProvisionStatusProvisioned, DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusRequired, } if operation != "" { @@ -1927,6 +1938,28 @@ func assertServiceInstanceReconciledGeneration(t *testing.T, obj runtime.Object, } } +func assertServiceInstanceObservedGeneration(t *testing.T, obj runtime.Object, observedGeneration int64) { + instance, ok := obj.(*v1beta1.ServiceInstance) + if !ok { + fatalf(t, "Couldn't convert object %+v into a *v1beta1.ServiceInstance", obj) + } + + if e, a := observedGeneration, instance.Status.ObservedGeneration; e != a { + fatalf(t, "unexpected observed generation: expected %v, got %v", e, a) + } +} + +func assertServiceInstanceProvisioned(t *testing.T, obj runtime.Object, provisionStatus v1beta1.ServiceInstanceProvisionStatus) { + instance, ok := obj.(*v1beta1.ServiceInstance) + if !ok { + fatalf(t, "Couldn't convert object %+v into a *v1beta1.ServiceInstance", obj) + } + + if e, a := provisionStatus, instance.Status.ProvisionStatus; e != a { + fatalf(t, "unexpected provision status: expected %v, got %v", e, a) + } +} + func assertServiceInstanceOperationStartTimeSet(t *testing.T, obj runtime.Object, isOperationStartTimeSet bool) { instance, ok := obj.(*v1beta1.ServiceInstance) if !ok { @@ -2039,6 +2072,8 @@ func assertServiceInstanceErrorBeforeRequest(t *testing.T, obj runtime.Object, r assertServiceInstanceCurrentOperationClear(t, obj) assertServiceInstanceOperationStartTimeSet(t, obj, false) assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Status.ReconciledGeneration) + assertServiceInstanceObservedGeneration(t, obj, originalInstance.Generation) + assertServiceInstanceProvisioned(t, obj, originalInstance.Status.ProvisionStatus) assertAsyncOpInProgressFalse(t, obj) assertServiceInstanceOrphanMitigationInProgressFalse(t, obj) assertServiceInstanceInProgressPropertiesNil(t, obj) @@ -2052,18 +2087,28 @@ func assertServiceInstanceOperationInProgress(t *testing.T, obj runtime.Object, func assertServiceInstanceOperationInProgressWithParameters(t *testing.T, obj runtime.Object, operation v1beta1.ServiceInstanceOperation, planName, planID string, inProgressParameters map[string]interface{}, inProgressParametersChecksum string, originalInstance *v1beta1.ServiceInstance) { reason := "" + var expectedObservedGeneration int64 switch operation { case v1beta1.ServiceInstanceOperationProvision: reason = provisioningInFlightReason + expectedObservedGeneration = originalInstance.Generation case v1beta1.ServiceInstanceOperationUpdate: reason = instanceUpdatingInFlightReason + expectedObservedGeneration = originalInstance.Generation case v1beta1.ServiceInstanceOperationDeprovision: reason = deprovisioningInFlightReason + if originalInstance.Status.OrphanMitigationInProgress { + expectedObservedGeneration = originalInstance.Status.ObservedGeneration + } else { + expectedObservedGeneration = originalInstance.Generation + } } assertServiceInstanceReadyFalse(t, obj, reason) assertServiceInstanceCurrentOperation(t, obj, operation) assertServiceInstanceOperationStartTimeSet(t, obj, true) assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Status.ReconciledGeneration) + assertServiceInstanceObservedGeneration(t, obj, expectedObservedGeneration) + assertServiceInstanceProvisioned(t, obj, originalInstance.Status.ProvisionStatus) assertAsyncOpInProgressFalse(t, obj) assertServiceInstanceOrphanMitigationInProgressFalse(t, obj) switch operation { @@ -2082,6 +2127,8 @@ func assertServiceInstanceStartingOrphanMitigation(t *testing.T, obj runtime.Obj assertServiceInstanceReadyFalse(t, obj, startingInstanceOrphanMitigationReason) assertServiceInstanceOperationStartTimeSet(t, obj, false) assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Status.ReconciledGeneration) + assertServiceInstanceObservedGeneration(t, obj, originalInstance.Generation) + assertServiceInstanceProvisioned(t, obj, originalInstance.Status.ProvisionStatus) assertServiceInstanceOrphanMitigationInProgressTrue(t, obj) assertServiceInstanceDeprovisionStatus(t, obj, v1beta1.ServiceInstanceDeprovisionStatusRequired) } @@ -2096,24 +2143,38 @@ func assertServiceInstanceOperationSuccessWithParameters(t *testing.T, obj runti readyStatus v1beta1.ConditionStatus deprovisionStatus v1beta1.ServiceInstanceDeprovisionStatus ) + var provisionStatus v1beta1.ServiceInstanceProvisionStatus + var observedGeneration int64 switch operation { case v1beta1.ServiceInstanceOperationProvision: + provisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned reason = successProvisionReason readyStatus = v1beta1.ConditionTrue deprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired + observedGeneration = originalInstance.Generation case v1beta1.ServiceInstanceOperationUpdate: + provisionStatus = v1beta1.ServiceInstanceProvisionStatusProvisioned reason = successUpdateInstanceReason readyStatus = v1beta1.ConditionTrue deprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusRequired + observedGeneration = originalInstance.Generation case v1beta1.ServiceInstanceOperationDeprovision: + provisionStatus = v1beta1.ServiceInstanceProvisionStatusNotProvisioned reason = successDeprovisionReason readyStatus = v1beta1.ConditionFalse deprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusSucceeded + if originalInstance.Status.OrphanMitigationInProgress { + observedGeneration = originalInstance.Status.ObservedGeneration + } else { + observedGeneration = originalInstance.Generation + } } assertServiceInstanceReadyCondition(t, obj, readyStatus, reason) assertServiceInstanceCurrentOperationClear(t, obj) assertServiceInstanceOperationStartTimeSet(t, obj, false) - assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Generation) + assertServiceInstanceReconciledGeneration(t, obj, observedGeneration) + assertServiceInstanceObservedGeneration(t, obj, observedGeneration) + assertServiceInstanceProvisioned(t, obj, provisionStatus) assertAsyncOpInProgressFalse(t, obj) assertServiceInstanceOrphanMitigationInProgressFalse(t, obj) if operation == v1beta1.ServiceInstanceOperationDeprovision { @@ -2143,13 +2204,9 @@ func assertServiceInstanceRequestFailingError(t *testing.T, obj runtime.Object, readyStatus = v1beta1.ConditionUnknown deprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusFailed } + assertServiceInstanceReadyCondition(t, obj, readyStatus, readyReason) - switch operation { - case v1beta1.ServiceInstanceOperationProvision, v1beta1.ServiceInstanceOperationDeprovision: - assertServiceInstanceCondition(t, obj, v1beta1.ServiceInstanceConditionFailed, v1beta1.ConditionTrue, failureReason) - case v1beta1.ServiceInstanceOperationUpdate: - assertServiceInstanceConditionsCount(t, obj, 1) - } + assertServiceInstanceCondition(t, obj, v1beta1.ServiceInstanceConditionFailed, v1beta1.ConditionTrue, failureReason) assertServiceInstanceOperationStartTimeSet(t, obj, false) assertAsyncOpInProgressFalse(t, obj) assertServiceInstanceExternalPropertiesUnchanged(t, obj, originalInstance) @@ -2160,6 +2217,8 @@ func assertServiceInstanceRequestFailingErrorNoOrphanMitigation(t *testing.T, ob assertServiceInstanceRequestFailingError(t, obj, operation, readyReason, failureReason, originalInstance) assertServiceInstanceCurrentOperationClear(t, obj) assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Generation) + assertServiceInstanceObservedGeneration(t, obj, originalInstance.Generation) + assertServiceInstanceProvisioned(t, obj, originalInstance.Status.ProvisionStatus) assertServiceInstanceOrphanMitigationInProgressFalse(t, obj) assertServiceInstanceInProgressPropertiesNil(t, obj) } @@ -2168,6 +2227,8 @@ func assertServiceInstanceRequestFailingErrorStartOrphanMitigation(t *testing.T, assertServiceInstanceRequestFailingError(t, obj, operation, readyReason, failureReason, originalInstance) assertServiceInstanceCurrentOperation(t, obj, v1beta1.ServiceInstanceOperationProvision) assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Status.ReconciledGeneration) + assertServiceInstanceObservedGeneration(t, obj, originalInstance.Generation) + assertServiceInstanceProvisioned(t, obj, originalInstance.Status.ProvisionStatus) assertServiceInstanceOrphanMitigationInProgressTrue(t, obj) } @@ -2187,6 +2248,8 @@ func assertServiceInstanceRequestRetriableErrorWithParameters(t *testing.T, obj assertServiceInstanceCurrentOperation(t, obj, operation) assertServiceInstanceOperationStartTimeSet(t, obj, true) assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Status.ReconciledGeneration) + assertServiceInstanceObservedGeneration(t, obj, originalInstance.Generation) + assertServiceInstanceProvisioned(t, obj, originalInstance.Status.ProvisionStatus) switch operation { case v1beta1.ServiceInstanceOperationProvision, v1beta1.ServiceInstanceOperationUpdate: assertServiceInstanceInProgressPropertiesPlan(t, obj, planName, planID) @@ -2198,7 +2261,35 @@ func assertServiceInstanceRequestRetriableErrorWithParameters(t *testing.T, obj assertServiceInstanceDeprovisionStatus(t, obj, originalInstance.Status.DeprovisionStatus) } -func assertServiceInstanceAsyncInProgress(t *testing.T, obj runtime.Object, operation v1beta1.ServiceInstanceOperation, operationKey string, planName, planID string, originalInstance *v1beta1.ServiceInstance) { +func assertServiceInstanceAsyncStartInProgress(t *testing.T, obj runtime.Object, operation v1beta1.ServiceInstanceOperation, operationKey string, planName, planID string, originalInstance *v1beta1.ServiceInstance) { + reason := "" + switch operation { + case v1beta1.ServiceInstanceOperationProvision: + reason = asyncProvisioningReason + case v1beta1.ServiceInstanceOperationUpdate: + reason = asyncUpdatingInstanceReason + case v1beta1.ServiceInstanceOperationDeprovision: + reason = asyncDeprovisioningReason + } + assertServiceInstanceReadyFalse(t, obj, reason) + assertServiceInstanceLastOperation(t, obj, operationKey) + assertServiceInstanceCurrentOperation(t, obj, operation) + assertServiceInstanceOperationStartTimeSet(t, obj, true) + assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Status.ReconciledGeneration) + assertServiceInstanceObservedGeneration(t, obj, originalInstance.Generation) + assertServiceInstanceProvisioned(t, obj, originalInstance.Status.ProvisionStatus) + switch operation { + case v1beta1.ServiceInstanceOperationProvision, v1beta1.ServiceInstanceOperationUpdate: + assertServiceInstanceInProgressPropertiesPlan(t, obj, planName, planID) + assertServiceInstanceInProgressPropertiesParameters(t, obj, nil, "") + case v1beta1.ServiceInstanceOperationDeprovision: + assertServiceInstanceInProgressPropertiesNil(t, obj) + } + assertAsyncOpInProgressTrue(t, obj) + assertServiceInstanceDeprovisionStatus(t, obj, originalInstance.Status.DeprovisionStatus) +} + +func assertServiceInstanceAsyncStillInProgress(t *testing.T, obj runtime.Object, operation v1beta1.ServiceInstanceOperation, operationKey string, planName, planID string, originalInstance *v1beta1.ServiceInstance) { reason := "" switch operation { case v1beta1.ServiceInstanceOperationProvision: @@ -2213,6 +2304,8 @@ func assertServiceInstanceAsyncInProgress(t *testing.T, obj runtime.Object, oper assertServiceInstanceCurrentOperation(t, obj, operation) assertServiceInstanceOperationStartTimeSet(t, obj, true) assertServiceInstanceReconciledGeneration(t, obj, originalInstance.Status.ReconciledGeneration) + assertServiceInstanceObservedGeneration(t, obj, originalInstance.Status.ObservedGeneration) + assertServiceInstanceProvisioned(t, obj, originalInstance.Status.ProvisionStatus) switch operation { case v1beta1.ServiceInstanceOperationProvision, v1beta1.ServiceInstanceOperationUpdate: assertServiceInstanceInProgressPropertiesPlan(t, obj, planName, planID) diff --git a/pkg/openapi/openapi_generated.go b/pkg/openapi/openapi_generated.go index e9454462e83..305d58f2971 100644 --- a/pkg/openapi/openapi_generated.go +++ b/pkg/openapi/openapi_generated.go @@ -1495,7 +1495,14 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA }, "reconciledGeneration": { SchemaProps: spec.SchemaProps{ - Description: "ReconciledGeneration is the 'Generation' of the serviceInstanceSpec that was last processed by the controller. The reconciled generation is updated even if the controller failed to process the spec.", + Description: "ReconciledGeneration is the 'Generation' of the serviceInstanceSpec that was last processed by the controller. The reconciled generation is updated even if the controller failed to process the spec. Deprecated: use ObservedGeneration instead", + Type: []string{"integer"}, + Format: "int64", + }, + }, + "observedGeneration": { + SchemaProps: spec.SchemaProps{ + Description: "ObservedGeneration is the 'Generation' of the serviceInstanceSpec that was last processed by the controller. The observed generation is updated whenever the status is updated regardless of operation result", Type: []string{"integer"}, Format: "int64", }, @@ -1518,6 +1525,13 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA Ref: ref("github.com/kubernetes-incubator/service-catalog/pkg/apis/servicecatalog/v1beta1.ServiceInstancePropertiesState"), }, }, + "provisionStatus": { + SchemaProps: spec.SchemaProps{ + Description: "ProvisionStatus describes whether the instance is in the provisioned state.", + Type: []string{"string"}, + Format: "", + }, + }, "deprovisionStatus": { SchemaProps: spec.SchemaProps{ Description: "DeprovisionStatus describes what has been done to deprovision the ServiceInstance.", @@ -1526,7 +1540,7 @@ func GetOpenAPIDefinitions(ref common.ReferenceCallback) map[string]common.OpenA }, }, }, - Required: []string{"conditions", "asyncOpInProgress", "orphanMitigationInProgress", "reconciledGeneration", "deprovisionStatus"}, + Required: []string{"conditions", "asyncOpInProgress", "orphanMitigationInProgress", "reconciledGeneration", "observedGeneration", "provisionStatus", "deprovisionStatus"}, }, }, Dependencies: []string{ diff --git a/test/integration/clientset_test.go b/test/integration/clientset_test.go index 27320b9ddb0..347ec39562e 100644 --- a/test/integration/clientset_test.go +++ b/test/integration/clientset_test.go @@ -923,6 +923,8 @@ func testInstanceClient(sType server.StorageType, client servicecatalogclient.In } instanceServer.Status = v1beta1.ServiceInstanceStatus{ ReconciledGeneration: instanceServer.Generation, + ObservedGeneration: instanceServer.Generation, + ProvisionStatus: v1beta1.ServiceInstanceProvisionStatusProvisioned, Conditions: []v1beta1.ServiceInstanceCondition{readyConditionTrue}, DeprovisionStatus: v1beta1.ServiceInstanceDeprovisionStatusNotRequired, } diff --git a/test/integration/controller_instance_test.go b/test/integration/controller_instance_test.go index cf3deb2f742..b565b5db7d3 100644 --- a/test/integration/controller_instance_test.go +++ b/test/integration/controller_instance_test.go @@ -562,11 +562,13 @@ func TestUpdateServiceInstanceChangePlans(t *testing.T) { ct.instance.Spec.ClusterServicePlanName = otherPlanID } - if _, err := ct.client.ServiceInstances(testNamespace).Update(ct.instance); err != nil { + updatedInstance, err := ct.client.ServiceInstances(testNamespace).Update(ct.instance) + if err != nil { t.Fatalf("error updating Instance: %v", err) } + ct.instance = updatedInstance - if err := util.WaitForInstanceReconciledGeneration(ct.client, testNamespace, testInstanceName, ct.instance.Status.ReconciledGeneration+1); err != nil { + if err := util.WaitForInstanceProcessedGeneration(ct.client, testNamespace, testInstanceName, ct.instance.Generation); err != nil { t.Fatalf("error waiting for instance to reconcile: %v", err) } @@ -833,11 +835,13 @@ func TestUpdateServiceInstanceUpdateParameters(t *testing.T) { ct.instance.Spec.UpdateRequests++ } - if _, err := ct.client.ServiceInstances(testNamespace).Update(ct.instance); err != nil { + updatedInstance, err := ct.client.ServiceInstances(testNamespace).Update(ct.instance) + if err != nil { t.Fatalf("error updating Instance: %v", err) } + ct.instance = updatedInstance - if err := util.WaitForInstanceReconciledGeneration(ct.client, testNamespace, testInstanceName, ct.instance.Status.ReconciledGeneration+1); err != nil { + if err := util.WaitForInstanceProcessedGeneration(ct.client, testNamespace, testInstanceName, ct.instance.Generation); err != nil { t.Fatalf("error waiting for instance to reconcile: %v", err) } @@ -1032,7 +1036,7 @@ func TestCreateServiceInstanceWithProvisionFailure(t *testing.T) { } if tc.expectFailCondition { - if err := util.WaitForInstanceReconciledGeneration(ct.client, ct.instance.Namespace, ct.instance.Name, 1); err != nil { + if err := util.WaitForInstanceProcessedGeneration(ct.client, ct.instance.Namespace, ct.instance.Name, 1); err != nil { t.Fatalf("error waiting for instance reconciliation to complete: %v", err) } } diff --git a/test/integration/controller_test.go b/test/integration/controller_test.go index 7fe502283d8..afeba225593 100644 --- a/test/integration/controller_test.go +++ b/test/integration/controller_test.go @@ -142,7 +142,7 @@ func TestBasicFlows(t *testing.T) { t.Fatalf("error updating Instance: %v", err) } - if err := util.WaitForInstanceReconciledGeneration(ct.client, testNamespace, testInstanceName, ct.instance.Status.ReconciledGeneration+1); err != nil { + if err := util.WaitForInstanceProcessedGeneration(ct.client, testNamespace, testInstanceName, ct.instance.Status.ReconciledGeneration+1); err != nil { t.Fatalf("error waiting for instance to reconcile: %v", err) } @@ -271,7 +271,7 @@ func TestBasicFlowsWithOriginatingIdentity(t *testing.T) { t.Fatalf("error updating Instance: %v", err) } - if err := util.WaitForInstanceReconciledGeneration(ct.client, testNamespace, testInstanceName, ct.instance.Status.ReconciledGeneration+1); err != nil { + if err := util.WaitForInstanceProcessedGeneration(ct.client, testNamespace, testInstanceName, ct.instance.Status.ReconciledGeneration+1); err != nil { t.Fatalf("error waiting for instance to reconcile: %v", err) } diff --git a/test/util/util.go b/test/util/util.go index 3f838395b11..b75531f2f4e 100644 --- a/test/util/util.go +++ b/test/util/util.go @@ -205,18 +205,20 @@ func WaitForInstanceToNotExist(client v1beta1servicecatalog.ServicecatalogV1beta ) } -// WaitForInstanceReconciledGeneration waits for the status of the named instance to +// WaitForInstanceProcessedGeneration waits for the status of the named instance to // have the specified reconciled generation. -func WaitForInstanceReconciledGeneration(client v1beta1servicecatalog.ServicecatalogV1beta1Interface, namespace, name string, reconciledGeneration int64) error { +func WaitForInstanceProcessedGeneration(client v1beta1servicecatalog.ServicecatalogV1beta1Interface, namespace, name string, processedGeneration int64) error { return wait.PollImmediate(500*time.Millisecond, wait.ForeverTestTimeout, func() (bool, error) { - glog.V(5).Infof("Waiting for instance %v/%v to have reconciled generation of %v", namespace, name, reconciledGeneration) + glog.V(5).Infof("Waiting for instance %v/%v to have processed generation of %v", namespace, name, processedGeneration) instance, err := client.ServiceInstances(namespace).Get(name, metav1.GetOptions{}) if nil != err { return false, fmt.Errorf("error getting Instance %v/%v: %v", namespace, name, err) } - if instance.Status.ReconciledGeneration == reconciledGeneration { + if instance.Status.ObservedGeneration >= processedGeneration && + (isServiceInstanceReady(instance) || isServiceInstanceFailed(instance)) && + !instance.Status.OrphanMitigationInProgress { return true, nil } @@ -225,6 +227,30 @@ func WaitForInstanceReconciledGeneration(client v1beta1servicecatalog.Servicecat ) } +// isServiceInstanceConditionTrue returns whether the given instance has a given condition +// with status true. +func isServiceInstanceConditionTrue(instance *v1beta1.ServiceInstance, conditionType v1beta1.ServiceInstanceConditionType) bool { + for _, cond := range instance.Status.Conditions { + if cond.Type == conditionType { + return cond.Status == v1beta1.ConditionTrue + } + } + + return false +} + +// isServiceInstanceReady returns whether the given instance has a ready condition +// with status true. +func isServiceInstanceReady(instance *v1beta1.ServiceInstance) bool { + return isServiceInstanceConditionTrue(instance, v1beta1.ServiceInstanceConditionReady) +} + +// isServiceInstanceFailed returns whether the instance has a failed condition with +// status true. +func isServiceInstanceFailed(instance *v1beta1.ServiceInstance) bool { + return isServiceInstanceConditionTrue(instance, v1beta1.ServiceInstanceConditionFailed) +} + // WaitForBindingCondition waits for the status of the named binding to contain // a condition whose type and status matches the supplied one and then returns // back the last binding condition of the same type requested during polling if found.