-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Bug 2009879: Add admin ack Upgradeable condition gate test #26446
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
Conversation
|
@jottofar: This pull request references Bugzilla bug 1999092, which is valid. The bug has been updated to refer to the pull request using the external bug tracker. 6 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
c3107bc to
e62599d
Compare
|
@jottofar: This pull request references Bugzilla bug 1999092, which is valid. 6 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
1 similar comment
|
@jottofar: This pull request references Bugzilla bug 1999092, which is valid. 6 validation(s) were run on this bug
Requesting review from QA contact: DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
e62599d to
b249623
Compare
b249623 to
360adde
Compare
|
/retest |
…e-1.22-api-removals-in-4.9
Avoid [1]:
alert ClusterNotUpgradeable fired for 1115 seconds with labels: {condition="Upgradeable", endpoint="metrics", name="version", severity="warning"}
for tests that install 4.8 and then sit on it for over an hour, now
that [2] has landed in 4.8. [3] will follow up with code to handle CI
that installs an earlier release and updates into 4.8 or later.
[1]: https://prow.ci.openshift.org/view/gs/origin-ci-test/logs/periodic-ci-openshift-release-master-nightly-4.8-e2e-aws-serial/1443018123379740672
[2]: openshift/cluster-version-operator#647
[3]: openshift/origin#26446
|
/retitle Bug 1999092: Add admin ack Upgradeable condition gate test |
| // AllTests includes all tests (minimal + disruption) | ||
| func AllTests() []upgrades.Test { | ||
| return []upgrades.Test{ | ||
| &adminack.UpgradeTest{}, |
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.
I don't know if the order here is important, but I'd like this test to run at the end of the suite (I'd have expected to have that happen via a [Late] annotation). But the alert.UpgradeTest{} precedent makes it seem like order does matter here. I'm not entirely sure how these are positioned relative to the update itself, but the logging makes them seem like they're currently running early in the job:
$ curl -s https://gcsweb-ci.apps.ci.l2s4.p1.openshiftapps.com/gcs/origin-ci-test/pr-logs/pull/26446/pull-ci-openshift-origin-release-4.8-e2e-aws-upgrade/1438885249361645568/artifacts/e2e-aws-upgrade/openshift-e2e-test/artifacts/e2e.log | grep -n 'Cluster should remain functional during upgrade\|admin ack' | grep -B100 passed
84:started: (0/5/5) "[sig-arch][Feature:ClusterUpgrade] Cluster should remain functional during upgrade [Disruptive] [Serial]"
95:[It] Cluster should remain functional during upgrade [Disruptive] [Serial]
167:STEP: Setting up admin ack test
170:Sep 17 16:10:42.260: INFO: Skipping admin ack test. Admin ack is not in this baseline.
1751:passed: (2h47m1s) 2021-09-17T18:57:41 "[sig-arch][Feature:ClusterUpgrade] Cluster should remain functional during upgrade [Disruptive] [Serial]"(with line 170 presumably being written quite a while before line 1751). Running later (post update) also protects 4.7 -> 4.8 jobs from having Upgradeable=False alerting go off, because the openshift/release#22368 approach will not protect those jobs (and some jobs run slow parallel-conformance suites after completing the update).
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.
It is order dependent. I put it early since the the upgrade cannot proceed until the ack is in place and the test ends leaving the ack in place.
360adde to
4e51920
Compare
|
/retitle Bug 2009879: Add admin ack Upgradeable condition gate test No target on bug 2009879 yet, while we work out whether this PR will be 4.8-only or not, but pointing away from the old bug series so the Bugzilla bot doesn't attempt to re-attach this PR. |
eab2736 to
2cdcbcc
Compare
|
/test verify |
52fe0bd to
513c7da
Compare
|
/retest |
2729e06 to
6b175c4
Compare
|
/hold |
98091aa to
b4ce478
Compare
|
/test verify |
|
@jottofar: The following tests failed, say
Full PR test history. Your PR dashboard. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: jianlinliu, jottofar The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Replaced by #26635, backporting the generic logic that landed in /close |
|
@wking: Closed this PR. DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
Since only later 4.8.z releases will contain the admin ack functionality, the test first checks for presence of the admin-gates configmap. If not found, indicating that the admin ack functionality is not part the baseline being tested, the test simply returns successfully.
Otherwise, test first verifies that Upgradeable condition is false for correct reason and with correct message. It then modifies the admin-acks configmap to ack the necessary admin ack gate and then waits for the Upgradeable condition to change to true.