OCPBUGS-74514: OCPBUGS-74515: OCPBUGS-74516: OCPBUGS-74517: remove bootimage update feature gates#2733
OCPBUGS-74514: OCPBUGS-74515: OCPBUGS-74516: OCPBUGS-74517: remove bootimage update feature gates#2733djoshy wants to merge 1 commit intoopenshift:masterfrom
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
openshift-ci-robot
added
jira/valid-reference
|
@djoshy: This pull request references Jira Issue OCPBUGS-74514, 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. |
|
Hello @djoshy! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/XXL
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration 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 (5)
📒 Files selected for processing (14)
💤 Files with no reviewable changes (12)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis pull request removes the ManagedBootImages feature gate and its cloud-specific variants (ManagedBootImagesAWS, ManagedBootImagesAzure, ManagedBootImagesvSphere) from the codebase. Changes include deleting feature gate definitions from the feature registry, removing related documentation entries, updating CRD annotations and validation tags, and removing these gates from multiple feature gate configuration manifests across different cluster profiles. The ManagedBootImagesCPMS feature gate is retained as the primary gate for related functionality. The net effect is streamlining feature gate definitions by consolidating to the CPMS variant. 🚥 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 |
|
/jira refresh |
Review Summary by QodoRemove deprecated boot image feature gates and consolidate to ManagedBootImagesCPMS
WalkthroughsDescription• Remove four deprecated boot image feature gates - ManagedBootImages, ManagedBootImagesAWS, ManagedBootImagesAzure, ManagedBootImagesvSphere • Update validation rules to only require ManagedBootImagesCPMS • Move managedBootImages fields to ungated CRD manifests • Delete ManagedBootImages-specific CRD manifest file Diagramflowchart LR
A["Four Boot Image FGs<br/>ManagedBootImages<br/>ManagedBootImagesAWS<br/>ManagedBootImagesAzure<br/>ManagedBootImagesvSphere"] -->|Remove| B["Feature Gate Definitions"]
A -->|Remove| C["Feature Gate Annotations"]
A -->|Remove| D["Payload Manifests"]
B -->|Update| E["ManagedBootImagesCPMS Only"]
C -->|Move to Ungated| F["CRD Manifests"]
D -->|Simplify| G["Feature Gate Lists"]
File Changes1. features/features.go
|
Code Review by Qodo
1. managedBootImagesStatus omits optional behavior
|
openshift-ci-robot
added
jira/valid-bug
|
@djoshy: This pull request references Jira Issue OCPBUGS-74514, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| managedBootImagesStatus: | ||
| description: |- | ||
| managedBootImagesStatus reflects what the latest cluster-validated boot image configuration is | ||
| and will be used by Machine Config Controller while performing boot image updates. | ||
| properties: |
There was a problem hiding this comment.
1. managedbootimagesstatus omits optional behavior 📘 Rule violation ✓ Correctness
The new CRD schema description for managedBootImagesStatus does not document what it means when the optional field is omitted. This violates the requirement to document optional/omitted semantics for fields using kubebuilder optionality.
Agent Prompt
## Issue description
`ManagedBootImagesStatus` is optional but its documentation does not explain the meaning/behavior when the field is omitted.
## Issue Context
The generated CRD schema descriptions are derived from the Go type comments. Compliance requires optional fields to document omitted behavior.
## Fix Focus Areas
- operator/v1/types_machineconfiguration.go[288-301]
- operator/v1/zz_generated.featuregated-crd-manifests/machineconfigurations.operator.openshift.io/AAA_ungated.yaml[714-718]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // MachineManagerManagedResourceType is a string enum used in the MachineManager type to describe the resource | ||
| // type to be registered. | ||
| // +openshift:validation:FeatureGateAwareEnum:requiredFeatureGate=ManagedBootImages,enum=machinesets | ||
| // +openshift:validation:FeatureGateAwareEnum:requiredFeatureGate=ManagedBootImages;ManagedBootImagesCPMS,enum=machinesets;controlplanemachinesets | ||
| // +kubebuilder:validation:Enum:="machinesets" | ||
| // +openshift:validation:FeatureGateAwareEnum:requiredFeatureGate=ManagedBootImagesCPMS,enum=machinesets;controlplanemachinesets | ||
| type MachineManagerMachineSetsResourceType string |
There was a problem hiding this comment.
2. resource enum contradicts docs 📘 Rule violation ✓ Correctness
The resource field documentation says controlplanemachinesets is valid, but the ungated enum only allows machinesets. This creates a documentation/validation mismatch for API consumers.
Agent Prompt
## Issue description
The `resource` field claims `controlplanemachinesets` is a valid value, but the ungated validation enum only permits `machinesets`, creating a contract mismatch.
## Issue Context
This PR introduces `+kubebuilder:validation:Enum:="machinesets"` while retaining documentation (and generated CRD description) that lists both `machinesets` and `controlplanemachinesets`. If `controlplanemachinesets` is intended to be feature-gated by `ManagedBootImagesCPMS`, the documentation needs to explicitly state that; otherwise, the validation needs to allow it.
## Fix Focus Areas
- operator/v1/types_machineconfiguration.go[369-386]
- operator/v1/types_machineconfiguration.go[430-441]
- operator/v1/zz_generated.featuregated-crd-manifests/machineconfigurations.operator.openshift.io/AAA_ungated.yaml[736-744]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
/test integration |
|
PR-Agent: could not fine a component named |
|
/lgtm Assuming the integration failure was a flake |
|
Scheduling tests matching the |
|
[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
|
/test verify-client-go This one failed too, hmm |
|
PR-Agent: could not fine a component named |
Looks like this needs a change as well, this used to be a "double" feature gate test file, will rename and push shortly |
djoshy
force-pushed
the
remove-boot-image-fg
branch
from
304a865 to
1af9b9a
Compare
|
New changes are detected. LGTM label has been removed. |
3 participants
This removes references to remove all boot image update feature gates:
The MCO has removed all references to them in openshift/machine-config-operator#5718