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 |
openshift-ci-robot
added
jira/valid-reference
|
@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. |
|
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 (14)
📒 Files selected for processing (23)
💤 Files with no reviewable changes (18)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughRemoved two OpenShift feature gates, MachineConfigNodes and PinnedImages. Their feature-gate variables and entries were removed from features/features.go and features.md, and those gates were deleted from multiple payload featureGate YAMLs. Feature-gate annotations and gated entries were removed from machineconfiguration v1 type files and CRD test YAMLs. Entire v1alpha1 API type files and tests for MachineConfigNode and PinnedImageSet were deleted and those types were removed from machineconfiguration/v1alpha1/register.go. No other control-flow changes were made. 🚥 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
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.11.3)Error: build linters: unable to load custom analyzer "kubeapilinter": tools/_output/bin/kube-api-linter.so, plugin: not implemented 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 |
isabella-janssen
force-pushed
the
ocpbugs-74513
branch
from
e875633 to
7e5f270
Compare
openshift-ci
bot
added
the
size/M
|
/lgtm Changes here are good when you're ready for this to go |
|
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
|
/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
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
| { | ||
| "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
| { | ||
| "name": "OpenShiftPodSecurityAdmission" | ||
| }, | ||
| { |
There was a problem hiding this comment.
2. Pinnedimages crds filtered out 🐞 Bug ✓ Correctness
PinnedImages is removed from the rendered FeatureGate enabled list, but several APIs/CRD partial manifests are still gated on PinnedImages, 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 pinned-images related schema/CRDs from rendered payloads.
Agent Prompt
### Issue description
The `PinnedImages` feature gate was removed from the payload FeatureGate enabled lists, but several APIs/CRD partial manifests are still gated on `PinnedImages` via `+openshift:enable:FeatureGate=PinnedImages` and `feature-gate.release.openshift.io/PinnedImages`. The manifest-merge generator filters gated manifests by the enabled feature gates from payload FeatureGate YAMLs, so these manifests become unselectable and will change generated outputs / fail `--verify`.
### Issue Context
If `PinnedImages` is intended to be always-on, remove the gating markers and regenerate the CRD outputs so the schema/CRDs are no longer conditioned on a non-existent feature gate.
### Fix Focus Areas
- machineconfiguration/v1alpha1/types_pinnedimageset.go[10-20]
- machineconfiguration/v1/types.go[440-548]
- machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/pinnedimagesets.machineconfiguration.openshift.io/PinnedImages.yaml[4-13]
- machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineconfigpools.machineconfiguration.openshift.io/PinnedImages.yaml[4-12]
- tools/codegen/pkg/manifestmerge/filters.go[440-475]
After removing the gating markers, run the CRD generators invoked by `hack/verify-codegen-crds.sh` and commit updated generated files (including any featuregated tests that still list `PinnedImages`).
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
isabella-janssen
force-pushed
the
ocpbugs-74513
branch
from
7e5f270 to
25e4204
Compare
|
New changes are detected. LGTM label has been removed. |
openshift-ci
bot
added
size/XXL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
machineconfiguration/v1alpha1/tests/pinnedimagesets.machineconfiguration.openshift.io/PinnedImages.yaml (1)
1-3:⚠️ Potential issue | 🟡 MinorRemove the stale
[TechPreview]prefix.This manifest is no longer gated in the header, so the title should stop describing it as TechPreview as well.
Suggested fix
-name: "[TechPreview] PinnedImageSet" +name: "PinnedImageSet"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@machineconfiguration/v1alpha1/tests/pinnedimagesets.machineconfiguration.openshift.io/PinnedImages.yaml` around lines 1 - 3, The manifest's metadata "name" field currently contains the stale prefix "[TechPreview] PinnedImageSet"; remove that prefix so the name reads "PinnedImageSet" (ensure the entry originally shown as name: "[TechPreview] PinnedImageSet" is updated) and keep crdName: pinnedimagesets.machineconfiguration.openshift.io and apiVersion unchanged; this aligns the title with the manifest's non-gated status and avoids the outdated TechPreview label.machineconfiguration/v1/tests/machineconfigpools.machineconfiguration.openshift.io/PinnedImages.yaml (1)
1-3:⚠️ Potential issue | 🟡 MinorDrop the stale
[TechPreview]label.The feature-gate header was removed here, so this manifest title now advertises the schema as gated when it no longer is. Please rename it to avoid misleading generated output and future readers.
Suggested fix
-name: "[TechPreview] MachineConfigPool" +name: "MachineConfigPool"🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@machineconfiguration/v1/tests/machineconfigpools.machineconfiguration.openshift.io/PinnedImages.yaml` around lines 1 - 3, The manifest's resource title still uses the stale label "name: \"[TechPreview] MachineConfigPool\"" which is misleading after the feature-gate header was removed; update the name value to remove the "[TechPreview]" prefix (e.g., "MachineConfigPool") so the manifest title matches the ungated schema, ensuring the crdName (machineconfigpools.machineconfiguration.openshift.io) remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In
`@machineconfiguration/v1/tests/machineconfigpools.machineconfiguration.openshift.io/PinnedImages.yaml`:
- Around line 1-3: The manifest's resource title still uses the stale label
"name: \"[TechPreview] MachineConfigPool\"" which is misleading after the
feature-gate header was removed; update the name value to remove the
"[TechPreview]" prefix (e.g., "MachineConfigPool") so the manifest title matches
the ungated schema, ensuring the crdName
(machineconfigpools.machineconfiguration.openshift.io) remains unchanged.
In
`@machineconfiguration/v1alpha1/tests/pinnedimagesets.machineconfiguration.openshift.io/PinnedImages.yaml`:
- Around line 1-3: The manifest's metadata "name" field currently contains the
stale prefix "[TechPreview] PinnedImageSet"; remove that prefix so the name
reads "PinnedImageSet" (ensure the entry originally shown as name:
"[TechPreview] PinnedImageSet" is updated) and keep crdName:
pinnedimagesets.machineconfiguration.openshift.io and apiVersion unchanged; this
aligns the title with the manifest's non-gated status and avoids the outdated
TechPreview label.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Central YAML (inherited)
Review profile: CHILL
Plan: Pro
Run ID: 430cef31-8993-428b-b32d-0c33951e607b
⛔ Files ignored due to path filters (9)
machineconfiguration/v1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineconfignodes.machineconfiguration.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineconfigpools.machineconfiguration.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineconfigpools.machineconfiguration.openshift.io/OSStreams.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/machineconfigpools.machineconfiguration.openshift.io/PinnedImages.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1/zz_generated.featuregated-crd-manifests/pinnedimagesets.machineconfiguration.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests.yamlis excluded by!**/zz_generated*machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/machineconfignodes.machineconfiguration.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**machineconfiguration/v1alpha1/zz_generated.featuregated-crd-manifests/pinnedimagesets.machineconfiguration.openshift.io/AAA_ungated.yamlis excluded by!**/zz_generated.featuregated-crd-manifests/**
📒 Files selected for processing (22)
features.mdfeatures/features.gomachineconfiguration/v1/tests/machineconfignodes.machineconfiguration.openshift.io/IrreconcilableMachineConfig.yamlmachineconfiguration/v1/tests/machineconfignodes.machineconfiguration.openshift.io/MachineConfigNodes.yamlmachineconfiguration/v1/tests/machineconfignodes.machineconfiguration.openshift.io/NoRegistryClusterInstall.yamlmachineconfiguration/v1/tests/machineconfigpools.machineconfiguration.openshift.io/PinnedImages.yamlmachineconfiguration/v1/tests/pinnedimagesets.machineconfiguration.openshift.io/PinnedImages.yamlmachineconfiguration/v1/types.gomachineconfiguration/v1/types_machineconfignode.gomachineconfiguration/v1/types_pinnedimageset.gomachineconfiguration/v1alpha1/tests/machineconfignodes.machineconfiguration.openshift.io/MachineConfigNodes.yamlmachineconfiguration/v1alpha1/tests/pinnedimagesets.machineconfiguration.openshift.io/PinnedImages.yamlmachineconfiguration/v1alpha1/types_machineconfignode.gomachineconfiguration/v1alpha1/types_pinnedimageset.gopayload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-DevPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yamlpayload-manifests/featuregates/featureGate-4-10-Hypershift-TechPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-DevPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yamlpayload-manifests/featuregates/featureGate-4-10-SelfManagedHA-TechPreviewNoUpgrade.yaml
💤 Files with no reviewable changes (17)
- machineconfiguration/v1alpha1/types_pinnedimageset.go
- payload-manifests/featuregates/featureGate-4-10-Hypershift-DevPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-DevPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-4-10-Hypershift-OKD.yaml
- payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-OKD.yaml
- payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-Default.yaml
- machineconfiguration/v1/types.go
- machineconfiguration/v1alpha1/tests/machineconfignodes.machineconfiguration.openshift.io/MachineConfigNodes.yaml
- features.md
- machineconfiguration/v1alpha1/types_machineconfignode.go
- payload-manifests/featuregates/featureGate-4-10-SelfManagedHA-TechPreviewNoUpgrade.yaml
- machineconfiguration/v1/tests/machineconfignodes.machineconfiguration.openshift.io/MachineConfigNodes.yaml
- machineconfiguration/v1/tests/machineconfignodes.machineconfiguration.openshift.io/IrreconcilableMachineConfig.yaml
- machineconfiguration/v1/types_machineconfignode.go
- machineconfiguration/v1/tests/pinnedimagesets.machineconfiguration.openshift.io/PinnedImages.yaml
- payload-manifests/featuregates/featureGate-4-10-Hypershift-Default.yaml
- payload-manifests/featuregates/featureGate-4-10-Hypershift-TechPreviewNoUpgrade.yaml
✅ Files skipped from review due to trivial changes (1)
- machineconfiguration/v1/tests/machineconfignodes.machineconfiguration.openshift.io/NoRegistryClusterInstall.yaml
🚧 Files skipped from review as they are similar to previous changes (1)
- features/features.go
isabella-janssen
force-pushed
the
ocpbugs-74513
branch
from
25e4204 to
715af4e
Compare
|
/test integration |
|
PR-Agent: could not fine a component named |
|
@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: 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. |
isabella-janssen
force-pushed
the
ocpbugs-74513
branch
from
715af4e to
fe4319e
Compare
isabella-janssen
changed the title
MachineConfigNodes and PinnedImages feature gatesMachineConfigNodes and PinnedImages feature gates and v1alpha1 references
isabella-janssen
force-pushed
the
ocpbugs-74513
branch
from
fe4319e to
018317a
Compare
|
/jira refresh |
|
@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. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
isabella-janssen
force-pushed
the
ocpbugs-74513
branch
from
018317a to
756044c
Compare
|
/test integration |
|
/test verify |
|
/test verify-deps |
|
@isabella-janssen: 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. |
3 participants
This removes the
MachineConfigNodesandPinnedImagesfeature gates as well as the v1alpha1 APIs for each, which were replaced with v1 APIs in 4.19 (#2255 & #2198).