MCO-2025: OCP-88366 and add OCP-88814 for osImageStream with osImageURL#6014
MCO-2025: OCP-88366 and add OCP-88814 for osImageStream with osImageURL#6014HarshwardhanPatil07 wants to merge 7 commits intoopenshift:mainfrom
Conversation
… is set Migrated from openshift-tests-private PR #29780. Verifies that status.osImageStream is not reported in the worker MCP when a custom osImageURL is configured, covering update-in-progress, post-update, and degraded-pool scenarios. Signed-off-by: HarshwardhanPatil07 <harshpat@redhat.com>
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@HarshwardhanPatil07: No Jira issue with key OCP-88366 exists in the tracker at https://redhat.atlassian.net. DetailsIn 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. |
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository: openshift/coderabbit/.coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (1)
WalkthroughAdds two skipped Ginkgo tests validating MCP ChangesosImageStream tests + MCP status accessor
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 10 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (10 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 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. Comment |
|
@HarshwardhanPatil07: No Jira issue with key OCP-88366 exists in the tracker at https://redhat.atlassian.net. DetailsIn 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. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
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-priv/mco_osimagestream.go`:
- Around line 238-246: The test degrades an MCP but only defers
degradedMC.DeleteWithWait(), which removes the bad MachineConfig but doesn't
wait for the pool to reconcile; add a defer call to mcp.waitForComplete()
(registered before the spec restore/deletion so it runs last, e.g. defer
mcp.waitForComplete() then defer mcp.SetSpec(mcp.GetSpecOrFail()) / defer
degradedMC.DeleteWithWait()) to ensure the MCP pool has returned to healthy
state before teardown completes.
- Around line 279-285: The test has a race because mcp.GetUpdatingStatus is
polled with Eventually and then mcp.GetStatusOsImageStream is checked once with
Expect; the MCP may leave Updating=True before that single check. Replace the
two-step check by polling both conditions atomically inside a single Eventually
call (e.g., pass a closure that reads mcp.GetUpdatingStatus and
mcp.GetStatusOsImageStream and returns a boolean/error) and assert that Updating
== True AND status.osImageStream is empty within the same poll; update the code
that currently calls Eventually(mcp.GetUpdatingStatus) and
Expect(mcp.GetStatusOsImageStream()) to use this single Eventually that verifies
both conditions together.
🪄 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: 52c4fd72-9001-4247-98e7-3f2374e4b4fb
📒 Files selected for processing (1)
test/extended-priv/mco_osimagestream.go
The jsonpath was returning the entire osImageStream struct as JSON
(e.g. {"name":"rhel-9"}) instead of just the name string ("rhel-9"),
causing BeEmpty() assertions to fail when the field was absent but
the struct was serialized as an empty object.
|
@HarshwardhanPatil07: No Jira issue with key OCP-88366 exists in the tracker at https://redhat.atlassian.net. DetailsIn 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. |
OCP-88366 now only validates osImageStream is empty when a valid osImageURL is applied. The degraded pool scenario is moved to a new test OCP-88814 which uses an invalid osImageURL to trigger degradation instead of an invalid ignition version, and verifies osImageStream recovers to rhel-9 after cleanup.
OTP label removed from test 88366, MCO-2025 prefix removed from both test titles as it belongs in the PR title only.
|
/retitle MCO-2025: OCP-88366 and add OCP-88814 for osImageStream with osImageURL |
openshift-ci
Bot
changed the title
|
@HarshwardhanPatil07: This pull request references MCO-2025 which is a valid jira issue. Warning: The referenced jira issue has an invalid target version for the target branch this PR targets: expected the story to target either version "5.0." or "openshift-5.0.", but it targets "4.22.0" instead. DetailsIn 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-robot
added
the
jira/valid-reference
After deleting the invalid MC, wait for the pool to stop being degraded before calling waitForComplete, which fails immediately on degraded pools.
…id osImageURL Invalid osImageURL causes deep node degradation with slow recovery. Switch to IGNITION_VERSION=99.99.0 which degrades the pool without attempting an OS pivot, allowing reliable cleanup with DeleteWithWait.
The MCO clears status.osImageStream when the pool is degraded, regardless of whether osImageURL is set. Updated assertion from ContainSubstring(rhel-9) to BeEmpty().
|
/lgtm |
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: HarshwardhanPatil07, sergiordlr The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@HarshwardhanPatil07: The following test failed, say
Full PR test history. Your PR dashboard. DetailsInstructions 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
Migrated from openshift-tests-private PR #29780. Verifies that status.osImageStream is not reported in the worker MCP when a custom osImageURL is configured, covering update-in-progress, post-update, and degraded-pool scenarios.
- What I did
- How to verify it
- Description for the changelog
Summary by CodeRabbit