Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If RollingUpgrade object is deleted during rollout, upgrade manager doesn't stop the rollout. #139

Open
uthark opened this issue Nov 14, 2020 · 1 comment

Comments

@uthark
Copy link
Contributor

uthark commented Nov 14, 2020

  1. It continues to rollout.
  2. It fails to update object (because it is gone)
{"level":"error","ts":1605329457.7081757,"logger":"controllers.RollingUpgrade","msg":"failed to update status","rollingupgrade":"rollingupgrade-node-us-west-2a","error":"Operation cannot be fulfilled on rollingupgrades.upgrademgr.keikoproj.io \"rollingupgrade-node-us-west-2a\": StorageError: invalid object, Code: 4, Key: /registry/upgrademgr.keikoproj.io/rollingupgrades/keiko-upgrade-manager/rollingupgrade-node-us-west-2a, ResourceVersion: 0, AdditionalErrorMsg: Precondition failed: UID in precondition: 5b4944eb-f154-412d-b6cb-11c65b6580c2, UID in object meta: ","stacktrace":"github.com/go-logr/zapr.(*zapLogger).Error\n\t/go/pkg/mod/github.com/go-logr/zapr@v0.1.0/zapr.go:128\ngithub.com/keikoproj/upgrade-manager/controllers.(*RollingUpgradeReconciler).error\n\t/workspace/controllers/rollingupgrade_controller.go:1132\ngithub.com/keikoproj/upgrade-manager/controllers.(*RollingUpgradeReconciler).UpdateInstance\n\t/workspace/controllers/rollingupgrade_controller.go:1061"}

Proper reconciliation should stop existing rollout and not fail.

@shrinandj
Copy link
Collaborator

There was some code added to handle this. However, there will always be a "time-of-check-to-time-of-use" window.

See

// Check if the err is "StorageError: invalid object". If so, the object was deleted...
and
// Check if the err is "StorageError: invalid object". If so, the object was deleted...

There were several tests done when the functionality was added: #97.

Maybe, some scenario was missed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants