Skip to content
This repository has been archived by the owner on May 6, 2022. It is now read-only.

Allow updates of instances that failed a previous update. #1502

Merged
merged 2 commits into from
Nov 1, 2017
Merged

Allow updates of instances that failed a previous update. #1502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
8dd8dc8
Allow updates of instances that failed a previous update.
staebler Oct 31, 2017
1770971
Do not set Failed condition when instance update fails
staebler Nov 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
90 changes: 62 additions & 28 deletions pkg/controller/controller_instance.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -858,21 +858,23 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance)
glog.Warning(pcb.Message(s))
c.recorder.Event(instance, corev1.EventTypeWarning, reason, s)

setServiceInstanceCondition(
toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
"ClusterServiceBrokerReturnedFailure",
s)
if isProvisioning {
setServiceInstanceCondition(
toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
"ClusterServiceBrokerReturnedFailure",
s)

if isProvisioning && shouldStartOrphanMitigation(httpErr.StatusCode) {
c.setServiceInstanceStartOrphanMitigation(toUpdate)
if shouldStartOrphanMitigation(httpErr.StatusCode) {
c.setServiceInstanceStartOrphanMitigation(toUpdate)

if _, err := c.updateServiceInstanceStatus(toUpdate); err != nil {
return err
}
if _, err := c.updateServiceInstanceStatus(toUpdate); err != nil {
return err
}

return httpErr
return httpErr
}
}

setServiceInstanceCondition(
Expand Down Expand Up @@ -913,14 +915,14 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance)
message = "Communication with the ClusterServiceBroker timed out; operation will not be retried: " + s
// Communication to the broker timed out. Treat as terminal failure and
// begin orphan mitigation.
setServiceInstanceCondition(
toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
reason,
message)

if isProvisioning {
setServiceInstanceCondition(
toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
reason,
message)
c.setServiceInstanceStartOrphanMitigation(toUpdate)
} else {
setServiceInstanceCondition(
Expand Down Expand Up @@ -950,11 +952,19 @@ func (c *controller) reconcileServiceInstance(instance *v1beta1.ServiceInstance)
s := "Stopping reconciliation retries because too much time has elapsed"
glog.Info(pcb.Message(s))
c.recorder.Event(instance, corev1.EventTypeWarning, errorReconciliationRetryTimeoutReason, s)
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
errorReconciliationRetryTimeoutReason,
s)
if isProvisioning {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
errorReconciliationRetryTimeoutReason,
s)
} else {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionReady,
v1beta1.ConditionFalse,
errorReconciliationRetryTimeoutReason,
s)
}
c.clearServiceInstanceCurrentOperation(toUpdate)
if _, err := c.updateServiceInstanceStatus(toUpdate); err != nil {
return err
Expand Down Expand Up @@ -1106,12 +1116,18 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
v1beta1.ConditionUnknown,
errorOrphanMitigationFailedReason,
"Orphan mitigation failed: "+s)
} else {
} else if deleting || provisioning {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
errorReconciliationRetryTimeoutReason,
s)
} else {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionReady,
v1beta1.ConditionFalse,
errorReconciliationRetryTimeoutReason,
s)
}

if !provisioning {
Expand Down Expand Up @@ -1255,12 +1271,18 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
v1beta1.ConditionUnknown,
errorOrphanMitigationFailedReason,
"Orphan mitigation failed: "+s)
} else {
} else if deleting || provisioning {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
errorReconciliationRetryTimeoutReason,
s)
} else {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionReady,
v1beta1.ConditionFalse,
errorReconciliationRetryTimeoutReason,
s)
}

if !provisioning {
Expand Down Expand Up @@ -1347,12 +1369,18 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
v1beta1.ConditionUnknown,
errorOrphanMitigationFailedReason,
"Orphan mitigation failed: "+s)
} else {
} else if deleting || provisioning {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to @n3wscott this will conflict with #1509, easy merge and would have been helpful in this PR to just have to find and fix this once.

setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
errorReconciliationRetryTimeoutReason,
s)
} else {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionReady,
v1beta1.ConditionFalse,
errorReconciliationRetryTimeoutReason,
s)
}

if !provisioning {
Expand Down Expand Up @@ -1505,7 +1533,7 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
message,
)

if !mitigatingOrphan {
if !mitigatingOrphan && (deleting || provisioning) {
setServiceInstanceCondition(
toUpdate,
v1beta1.ServiceInstanceConditionFailed,
Expand Down Expand Up @@ -1539,12 +1567,18 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
v1beta1.ConditionUnknown,
errorOrphanMitigationFailedReason,
"Orphan mitigation failed: "+s)
} else {
} else if deleting || provisioning {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionFailed,
v1beta1.ConditionTrue,
errorReconciliationRetryTimeoutReason,
s)
} else {
setServiceInstanceCondition(toUpdate,
v1beta1.ServiceInstanceConditionReady,
v1beta1.ConditionFalse,
errorReconciliationRetryTimeoutReason,
s)
}

if !provisioning {
Expand Down
9 changes: 7 additions & 2 deletions pkg/controller/controller_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1696,7 +1696,7 @@ func assertServiceInstanceConditionsCount(t *testing.T, obj runtime.Object, coun
}

if e, a := count, len(instance.Status.Conditions); e != a {
t.Fatalf("Expected %v condition, got %v", e, a)
fatalf(t, "Expected %v condition, got %v", e, a)
}
}

Expand Down Expand Up @@ -1943,7 +1943,12 @@ func assertServiceInstanceRequestFailingError(t *testing.T, obj runtime.Object,
deprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusFailed
}
assertServiceInstanceReadyCondition(t, obj, readyStatus, readyReason)
assertServiceInstanceCondition(t, obj, v1beta1.ServiceInstanceConditionFailed, v1beta1.ConditionTrue, failureReason)
switch operation {
case v1beta1.ServiceInstanceOperationProvision, v1beta1.ServiceInstanceOperationDeprovision:
assertServiceInstanceCondition(t, obj, v1beta1.ServiceInstanceConditionFailed, v1beta1.ConditionTrue, failureReason)
case v1beta1.ServiceInstanceOperationUpdate:
assertServiceInstanceConditionsCount(t, obj, 1)
}
assertServiceInstanceOperationStartTimeSet(t, obj, false)
assertAsyncOpInProgressFalse(t, obj)
assertServiceInstanceExternalPropertiesUnchanged(t, obj, originalInstance)
Expand Down