Skip to content

Decrement retries count during migration#16340

Merged
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
enj:enj/i/migrate_retry_limit
Sep 14, 2017
Merged

Decrement retries count during migration#16340
openshift-merge-robot merged 1 commit intoopenshift:masterfrom
enj:enj/i/migrate_retry_limit

Conversation

@enj
Copy link
Contributor

@enj enj commented Sep 14, 2017

This change fixes a bug in the migration command that could lead to it being permanently stuck in a retry loop. This would only happen if some other entity continuously changed a resource that was in the process of being migrated. The command would then attempt to retry due to the conflict error. Since the retry count was never decremented, the migration would essentially be stuck in an infinite loop. To avoid having to mutate global state, retries was removed as a field of migrateTracker. It is instead directly passed to the try function and thus is reset on every invocation of result.Visit.

Signed-off-by: Monis Khan mkhan@redhat.com

/assign @smarterclayton

@sdodson @jupierce you saw this during an upgrade: https://bugzilla.redhat.com/show_bug.cgi?id=1489168

This change fixes a bug in the migration command that could lead to
it being permanently stuck in a retry loop.  This would only happen
if some other entity continuously changed a resource that was in the
process of being migrated.  The command would then attempt to retry
due to the conflict error.  Since the retry count was never
decremented, the migration would essentially be stuck in an infinite
loop.  To avoid having to mutate global state, retries was removed
as a field of migrateTracker.  It is instead directly passed to the
try function and thus is reset on every invocation of result.Visit.

Signed-off-by: Monis Khan <mkhan@redhat.com>
@openshift-ci-robot openshift-ci-robot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Sep 14, 2017
@enj
Copy link
Contributor Author

enj commented Sep 14, 2017

/retest

docker.io flake:

+ hack/env JUNIT_REPORT=true make test-cmd -k
[INFO] Pulling the openshift/origin-release:golang-1.8 image to update it...
Trying to pull repository registry.access.redhat.com/openshift/origin-release ... 
Trying to pull repository docker.io/openshift/origin-release ... 
Get https://registry-1.docker.io/v2/openshift/origin-release/manifests/golang-1.8: EOF
[ERROR] PID 12775: hack/lib/build/environment.sh:257: `docker pull "${OS_BUILD_ENV_IMAGE}"` exited with status 1.
[INFO] 		Stack Trace: 
[INFO] 		  1: hack/lib/build/environment.sh:257: `docker pull "${OS_BUILD_ENV_IMAGE}"`
[INFO] 		  2: hack/env:42: os::build::environment::run
[INFO]   Exiting with code 1.
++ export status=FAILURE
++ status=FAILURE
+ set +o xtrace

@smarterclayton
Copy link
Contributor

/lgtm

@openshift-ci-robot openshift-ci-robot added the lgtm Indicates that a PR is ready to be merged. label Sep 14, 2017
@openshift-merge-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: enj, smarterclayton

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@openshift-merge-robot openshift-merge-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Sep 14, 2017
@openshift-merge-robot
Copy link
Contributor

Automatic merge from submit-queue (batch tested with PRs 16339, 16340)

@openshift-merge-robot openshift-merge-robot merged commit 6d3b658 into openshift:master Sep 14, 2017
jupierce added a commit that referenced this pull request Sep 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approved Indicates a PR has been approved by an approver from all required OWNERS files. lgtm Indicates that a PR is ready to be merged. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants