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

Conversation

staebler
Copy link
Contributor

Fixes #1487.

@k8s-ci-robot k8s-ci-robot added cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 31, 2017
@@ -1426,6 +1426,9 @@ func (c *controller) pollServiceInstance(instance *v1beta1.ServiceInstance) erro
if deleting {
toUpdate.Status.DeprovisionStatus = v1beta1.ServiceInstanceDeprovisionStatusSucceeded
}
if !deleting {
Copy link
Contributor

Choose a reason for hiding this comment

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

s/if !deleting/else/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason that I didn't use else here is that the two branches are not related. It is just coincidental that the conditions are opposites.

@@ -2020,3 +2023,16 @@ func shouldStartOrphanMitigation(statusCode int) bool {
statusCode == http.StatusRequestTimeout ||
is5XX
}

// removeServiceInstanceFailedCondition removes the Failed condition from the
Copy link
Contributor

Choose a reason for hiding this comment

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

So, when an instance which provisioned but then failed an update is then updated again, we remove the failed condition? maybe we just shouldn't set it when updates fail. WDYT?

@k8s-ci-robot k8s-ci-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Nov 1, 2017
@staebler staebler force-pushed the 1487-allow_update_after_failed_update branch from b501844 to 1770971 Compare November 1, 2017 14:00
@pmorie pmorie added the LGTM1 label Nov 1, 2017
@@ -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.

@n3wscott
Copy link
Contributor

n3wscott commented Nov 1, 2017

LGTM

@kibbles-n-bytes kibbles-n-bytes merged commit f6c891d into kubernetes-retired:master Nov 1, 2017
@kibbles-n-bytes kibbles-n-bytes added this to the 0.1.2 milestone Nov 2, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. LGTM1 LGTM2 size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants