Skip to content

NO-ISSUE: Fix MCN Applied condition checks for TP and non-TP clusters#5796

Open
ptalgulk01 wants to merge 1 commit intoopenshift:mainfrom
ptalgulk01:fix-mcn-applied-condition-status
Open

NO-ISSUE: Fix MCN Applied condition checks for TP and non-TP clusters#5796
ptalgulk01 wants to merge 1 commit intoopenshift:mainfrom
ptalgulk01:fix-mcn-applied-condition-status

Conversation

@ptalgulk01
Copy link

  • Add dynamic condition checking to support both TechPreview and non-TechPreview clusters. The new GetAppliedConditionStatus() automatically checks the correct condition type based on cluster type.
  • Also improved VERSION_ID regex pattern for better version parsing.

@coderabbitai
Copy link

coderabbitai bot commented Mar 24, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fa70bd4a-0962-4c96-9893-c15763f78228

📥 Commits

Reviewing files that changed from the base of the PR and between a9870c3 and 915d537.

📒 Files selected for processing (3)
  • test/extended-priv/machineconfignode.go
  • test/extended-priv/mco_machineconfignode.go
  • test/extended-priv/node.go
🚧 Files skipped from review as they are similar to previous changes (1)
  • test/extended-priv/node.go

Walkthrough

Replaced MCN accessor GetAppliedFilesAndOS() with GetAppliedFiles() and added GetAppliedConditionStatus() which prefers the "AppliedFilesAndOS" status from .status.conditions and falls back to GetAppliedFiles(). Tests were updated to use the new helper. The RHEL version regex now accepts single or double quotes and restricts the capture to digits and dots.

Changes

Cohort / File(s) Summary
MCN Condition Accessors
test/extended-priv/machineconfignode.go
Removed GetAppliedFilesAndOS(); added GetAppliedFiles() (reads "AppliedFiles") and GetAppliedConditionStatus() which first reads the "AppliedFilesAndOS" condition status from .status.conditions via JSONPath and falls back to GetAppliedFiles() if empty or lookup fails.
MCN Test Updates
test/extended-priv/mco_machineconfignode.go
Replaced uses of GetAppliedFilesAndOS() with GetAppliedConditionStatus() in condition-transition and degraded-node tests; adjusted polling assertions to check Unknown at ~1m and True at ~3m.
RHEL Version Parsing
test/extended-priv/node.go
Updated Node.GetRHELVersion() regex to accept VERSION_ID values enclosed in single or double quotes and constrain the captured version to digits and dots ([0-9.]+).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 golangci-lint (2.11.3)

Command failed


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sergiordlr
Copy link
Contributor

/lgtm

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. label Mar 24, 2026
@ptalgulk01 ptalgulk01 force-pushed the fix-mcn-applied-condition-status branch from a9870c3 to 915d537 Compare March 24, 2026 13:10
@openshift-ci openshift-ci bot removed the lgtm Indicates that a PR is ready to be merged. label Mar 24, 2026
Copy link
Member

@isabella-janssen isabella-janssen 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 Mar 24, 2026
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 24, 2026

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: isabella-janssen, ptalgulk01, sergiordlr

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details 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 Mar 24, 2026
@isabella-janssen
Copy link
Member

/retitle NO-ISSUE: Fix MCN Applied condition checks for TP and non-TP clusters

/retest-required

@openshift-ci openshift-ci bot changed the title Fix MCN Applied condition checks for TP and non-TP clusters NO-ISSUE: Fix MCN Applied condition checks for TP and non-TP clusters Mar 24, 2026
@openshift-ci-robot openshift-ci-robot added the jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. label Mar 24, 2026
@openshift-ci-robot
Copy link
Contributor

@ptalgulk01: This pull request explicitly references no jira issue.

Details

In response to this:

  • Add dynamic condition checking to support both TechPreview and non-TechPreview clusters. The new GetAppliedConditionStatus() automatically checks the correct condition type based on cluster type.
  • Also improved VERSION_ID regex pattern for better version parsing.

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.

@isabella-janssen
Copy link
Member

/verified later by @isabella-janssen

This is a simple change to our tests that can be verified in payload runs after this merges.

@openshift-ci-robot
Copy link
Contributor

@isabella-janssen: Only users can be targets for the /verified later command.

Details

In response to this:

/verified later by @isabella-janssen

This is a simple change to our tests that can be verified in payload runs after this merges.

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.

@isabella-janssen
Copy link
Member

/verified later @isabella-janssen

@openshift-ci-robot openshift-ci-robot added verified-later verified Signifies that the PR passed pre-merge verification criteria labels Mar 25, 2026
@openshift-ci-robot
Copy link
Contributor

@isabella-janssen: This PR has been marked to be verified later by @isabella-janssen.

Details

In response to this:

/verified later @isabella-janssen

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.

@isabella-janssen
Copy link
Member

/retest-required

@isabella-janssen
Copy link
Member

/override ci/prow/e2e-hypershift

This PR does not impact hypershift

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 25, 2026

@isabella-janssen: Overrode contexts on behalf of isabella-janssen: ci/prow/e2e-hypershift

Details

In response to this:

/override ci/prow/e2e-hypershift

This PR does not impact hypershift

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.

@openshift-ci
Copy link
Contributor

openshift-ci bot commented Mar 26, 2026

@ptalgulk01: 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-gcp-op-ocl 915d537 link false /test e2e-gcp-op-ocl
ci/prow/e2e-gcp-op-part2 915d537 link unknown /test e2e-gcp-op-part2
ci/prow/e2e-gcp-op-part1 915d537 link unknown /test e2e-gcp-op-part1

Full PR test history. Your PR dashboard.

Details

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.

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. jira/valid-reference Indicates that this PR references a valid Jira ticket of any type. lgtm Indicates that a PR is ready to be merged. verified Signifies that the PR passed pre-merge verification criteria verified-later

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants