OCPBUGS-6851: [release-4.11] upgrade/adminack: guarantee one admin ack check post-upgrade #27685
Conversation
While looking into OCPBUGS-5505 I discovered that some 4.10->4.11 upgrade job runs perform an Admin Ack check, while some do not. 4.11 has a `ack-4.11-kube-1.25-api-removals-in-4.12` gate, so these upgrade jobs sometimes test that `Upgradeable` goes `false` after the ugprade, and sometimes they do not. This is only determined by the polling race condition: the check is executed once per 10 minutes, and we cancel the polling after upgrade is completed. This means that in some cases we are lucky and manage to run one check before the cancel, and sometimes we are not and only check while still on the base version. Add a guaranteed single check execution after the upgrade, so that admin ack is always checked at least once with the upgrade target version. Doing checks after `done` is signalled has prior art in the alert test.
The `done` signal is either a timeout or "upgrade finished, stop testing". We do not need to perform the last check in the former case. Track versions that we check and when we get the signal, check whether the current version was checked at least once, and if not, check it before terminating.
…ade check openshift#27645 intended to add a guaranteed post-upgrade check but I have overlooked how exactly the polling is implemented and terminated, leading to the post-upgrade check never actually execute. Previously the test used `PollImmediateWithContext` for the each-10-minutes check. The `ConditionFunc` never actually returned `true` or non-nil `err`, so the `PollImmediateWithContext` never terminated by the means of `ConditionFunc`: it was always terminated by the `ctx.Done()` that the framework does on finished upgrade (or a test timeout). This means that `PollImmediateWithContext` always terminated with `err=wait.ErrWaitTimeout` and the `Test` method immediately returned, so the "guaranteed" check code is never reached. Given our `ConditionFunc` never terminates the polling, we can simplify and use the `wait.UntilWithContext` instead, which is a simpler version that precisely implements the desired loop (poll until context is done).
During testing of OCPBUGS-5505, it was discovered that even with shortening the CVO cache TTL, CVO may still only update `Upgradeable` in its sync interval, which may be as high as 4 minutes. Hence the tests needs to wait for that time (I added 5 second buffer on top of that).
|
@petr-muller: No Bugzilla bug is referenced in the title of this pull request. 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. |
openshift-ci
bot
added
the
do-not-merge/hold
petr-muller
changed the title
|
@petr-muller: No Bugzilla bug is referenced in the title of this pull request. 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. |
|
/retest |
|
@petr-muller: The following test 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. |
|
/jira cherrypick OCPBUGS-6850 |
|
@petr-muller: Jira Issue OCPBUGS-6850 has been cloned as Jira Issue OCPBUGS-6851. Retitling PR to link against new bug. 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. |
|
/skip |
|
/retitle OCPBUGS-6851: [release-4.11] upgrade/adminack: guarantee one admin ack check post-upgrade |
openshift-ci
bot
changed the title
|
@petr-muller: No Bugzilla bug is referenced in the title of this pull request. 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. |
openshift-ci-robot
added
jira/severity-moderate
|
@petr-muller: This pull request references Jira Issue OCPBUGS-6851, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: LalatenduMohanty, petr-muller The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/label backport-risk-assessed |
openshift-ci
bot
added
the
backport-risk-assessed
openshift-ci
bot
added
the
cherry-pick-approved
openshift-ci
bot
assigned
dgoodwin,
gangwgr,
jadhaj,
jitendar-singh,
kasturinarra,
pamoedom,
prietyc123,
stbenjam and
sunilcio
|
/jira refresh |
|
@petr-muller: This pull request references Jira Issue OCPBUGS-6851, which is invalid:
Comment 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. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@petr-muller: This pull request references Jira Issue OCPBUGS-6851, which is valid. The bug has been moved to the POST state. 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. |
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
|
@petr-muller: All pull requests linked via external trackers have merged: Jira Issue OCPBUGS-6851 has been moved to the MODIFIED state. 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. |
13 participants
This is a cherry-pick of #27645 and #27678
/hold