OCPBUGS-24705: consider currentImage and desiredImage annotations

[cheesesashimi]

- What I did

The isNodeUnavailable() code was not taking the currentImage / desiredImage annotations into consideration when it determines if the node should be considered unavailable. While opting into on-cluster builds, the following would happen:

1. A node gets updated with the desiredImage annotation, indicating that the MCD should transition it to an image.
2. There is a lag between when this annotation is applied and the MCD transitions from "Done" -> "Working".
3. During this lag period, the NodeController may re-enter the sync loop.
4. Because the MCD state has not transitioned yet to "Working" yet and we were not considering the currentImage / desiredImage annotations, the NodeController will apply the desiredImage annotation to another node instead.

This adds a check for the presence of the currentImage / desiredImage annotations. If neither annotation is present, the NodeController will not take these annotations into consideration. However, if either of them is present, it will test their equality before making a determination that a node is "Done". This PR also adds additional test cases in an attempt to verify additional unforeseen edge-cases.

- How to verify it

1. Bring up an OpenShift 4.16 cluster.
2. Opt into on-cluster builds using either the docs within this repo or my onclustertesting helpers.
3. Opt the master pool into on-cluster builds by adding the appropriate label: $ oc label mcp/master 'machineconfiguration.openshift.io/layering-enabled='.
4. Only one control plane node should go offline at once.

- Description for the changelog
Consider currentImage / desiredImage annotations when determining if a node is unavailable

[openshift-ci-robot]

@cheesesashimi: This pull request references Jira Issue OCPBUGS-24705, which is invalid:

• expected the bug to target the "4.16.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

- What I did

The isNodeUnavailable() code was not taking the currentImage / desiredImage annotations into consideration when it determines if the node should be considered unavailable. While opting into on-cluster builds, the following would happen:

1. A node gets updated with the desiredImage annotation, indicating that the MCD should transition it to an image.
2. There is a lag between when this annotation is applied and the MCD transitions from "Done" -> "Working".
3. During this lag period, the NodeController may re-enter the sync loop.
4. Because the MCD state has not transitioned yet to "Working" yet and we were not considering the currentImage / desiredImage annotations, the NodeController will apply the desiredImage annotation to another node instead.

My fix adds a check for the presence of the currentImage / desiredImage annotations. If neither annotation is present, the NodeController will not take these annotations into consideration. However, if either of them is present, it will test their equality before making a determination that a node is "Done". This PR also adds additional test cases in an attempt to verify additional unforeseen edge-cases.

- How to verify it

1. Bring up an OpenShift 4.16 cluster.
2. Opt into on-cluster builds using either the docs within this repo or my onclustertesting helpers.
3. Opt the master pool into on-cluster builds by adding the appropriate label: $ oc label mcp/master 'machineconfiguration.openshift.io/layering-enabled='.
4. Only one control plane node should go offline at once.

- Description for the changelog
Consider currentImage / desiredImage annotations when determining if a node is unavailable

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-ci]

Skipping CI for Draft Pull Request.
If you want CI signal for your change, please convert it to an actual PR.
You can still manually trigger a test run with /test all

[cheesesashimi]

/jira refresh

[openshift-ci-robot]

@cheesesashimi: This pull request references Jira Issue OCPBUGS-24705, which is invalid:

• expected the bug to target the "4.16.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

[cheesesashimi]

/jira refresh

[openshift-ci-robot]

@cheesesashimi: This pull request references Jira Issue OCPBUGS-24705, which is invalid:

• expected the bug to target the "4.16.0" version, but no target version was set

Comment /jira refresh to re-evaluate validity if changes to the Jira bug are made, or edit the title of this pull request to link to a different bug.

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

[yuqi-zhang]

/jira refresh

[cheesesashimi]

/jira refresh

[openshift-ci-robot]

@cheesesashimi: This pull request references Jira Issue OCPBUGS-24705, which is valid. The bug has been moved to the POST state.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.16.0) matches configured target version for branch (4.16.0)
• bug is in the state ASSIGNED, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @sergiordlr

The bug has been updated to refer to the pull request using the external bug tracker.

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

[cheesesashimi]

/jira refresh

[openshift-ci-robot]

@cheesesashimi: This pull request references Jira Issue OCPBUGS-24705, which is valid.

3 validation(s) were run on this bug

• bug is open, matching expected state (open)
• bug target version (4.16.0) matches configured target version for branch (4.16.0)
• bug is in the state POST, which is one of the valid states (NEW, ASSIGNED, POST)

Requesting review from QA contact:
/cc @sergiordlr

In response to this:

/jira refresh

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 openshift-eng/jira-lifecycle-plugin repository.

[cdoern]

looks good! just one nit

[yuqi-zhang]

/lgtm

Logic is very clear, and great test coverage!

[yuqi-zhang]

/override ci/prow/e2e-gcp-op-single-node

pre-emptively

[openshift-ci]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cdoern, cheesesashimi, yuqi-zhang

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:

• OWNERS [cdoern,cheesesashimi,yuqi-zhang]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[openshift-ci]

@yuqi-zhang: Overrode contexts on behalf of yuqi-zhang: ci/prow/e2e-gcp-op-single-node

In response to this:

/override ci/prow/e2e-gcp-op-single-node

pre-emptively

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]

/retest-required

Remaining retests: 0 against base HEAD a460e63 and 2 for PR HEAD 124dd45 in total

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD a3dbd65 and 1 for PR HEAD 124dd45 in total

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD c5f53e2 and 0 for PR HEAD 124dd45 in total

[openshift-ci-robot]

/hold

Revision 124dd45 was retested 3 times: holding

[yuqi-zhang]

/hold cancel
/override ci/prow/e2e-gcp-op-single-node

[yuqi-zhang]

/hold cancel

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD aa1496f and 2 for PR HEAD 124dd45 in total

[openshift-ci]

@cheesesashimi: The following test 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/okd-scos-e2e-aws-ovn	124dd45	link	false	/test okd-scos-e2e-aws-ovn

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.

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 2da0539 and 1 for PR HEAD 124dd45 in total

[openshift-ci-robot]

/retest-required

Remaining retests: 0 against base HEAD 030bcf7 and 0 for PR HEAD 124dd45 in total

[openshift-ci-robot]

@cheesesashimi: Jira Issue OCPBUGS-24705: All pull requests linked via external trackers have merged:

• openshift/machine-config-operator#4135

Jira Issue OCPBUGS-24705 has been moved to the MODIFIED state.

In response to this:

- What I did

The isNodeUnavailable() code was not taking the currentImage / desiredImage annotations into consideration when it determines if the node should be considered unavailable. While opting into on-cluster builds, the following would happen:

1. A node gets updated with the desiredImage annotation, indicating that the MCD should transition it to an image.
2. There is a lag between when this annotation is applied and the MCD transitions from "Done" -> "Working".
3. During this lag period, the NodeController may re-enter the sync loop.
4. Because the MCD state has not transitioned yet to "Working" yet and we were not considering the currentImage / desiredImage annotations, the NodeController will apply the desiredImage annotation to another node instead.

This adds a check for the presence of the currentImage / desiredImage annotations. If neither annotation is present, the NodeController will not take these annotations into consideration. However, if either of them is present, it will test their equality before making a determination that a node is "Done". This PR also adds additional test cases in an attempt to verify additional unforeseen edge-cases.

- How to verify it

1. Bring up an OpenShift 4.16 cluster.
2. Opt into on-cluster builds using either the docs within this repo or my onclustertesting helpers.
3. Opt the master pool into on-cluster builds by adding the appropriate label: $ oc label mcp/master 'machineconfiguration.openshift.io/layering-enabled='.
4. Only one control plane node should go offline at once.

- Description for the changelog
Consider currentImage / desiredImage annotations when determining if a node is unavailable

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 openshift-eng/jira-lifecycle-plugin repository.

[openshift-bot]

[ART PR BUILD NOTIFIER]

This PR has been included in build openshift-proxy-pull-test-container-v4.16.0-202402020041.p0.g523ea84.assembly.stream for distgit openshift-proxy-pull-test.
All builds following this will include this PR.

[openshift-merge-robot]

Fix included in accepted release 4.16.0-0.nightly-2024-02-02-224339