ClusterAPI revision components can be empty#2728
ClusterAPI revision components can be empty#2728mdbooth wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
📝 WalkthroughWalkthroughThe ClusterAPIInstallerRevision.Components field was changed from required to optional; its comment was expanded to state that omitting the field results in no components being installed and to retain the installation-order note. Generated API documentation and the CRD were updated to reflect optionality. Validation constraints for the field (MinItems=1 and MaxItems=32) remain present. 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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.5.0)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented Comment |
|
Hello @mdbooth! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/S
| // +required | ||
| // +optional | ||
| // +listType=atomic | ||
| // +kubebuilder:validation:MinItems=1 |
There was a problem hiding this comment.
Please put this back. We don't want an unstructured client to be able to set components: [] since a structured client cannot do this, it would create something that doesn't round trip.
| // revision. Components will be installed in the order they are listed. | ||
| // components is a list of components which will be installed by this | ||
| // revision. Components will be installed in the order they are listed. An | ||
| // empty or omitted list means no components will be installed. |
There was a problem hiding this comment.
Just omitted, not empty, we don't want to create a difference between omitted and empty and we don't want folks to write components: [] explicitly out
A revision with no components is actually a useful construct. It explicitly declares that the revision is not managing any components, or their manifests. The installer should not install anything, and should remove any components associated with an older revision.
mdbooth
force-pushed
the
capio-empty-revision
branch
from
db8a25b to
74577dd
Compare
|
/lgtm |
![coderabbitai[bot]](https://avatars.githubusercontent.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 the current code and only fix it if needed.
Inline comments:
In `@operator/v1alpha1/zz_generated.swagger_doc_generated.go`:
- Line 174: Update the source comment for the Components field in
operator/v1alpha1/types_clusterapi.go to add the missing comma so the sentence
reads "If omitted, no components will be installed." (reference the Components
field / components description in that file), then re-run the swagger/doc
generator to regenerate operator/v1alpha1/zz_generated.swagger_doc_generated.go
so the corrected punctuation appears in the generated output.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to data retention organization setting
⛔ Files ignored due to path filters (3)
openapi/generated_openapi/zz_generated.openapi.gois excluded by!openapi/**operator/v1alpha1/zz_generated.crd-manifests/0000_30_cluster-api_01_clusterapis.crd.yamlis excluded by!**/zz_generated.crd-manifests/*operator/v1alpha1/zz_generated.featuregated-crd-manifests/clusterapis.operator.openshift.io/ClusterAPIMachineManagement.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**
📒 Files selected for processing (3)
operator/v1alpha1/types_clusterapi.gooperator/v1alpha1/zz_generated.swagger_doc_generated.gopayload-manifests/crds/0000_30_cluster-api_01_clusterapis.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (2)
- operator/v1alpha1/types_clusterapi.go
- payload-manifests/crds/0000_30_cluster-api_01_clusterapis.crd.yaml
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: JoelSpeed 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 |
ⓘ Your monthly quota for Qodo has expired. Upgrade your plan ⓘ Paying users. Check that your Qodo account is linked with this Git user account |
|
/retest-required |
|
/retest |
|
/test e2e-aws-serial-techpreview-1of2 |
ⓘ Your monthly quota for Qodo has expired. Upgrade your plan ⓘ Paying users. Check that your Qodo account is linked with this Git user account |
|
@mdbooth: 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. |
3 participants
A revision with no components is actually a useful construct. It explicitly declares that the revision is not managing any components, or their manifests. The installer should not install anything, and should remove any components associated with an older revision.