-
Notifications
You must be signed in to change notification settings - Fork 697
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
Change upgrade path validation for 8.0 to only allow 7.17 #5261
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
// this should not happen, since this is the already persisted version | ||
errs = append(errs, field.Invalid(field.NewPath("spec").Child("version"), current.Spec.Version, parseStoredVersionErrMsg)) | ||
currentVer, ferr := currentVersion(current) | ||
if ferr != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ferr
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ferr
for field.Error
which is what the webhook API works with, err
is used as a variable for a regular builtin error
below
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM as well, but your non-deterministic TestDelayingQueueInvariants
tests are failing you in ci.
Jenkins test this please TestDelayingQueueInvariants is flaky sometimes unfortunately |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
This sets the minimal supported version for 8.0 to 7.17. It also changes the validation on version upgrade to take the .status.Version into account instead of the spec.Version (which it still uses as a fallback in case we do not have a valid .status.Version). The rationale behind that is that we want to make sure that users do update the version of the spec in quick succession without waiting for the rollout of the upgrade to complete thereby unwittingly circumventing the version restriction.
) This sets the minimal supported version for 8.0 to 7.17. It also changes the validation on version upgrade to take the .status.Version into account instead of the spec.Version (which it still uses as a fallback in case we do not have a valid .status.Version). The rationale behind that is that we want to make sure that users do update the version of the spec in quick succession without waiting for the rollout of the upgrade to complete thereby unwittingly circumventing the version restriction.
This sets the minimal supported version for 8.0 to 7.17. It also changes the validation on version upgrade to take the .status.Version into account instead of the spec.Version (which it still uses as a fallback in case we do not have a valid .status.Version). The rationale behind that is that we want to make sure that users do update the version of the spec in quick succession without waiting for the rollout of the upgrade to complete thereby unwittingly circumventing the version restriction.
Fixes #5258
This sets the minimal supported version for 8.0 to 7.17. It also changes the validation on version upgrade to take the
.status.Version
into account instead of thespec.Version
(which it still uses as a fallback in case we do not have a valid.status.Version
). The rationale behind that is that we want to make sure that users do update the version of thespec
in quick succession without waiting for the rollout of the upgrade to complete thereby unwittingly circumventing the version restriction.