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

[release-4.11] upgrade/adminack: guarantee one admin ack check post-upgrade #27660

Conversation

openshift-cherrypick-robot

This is an automated cherry-pick of #27645

/assign petr-muller

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.
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 16, 2023

@openshift-cherrypick-robot: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/e2e-aws-serial 48ea1bf link true /test e2e-aws-serial
ci/prow/e2e-aws-csi 48ea1bf link false /test e2e-aws-csi
ci/prow/e2e-aws-single-node 48ea1bf link false /test e2e-aws-single-node
ci/prow/e2e-aws-single-node-upgrade 48ea1bf link false /test e2e-aws-single-node-upgrade

Full PR test history. Your PR dashboard.

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. I understand the commands that are listed here.

Copy link
Member

@wking wking left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Jan 17, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 17, 2023

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: openshift-cherrypick-robot, wking

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 /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 17, 2023
@petr-muller
Copy link
Member

This should not merge until openshift/cluster-version-operator#885 does otherwise it will make CI signal worse

/hold

@openshift-ci openshift-ci bot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Jan 19, 2023
@petr-muller
Copy link
Member

Will also need a cherry pick of #27678

@petr-muller
Copy link
Member

petr-muller commented Jan 26, 2023

/close
Superseded by #27685

@openshift-ci openshift-ci bot closed this Jan 26, 2023
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Jan 26, 2023

@petr-muller: Closed this PR.

In response to this:

/close
Superceded by #27685

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm Indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants