OCPBUGS-74513: OCPBUGS-74519: OCPBUGS-78487: Remove MachineConfigNodes and PinnedImages feature gates and v1alpha1 references#2715
Conversation
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
|
@isabella-janssen: This pull request references Jira Issue OCPBUGS-74513, which is valid. 3 validation(s) were run on this bug
Requesting review from QA contact: 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. |
|
Important Review skippedAuto reviews are limited based on label configuration. 🚫 Review skipped — only excluded labels are configured. (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis pull request removes two feature gates—MachineConfigNodes and PinnedImages—from the codebase. The removal includes deleting the feature gate definitions from the Go code, removing them from multiple feature gate manifest files across different platform configurations, and deleting feature gate annotations from API type definitions. Additionally, the v1alpha1 API definitions for MachineConfigNode and PinnedImageSet are deleted entirely, along with their test files and registration entries. Feature gate references are also cleaned up from CRD test manifests and generated manifests. 🚥 Pre-merge checks | ✅ 7 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Comment |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/6aa05d40-0c44-11f1-9ce4-4d5ce96c4ea8-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/843dfb40-0c44-11f1-9e48-755722ba80a8-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/4e394a80-0cfe-11f1-9008-fbd59384bacd-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/524f9610-0cfe-11f1-8b6a-9e13349dc0d7-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5fc92400-0cfe-11f1-9bf2-4de395b04ad1-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/5da83fd0-0cfe-11f1-8b6b-9146d3917b7a-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/633fd430-0cfe-11f1-96e2-6f48c960f3ec-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/7b4b40c0-0dbf-11f1-8257-094951812dc9-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/7da906e0-0dbf-11f1-8e35-60e130204d6c-0 |
|
@isabella-janssen: This PR was included in a payload test run from openshift/machine-config-operator#5609
See details on https://pr-payload-tests.ci.openshift.org/runs/ci/804c8890-0dbf-11f1-95a3-c5ba819bee74-0 |
e875633 to
7e5f270
Compare
|
/lgtm Changes here are good when you're ready for this to go |
|
Scheduling tests matching the |
|
/retest-required |
ⓘ You are approaching your monthly quota for Qodo. Upgrade your plan Review Summary by QodoRemove MachineConfigNodes and PinnedImages feature gates
WalkthroughsDescription• Remove MachineConfigNodes feature gate definition and references • Remove PinnedImages feature gate definition and references • Update feature gate manifest files across all cluster profiles • Fix indentation inconsistencies in feature gate definitions Diagramflowchart LR
A["Feature Gate Definitions"] -->|Remove MachineConfigNodes| B["Updated features.go"]
A -->|Remove PinnedImages| B
B -->|Update documentation| C["features.md"]
B -->|Update manifests| D["Manifest Files"]
D -->|8 cluster profile variants| E["Cleaned Feature Gates"]
File Changes1. features/features.go
|
Code Review by Qodo
1. MachineConfigNodes CRDs filtered out
|
| { | ||
| "name": "MachineConfigNodes" | ||
| }, | ||
| { | ||
| "name": "ManagedBootImagesCPMS" | ||
| }, |
There was a problem hiding this comment.
1. Machineconfignodes crds filtered out 🐞 Bug ✓ Correctness
MachineConfigNodes is removed from the rendered FeatureGate enabled list, but MachineConfigNode CRDs/types are still annotated as gated on MachineConfigNodes, so manifest-merge will exclude those manifests for every FeatureSet/profile. This will change generated CRD outputs (and likely fail verify-codegen-crds) and can drop MachineConfigNode gated content from rendered payloads.
Agent Prompt
### Issue description
`MachineConfigNodes` was removed from the payload FeatureGate enabled lists, but some CRDs/types are still marked with `+openshift:enable:FeatureGate=MachineConfigNodes` and generated partial manifests still carry `feature-gate.release.openshift.io/MachineConfigNodes`. The manifest-merge generator filters gated manifests by the enabled feature gates from payload FeatureGate YAMLs, so these inputs become unselectable and will change generated outputs / fail `--verify`.
### Issue Context
The intent of this PR is to remove the feature gate entirely (i.e., feature becomes unconditional). That requires removing the API/CRD gating markers as well and regenerating the derived manifests.
### Fix Focus Areas
- machineconfiguration/v1alpha1/types_machineconfignode.go[10-20]
- machineconfiguration/v1/types_machineconfignode.go[10-20]
- machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/machineconfignodes.machineconfiguration.openshift.io/MachineConfigNodes.yaml[4-13]
- tools/codegen/pkg/manifestmerge/filters.go[440-475]
After removing the gating markers, run the repo’s CRD codegen update (the same generators used by `hack/verify-codegen-crds.sh`) and commit the updated generated outputs (including any featuregated test cases that still mention `MachineConfigNodes`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
756044c to
9558291
Compare
|
/test verify |
|
Needs a rebase to fix the conflict, but otherwise LGTM |
9558291 to
81c614a
Compare
|
/payload-job periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-1of2 periodic-ci-openshift-machine-config-operator-release-4.21-periodics-e2e-aws-mco-disruptive-techpreview-2of2 periodic-ci-openshift-release-main-ci-4.22-e2e-aws-ovn-techpreview-serial-1of3 periodic-ci-openshift-release-main-ci-4.22-e2e-aws-ovn-techpreview-serial-2of3 periodic-ci-openshift-release-main-ci-4.22-e2e-aws-ovn-techpreview-serial-3of3 For verification |
|
@isabella-janssen: trigger 5 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/a37b9e80-213f-11f1-8e6e-842d08329e23-0 |
|
@isabella-janssen: trigger 5 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/ab5e8270-213f-11f1-8424-3fec109cc418-0 |
|
/lgtm |
|
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 |
|
/verified by payloads Payload rehearsals look good; the failure in |
|
@isabella-janssen: 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-required |
|
@isabella-janssen: 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. |
|
@isabella-janssen: Jira Issue Verification Checks: Jira Issue OCPBUGS-74513 Jira Issue OCPBUGS-74513 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. |
since it has been removed in openshift/api#2715 and no longer required
since it has been removed in openshift/api#2715 and no longer required
This removes the
MachineConfigNodesandPinnedImagesfeature gates as well as the v1alpha1 APIs for each, which were replaced with v1 APIs in 4.19 (#2255 & #2198).