OCPBUGS-65497: Add all managed resources to ClusterOperator relatedObjects#404
OCPBUGS-65497: Add all managed resources to ClusterOperator relatedObjects#404RadekManak wants to merge 2 commits into
Conversation
Add ClusterRole, ClusterRoleBinding, and ValidatingWebhookConfiguration to the ClusterOperator's relatedObjects to ensure they are collected by oc adm inspect and must-gather for debugging purposes.
Add serviceaccounts, services, deployments, roles, and rolebindings to the relatedObjects list so that oc adm inspect and must-gather collect the full set of resources managed by the operator.
|
@RadekManak: This pull request references Jira Issue OCPBUGS-65497, which is invalid:
Comment The bug has been updated to refer to the pull request using the external bug tracker. 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. |
WalkthroughThe PR expands the set of Kubernetes resources tracked as related objects for the control-plane-machine-set operator by adding eight new resource types (serviceaccounts, services, deployments, roles, rolebindings, clusterroles, clusterrolebindings, and validatingwebhookconfigurations) across both the Go implementation and the manifest declaration. ChangesExpand Related Objects for Control-Plane-Machine-Set Operator
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 11 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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.12.1)Error: can't load config: unsupported version of the configuration: "" See https://golangci-lint.run/docs/product/migration-guide for migration instructions Comment |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
pkg/controllers/controlplanemachineset/cluster_operator.go (1)
54-98: ⚡ Quick winAdd a parity test for the duplicated related-object list.
relatedObjects()is now a second hard-coded source of truth next tomanifests/0000_30_control-plane-machine-set-operator_04_clusteroperator.yaml. A small test that compares the two would catch future renames/additions before inspect or must-gather coverage drifts.🤖 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 `@pkg/controllers/controlplanemachineset/cluster_operator.go` around lines 54 - 98, Add a parity unit test that ensures the hard-coded relatedObjects() list matches the relatedObjects declared in the manifest file manifests/0000_30_control-plane-machine-set-operator_04_clusteroperator.yaml: write a test (e.g., controlplanemachineset_relatedobjects_test.go) that calls relatedObjects(), loads and parses the ClusterOperator YAML from the manifest file, extracts the relatedObjects entries (group, resource, name, namespace), normalizes both lists into comparable sets (ignore ordering), and fails if there is any difference so future renames/additions are caught automatically.
🤖 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 `@pkg/controllers/controlplanemachineset/cluster_operator.go`:
- Around line 54-98: Add a parity unit test that ensures the hard-coded
relatedObjects() list matches the relatedObjects declared in the manifest file
manifests/0000_30_control-plane-machine-set-operator_04_clusteroperator.yaml:
write a test (e.g., controlplanemachineset_relatedobjects_test.go) that calls
relatedObjects(), loads and parses the ClusterOperator YAML from the manifest
file, extracts the relatedObjects entries (group, resource, name, namespace),
normalizes both lists into comparable sets (ignore ordering), and fails if there
is any difference so future renames/additions are caught automatically.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: openshift/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: 9d39db61-8275-4aaa-84bb-b8115826240f
📒 Files selected for processing (2)
manifests/0000_30_control-plane-machine-set-operator_04_clusteroperator.yamlpkg/controllers/controlplanemachineset/cluster_operator.go
|
@RadekManak: 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. |
Adds all resources managed by the control-plane-machine-set operator to the ClusterOperator's relatedObjects list, ensuring
oc adm inspectand must-gather collect the complete set of resources needed for debugging.Resources added
Both the static manifest YAML and the Go source are kept in sync.
Summary by CodeRabbit