-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Ignore upgradable false condition on TechPreview clusters #26349
Ignore upgradable false condition on TechPreview clusters #26349
Conversation
ff1d9b8
to
e25520b
Compare
I have now tested this on a cluster and it is working as expected |
/retest |
// kube-apiserver blocks upgrades when feature gates are present. | ||
// Allow testing of TechPreviewNoUpgrade clusters by ignoring this condition. | ||
if isTechPreview && name == "kube-apiserver" && isKubeAPIUpgradableTechPreviewCondition(worstCondition) { | ||
continue |
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.
Does this risk ignoring other failure conditions?
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.
This may depend on what surprisingConditions
returns. I would expect this condition to be relatively low down the list of bad conditions, but it's something I need to check.
If my assumption that this isn't a terrible condition is correct, then another failure should take precedence in assignment of worstCondition
and we should be safe.
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.
Confirmed, Upgradable is the last one it looks for when it's searching the conditions, this should be safe as is, if the available or degraded conditions are not as expected they will take precedence
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.
+1, this generally makes sense to me and will help with the tests
/lgtm |
/assign @njhale |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed, mdbooth, smarterclayton The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
/retest-required |
/retest-required Please review the full test history for this PR and help us cut down flakes. |
While trying to set up CI testing for clusters with a
TechPreviewNoUpgrade
feature gate, this test fails as Kube API server blocks upgrades for tech preview clusters.This PR adds an exception so that if a cluster has a
TechPreviewNoUpgrade
feature set enabled, and the appropriate condition, this will no longer prevent the test from passing.Specifically it prevents this error:
Ref: openshift/release#19845