OTA-1956: Skip ConsolePlugin manifests during CVO bootstrap#8839
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
the
jira/valid-reference
|
@jhadvig: This pull request references OTA-1956 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 the "5.0.0" version, but no target version was set. 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. |
jhadvig
force-pushed
the
skip-consoleplugin-bootstrap
branch
from
1e4b5e9 to
0354a8d
Compare
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Enterprise Run ID: ⛔ Files ignored due to path filters (5)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe payload preparation script now deletes 🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
openshift-ci
Bot
added
area/control-plane-operator
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go (1)
203-209: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a regression assertion for the new manifest cleanup.
TestPreparePayloadScriptalready checks the other generatedrm -fstatements, but this new*_consoleplugin.yamlremoval is not covered in the provided test cases. Adding one assertion there would lock in the bootstrap fix and catch future regressions.🤖 Prompt for 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. In `@control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go` around lines 203 - 209, Add a regression assertion in TestPreparePayloadScript to cover the new manifest cleanup generated by preparePayloadScript. Update the existing checks for the script-building logic in deployment.go so the test also verifies the rm -f statement removing *_consoleplugin.yaml alongside the other manifest cleanup commands. Ensure the assertion targets the same generated script content used for the existing *_deployment.yaml and *_servicemonitor.yaml checks.
🤖 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.
Nitpick comments:
In `@control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go`:
- Around line 203-209: Add a regression assertion in TestPreparePayloadScript to
cover the new manifest cleanup generated by preparePayloadScript. Update the
existing checks for the script-building logic in deployment.go so the test also
verifies the rm -f statement removing *_consoleplugin.yaml alongside the other
manifest cleanup commands. Ensure the assertion targets the same generated
script content used for the existing *_deployment.yaml and *_servicemonitor.yaml
checks.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 5f19ef6b-dc02-452b-8767-ba6b6d6569de
📒 Files selected for processing (1)
control-plane-operator/controllers/hostedcontrolplane/v2/cvo/deployment.go
|
/test ci/prow/verify-deps |
|
/test verify-deps |
| fmt.Sprintf("cp -R /manifests %s/", payloadDir), | ||
| fmt.Sprintf("rm -f %s/manifests/*_deployment.yaml", payloadDir), | ||
| fmt.Sprintf("rm -f %s/manifests/*_servicemonitor.yaml", payloadDir), | ||
| fmt.Sprintf("rm -f %s/manifests/*_consoleplugin.yaml", payloadDir), |
There was a problem hiding this comment.
While just removing the ConsolePlugin like this may be sufficient to recover HyperShift install in openshift/cluster-version-operator#1398, I'd expect us to be using %s/manifests/0000_50_cluster-update-console-plugin_* or similar to remove all of these:
cluster-version-operator$ git show --stat=200 origin/pr/1398 |grep install/
install/0000_50_cluster-update-console-plugin_10_namespace.yaml | 15 +++++++++++++++
install/0000_50_cluster-update-console-plugin_15_serviceaccount.yaml | 11 +++++++++++
install/0000_50_cluster-update-console-plugin_20_networkpolicy.yaml | 16 ++++++++++++++++
install/0000_50_cluster-update-console-plugin_30_configmap.yaml | 28 ++++++++++++++++++++++++++++
install/0000_50_cluster-update-console-plugin_50_deployment.yaml | 78 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
install/0000_50_cluster-update-console-plugin_60_service.yaml | 20 ++++++++++++++++++++
install/0000_50_cluster-update-console-plugin_90_consoleplugin.yaml | 21 +++++++++++++++++++++
install/image-references | 8 ++++++++The existing *_deployment.yaml would cover the plugin Deployment, but I'm not seeing anything that would cover the Namespace, etc. And I don't see a point to running anything cluster-update-console-plugin related on HyperShift (where folks launch updates via HostedCluster or some other higher-level API/UI, and do not use in-cluster consoles). Or a point to running anything cluster-update-console-plugin without a ConsolePlugin to get that Service wired up for console rendering.
There was a problem hiding this comment.
Good call, updated to 0000_50_cluster-update-console-plugin_* to remove all console plugin manifests. No point running any of them on HyperShift.
There was a problem hiding this comment.
IIUC, by removing these behavior does not actually change because the CVO took over the installation of these manifests. Is this correct?
There was a problem hiding this comment.
The behavior on current HyperShift is "there are no cluster-update console plugin resources", because cvo#1398 hasn't merged yet. With this HyperShift pull, even after cvo#1398 merges, and TechPreviewNoUpdate standalone clusters get the plugin resources, HyperShift will continue to not get the plugin resources. Because we're deleting those manifests with this HyperShift pull, so the HyperShift-run CVO will not know about them. Because cluster-update is not something HyperShift cluster-owners action from hosted-cluster, spoke-side APIs/UIs; instead, they action cluster updates via HostedCluster or higher-level, hub-ward APIs and UIs.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8839 +/- ##
=======================================
Coverage 43.02% 43.02%
=======================================
Files 766 766
Lines 94780 94781 +1
=======================================
+ Hits 40775 40776 +1
Misses 51186 51186
Partials 2819 2819
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
jhadvig
force-pushed
the
skip-consoleplugin-bootstrap
branch
from
0354a8d to
3d26691
Compare
The ConsolePlugin CRD (console.openshift.io/v1) is not available during HyperShift bootstrap — the console operator has not started yet. The CVO bootstrap init container gets stuck in an infinite retry loop trying to kubectl apply the ConsolePlugin manifest. Add *_consoleplugin.yaml to the wildcard removals alongside the existing *_deployment.yaml and *_servicemonitor.yaml patterns. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
jhadvig
force-pushed
the
skip-consoleplugin-bootstrap
branch
from
3d26691 to
24ac382
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: jhadvig, muraee 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 |
openshift-ci
Bot
added
the
approved
|
Scheduling tests matching the |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
Test Resultse2e-aws
e2e-aks
|
|
/retest |
AI Test Failure AnalysisJob: Generated by hypershift-analyze-e2e-failure post-step using Claude claude-opus-4-6 |
|
I now have all the evidence for both jobs. Here's the complete analysis: Test Failure Analysis CompleteJob InformationJob 1:
Job 2:
Test Failure AnalysisErrorSummaryBoth jobs failed with the identical root cause: the Root CausePre-existing bootstrap race condition in packageserver, not caused by PR #8839. The This race happens consistently across both AWS and AKS platforms:
The PR #8839 is unrelated. The PR only modifies the CVO bootstrap init container's Recommendations
Evidence
|
|
/retest |
|
/verified by CI, and we'll run even more CI on cvo#1398 |
openshift-ci-robot
added
the
verified
|
@wking: This PR has been marked as verified by 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. |
|
@jhadvig: all tests passed! 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. |
7 participants
Summary
The CVO bootstrap init container in HyperShift gets stuck in an infinite retry loop trying to
kubectl applytheConsolePluginmanifest (0000_50_cluster-update-console-plugin_90_consoleplugin.yaml). TheConsolePluginCRD (console.openshift.io/v1) is not available during bootstrap because the console operator hasn't started yet.Add
*_consoleplugin.yamlto the wildcard removals inpreparePayloadScript, alongside the existing*_deployment.yamland*_servicemonitor.yamlpatterns that are already removed for the same reason (CRDs not available during bootstrap).Root cause
From CVO PR #1398 CI logs:
The bootstrap init container retries this indefinitely, preventing the CVO pod from starting.
Related
Summary by CodeRabbit
0000_50_cluster-update-console-plugin_*from the copied/var/payload/manifests/content before the release workflow continues, preventing unintended console plugin resources from being processed.