WIP - SNO Stabilization - #6362
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci
Bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughChangesImage-mode upgrade flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ImageModeTest
participant MachineConfigNode
participant MachineConfigPool
participant UpgradeMonitor
UpgradeMonitor->>MachineConfigNode: Compare desired image and build SSA payload
ImageModeTest->>MachineConfigNode: Validate transition conditions
MachineConfigNode-->>ImageModeTest: Return condition state
ImageModeTest->>MachineConfigPool: Poll configuration and node counts
MachineConfigPool-->>ImageModeTest: Return convergence status
ImageModeTest->>MachineConfigPool: Wait for Updated after MOSC removal
MachineConfigPool-->>ImageModeTest: Return updated pool state
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error, 1 warning)
✅ Passed checks (13 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-single-node-disruptive-techpreview-1of3 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-single-node-disruptive-techpreview-2of3 periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-single-node-disruptive-techpreview-3of3 |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
@isabella-janssen: trigger 4 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/52cacfc0-8f43-11f1-8f85-ba7639252e8b-0 |
openshift-ci
Bot
added
the
approved
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/upgrademonitor/upgrade_monitor.go`:
- Around line 535-552: Update the no-diff guard in the MCN reconciliation flow
to compare mcNode.Spec.ConfigImage.DesiredImage directly with
newMCNode.Spec.ConfigImage.DesiredImage, without gating that comparison on
FeatureGateImageModeStatusReporting. Preserve the existing SSA payload behavior
in specApplyConfig so the empty desired image transition proceeds and clears the
field.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 14f15b7d-2eb7-470e-9340-4cd22e57aea8
📒 Files selected for processing (4)
pkg/upgrademonitor/upgrade_monitor.gotest/extended/image_mode_status_reporting.gotest/extended/machineconfignode.gotest/extended/machineconfigpool.go
|
/test tls-pqc-readiness |
|
/payload abort |
|
@isabella-janssen: it appears that you have attempted to use some version of the payload command, but your comment was incorrectly formatted and cannot be acted upon. See the docs for usage info. |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-single-node-disruptive-techpreview |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-disruptive-techpreview |
|
@isabella-janssen: trigger 4 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/efaf7890-8f61-11f1-92ec-e6c7fe0a00e3-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview |
|
@isabella-janssen: trigger 3 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/fed3d050-8f61-11f1-9417-2878328269f2-0 |
|
@isabella-janssen: trigger 1 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/0363dfc0-8f62-11f1-8d53-5a432a6811e0-0 |
7dc2156 to
1e3957e
Compare
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-single-node-disruptive-techpreview |
|
@isabella-janssen: trigger 4 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/ea062750-9019-11f1-9868-58c818aa0ee9-0 |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-disruptive-techpreview |
|
@isabella-janssen: trigger 3 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/ef5fa140-9019-11f1-9446-fc8702133699-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview |
|
@isabella-janssen: trigger 1 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/f367bc00-9019-11f1-848b-36ee69c9d173-0 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended/image_mode_status_reporting.go`:
- Around line 423-449: The polling callback around
mcnAndNodeAnnotationMachineCountsMatch must distinguish non-transient mcpErr or
nodesErr failures from count mismatches. Propagate the helper’s non-retryable
error state and call o.StopTrying before entering the mismatch-deadline logic,
while retaining retries only for connection errors and genuine count mismatches.
Ensure all returned errors are explicitly handled rather than ignored across the
additionally affected polling paths.
- Around line 465-469: Update the MCP error logging in the transient and
non-transient branches around isTransientConnectionError, including the
corresponding GetNodesByRole path, to avoid logging raw mcpErr values. Log only
stable error categories or use a redacted error representation that excludes
request URLs and transport details, while preserving the existing retry
classification and return behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 2b99dd5b-c9c0-4bd9-a6c0-3f906a841502
📒 Files selected for processing (4)
pkg/upgrademonitor/upgrade_monitor.gotest/extended/image_mode_status_reporting.gotest/extended/machineconfignode.gotest/extended/machineconfigpool.go
🚧 Files skipped from review as they are similar to previous changes (3)
- test/extended/machineconfignode.go
- test/extended/machineconfigpool.go
- pkg/upgrademonitor/upgrade_monitor.go
|
/payload-aggregate periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview 5 |
|
@isabella-janssen: trigger 1 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/2c32cc80-90d0-11f1-91c8-d3817e296d5e-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview |
|
@isabella-janssen: trigger 1 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/b68f04d0-910b-11f1-8c6d-361e78c7ce23-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview |
|
@isabella-janssen: trigger 1 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/8eec6c80-9131-11f1-93ca-c14894c258fd-0 |
…en/disrutive-suite-stabilization""
1e3957e to
02857f7
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-disruptive-techpreview |
|
@isabella-janssen: trigger 3 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/ca320d00-91b2-11f1-8e66-7fde7a253def-0 |
|
/payload-job periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview |
|
@isabella-janssen: trigger 1 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/dcb60a80-91b2-11f1-86c4-39d6063a608f-0 |
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-single-node-disruptive-techpreview |
|
@isabella-janssen: trigger 3 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/e1335540-91b2-11f1-8159-b01181677f35-0 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@test/extended/image_mode_status_reporting.go`:
- Around line 423-430: In the mismatch-tracking callback around
mcnAndNodeAnnotationMachineCountsMatch, reset firstMismatchTime to its zero
value whenever isConnErr is true before returning false. Preserve the existing
retry behavior so mismatchDeadline applies only to uninterrupted count
mismatches while the outer Eventually timeout continues bounding
connection-error retries.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: b9a88149-03fa-4678-aabc-a531fa8045b8
📒 Files selected for processing (4)
pkg/upgrademonitor/upgrade_monitor.gotest/extended/image_mode_status_reporting.gotest/extended/machineconfignode.gotest/extended/machineconfigpool.go
🚧 Files skipped from review as they are similar to previous changes (3)
- test/extended/machineconfigpool.go
- pkg/upgrademonitor/upgrade_monitor.go
- test/extended/machineconfignode.go
02857f7 to
4658a49
Compare
|
/payload-job periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-single-node-disruptive-techpreview periodic-ci-openshift-release-main-ci-5.0-e2e-aws-ovn-techpreview periodic-ci-openshift-machine-config-operator-release-5.0-periodics-e2e-aws-mco-disruptive-techpreview |
|
@isabella-janssen: trigger 7 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/5f0d35e0-91b7-11f1-93cb-d446440f7f9a-0 |
1 participant
- What I did
- How to verify it
- Description for the changelog
Summary by CodeRabbit