OCPBUGS-74514: OCPBUGS-74515: OCPBUGS-74516: OCPBUGS-74517: remove bootimage update feature gates#2733
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 info⚙️ Run configurationConfiguration used: Repository YAML (base), Central YAML (inherited) Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (6)
📒 Files selected for processing (13)
💤 Files with no reviewable changes (12)
📝 WalkthroughWalkthroughThis pull request removes the ManagedBootImages feature gate and its cloud-specific variants (ManagedBootImagesAWS, ManagedBootImagesAzure, ManagedBootImagesvSphere) from feature registry, documentation, payload manifests, and test CRD manifests. It deletes the feature-gate declarations and their registrations, removes ManagedBootImages annotations from MachineConfiguration CRD types, updates validation tags to reference only ManagedBootImagesCPMS, and removes related rows from features.md. The ManagedBootImagesCPMS gate is retained. Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (1 error)
✅ Passed checks (4 passed)
✨ 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 |
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
|
/lgtm |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-gcp-ovn-serial periodic-ci-openshift-machine-config-operator-release-4.22-periodics-e2e-vsphere-mco-disruptive |
|
@djoshy: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/684e73a0-189d-11f1-877a-d5d8840c625e-0 |
|
/payload-job periodic-ci-openshift-release-main-nightly-4.22-e2e-gcp-ovn-serial periodic-ci-openshift-machine-config-operator-release-4.22-periodics-e2e-vsphere-mco-disruptive |
|
@djoshy: trigger 2 job(s) for the /payload-(with-prs|job|aggregate|job-with-prs|aggregate-with-prs) command
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/d1b7ac80-18a2-11f1-902d-bb7f24d3d556-0 |
|
/pipeline required |
|
Scheduling tests matching the |
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
/retest |
|
/verified by payloads payload runs look good, boot image tests are passing, failures are unrelated /retest |
openshift-ci-robot
added
the
verified
|
@djoshy: 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. |
|
/retest |
|
/retest-required |
djoshy
force-pushed
the
remove-boot-image-fg
branch
from
06eb322 to
902f351
Compare
openshift-ci-robot
removed
the
verified
|
/verified by payloads /pipeline required |
|
Scheduling tests matching the |
openshift-ci-robot
added
the
verified
|
@djoshy: 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. |
|
/lgtm |
|
Tests from second stage were triggered manually. Pipeline can be controlled only manually, until HEAD changes. Use command to trigger second stage. |
|
[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 |
|
@djoshy: 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. |
|
@djoshy: Jira Issue Verification Checks: Jira Issue OCPBUGS-74514 Jira Issue OCPBUGS-74514 has been moved to the MODIFIED state and will move to the VERIFIED state when the change is available in an accepted nightly payload. 🕓 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. |
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