OCPBUGS-64825: Add MachineConfigNode informer to trigger MCP machine count syncs #5422
Conversation
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: isabella-janssen 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 |
openshift-ci
bot
added
the
approved
isabella-janssen
force-pushed
the
ocpbugs-64825
branch
from
7fae2f5 to
6dc06b0
Compare
|
/payload-job periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-1of2 periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-2of2 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-1of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-2of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-3of3 |
|
@isabella-janssen: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/1a8c0660-c4be-11f0-8d30-0331b62d98be-0 |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-1of2 periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-2of2 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-1of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-2of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-3of3 |
|
@isabella-janssen: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/c463a790-c556-11f0-8f5e-89314842e1fc-0 |
isabella-janssen
force-pushed
the
ocpbugs-64825
branch
2 times, most recently
from
768b0fc to
df7c4e1
Compare
|
/payload-job periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-1of2 periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-2of2 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-1of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-2of3 periodic-ci-openshift-release-master-ci-4.21-e2e-aws-ovn-techpreview-serial-3of3 |
|
@isabella-janssen: trigger 5 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/f4ffb360-c571-11f0-80ff-b3f773b50dbd-0 |
isabella-janssen
changed the title
openshift-ci-robot
added
jira/valid-reference
|
@isabella-janssen: This pull request references Jira Issue OCPBUGS-64825, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. In 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 openshift-eng/jira-lifecycle-plugin repository. |
|
/jira refresh |
openshift-ci-robot
added
jira/valid-bug
|
@isabella-janssen: This pull request references Jira Issue OCPBUGS-64825, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: In 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 openshift-eng/jira-lifecycle-plugin repository. |
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
isabella-janssen
force-pushed
the
ocpbugs-64825
branch
from
df7c4e1 to
a75f97b
Compare
isabella-janssen
force-pushed
the
ocpbugs-64825
branch
from
35be309 to
0e0599e
Compare
djoshy
left a comment
djoshy
left a comment
There was a problem hiding this comment.
Generally looks good, would adding unit test(s) for this case be viable?
@djoshy I was considering adding unit tests but did not for two reasons:
I think unit tests would be a good idea because signal from (1) takes a lot longer than the feedback from unit tests, but I'd need to better understand how to handle feature gate "enablement" in unit tests to remediate (2). Do you have an example that would help solve (2)? |
We have a few unit tests that initializes controllers with feature gates enabled. I don't think we need to necessarily try to test the informer reaction since that's pretty standard(although that would be great if we want to add a general unit test for the syncs), I was thinking something more surgical in |
|
@isabella-janssen: The following tests failed, say
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-sigs/prow repository. I understand the commands that are listed here. |
3 participants
Closes: OCPBUGS-64825
- What I did
This adds a MachineConfigNode informer in the node controller so that changes to a node's MCN trigger syncs of the MachineConfigPool machine counts. This change is necessary with the introduction of #5141, which calculates the updated and degraded machine counts with MCN conditions instead of node annotations.
- How to verify it
This can be tested locally by launching a 4.21 tech preview cluster with this PR included. It can also be tested by ensuring that all existing
MachineConfigNodeandImageModeStatusReportingpass--specifically theShould properly report MCN conditions on node degradetest mentioned in the attached bug.- Description for the changelog
OCPBUGS-64825: Add MachineConfigNode informer to trigger MCP machine count syncs