Move ProfileCustomizations under a new GA gate #2590
Conversation
|
Pipeline controller notification For optional jobs, comment |
|
Hello @tkashem! Some important instructions when contributing to openshift/api: |
tkashem
changed the title
WalkthroughReplaces the DynamicResourceAllocation gate with HyperShiftOnlyDynamicResourceAllocation, adds that new gate and related test, updates CRD and payload manifests to mark Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–30 minutes
✨ 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 |
openshift-ci
bot
added
the
size/XXL
![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
🧹 Nitpick comments (1)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml (1)
112-115: Consider enhancing the deprecation notice with migration guidance.The deprecation notice clearly states that
profileCustomizationsis no longer needed since DRA is GA. However, consider adding:
- A timeline for field removal (e.g., "will be removed in version X.Y")
- Guidance on what users should do if they're currently using this field
This would help users plan their migrations more effectively.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (29)
config/v1/types_scheduling.go(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(2 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.featuregated-crd-manifests.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(2 hunks)config/v1/zz_generated.swagger_doc_generated.go(1 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml(1 hunks)
💤 Files with no reviewable changes (6)
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
features.mdconfig/v1/types_scheduling.goconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlfeatures/features.gopayload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yamlconfig/v1/zz_generated.swagger_doc_generated.goconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-Default.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-Default.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yamlconfig/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlopenapi/generated_openapi/zz_generated.openapi.goconfig/v1/zz_generated.featuregated-crd-manifests.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml
🔇 Additional comments (22)
payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1)
32-34: ✓ Approved. Consistent addition to disabled gates list, correctly positioned alphabetically.payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
1-142: ✓ Approved. New CRD correctly omitsprofileCustomizationsfor TechPreviewNoUpgrade variant, aligning with the feature-gate-driven schema strategy.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
1-122: ✓ Approved. New Hypershift TechPreviewNoUpgrade CRD variant correctly omitsprofileCustomizations, maintaining consistency with the feature-gate strategy.payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1)
123-125: ✓ Approved. Correctly positioned in alphabetical order within disabled gates, consistent with Hypershift-only scope of the feature.features.md (1)
8-8: ✓ Approved. Documentation accurately reflects the feature gate's status: enabled on Hypershift Default, disabled elsewhere.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml (1)
9-9: ✓ Approved with note. CRD correctly includesprofileCustomizationsin Default variant. Per PR objectives, verify that deprecation annotations have been added to this field in the corresponding Go type definitions and OpenAPI specs (referenced in AI summary as updated inconfig/v1/types_scheduling.goandopenapi/generated_openapi/zz_generated.openapi.go).Also applies to: 111-130
payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)
23-25: ✓ Approved. Correctly positioned in alphabetical order, consistent with feature-gate strategy for DevPreviewNoUpgrade variant.payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1)
32-34: ✓ Approved. Consistently positioned in alphabetical order, correctly disabled for TechPreviewNoUpgrade variant.payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1)
275-277: Correct placement and status for GA gate in Default configuration.The
HyperShiftOnlyDynamicResourceAllocationgate is correctly added to theenabledlist for Hypershift-Default, which is appropriate for a GA feature.Please verify that all companion feature gate manifests (TechPreviewNoUpgrade, SelfManagedHA variants) have been updated consistently with this gate placed in their
disabledlists, as indicated in the PR summary.payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1)
23-25: Correct placement and status for GA gate in DevPreviewNoUpgrade configuration.The
HyperShiftOnlyDynamicResourceAllocationgate is correctly added to thedisabledlist for DevPreviewNoUpgrade, which is appropriate since GA features should not be enabled in preview feature sets.config/v1/types_scheduling.go (1)
50-54: Deprecation note and new HyperShift-only gate annotation look correctMarking
ProfileCustomizationsas deprecated and moving it underHyperShiftOnlyDynamicResourceAllocationis consistent with the PR’s intent and keeps the Go type/API stable while tightening gating. No issues from a schema or maintainability perspective.config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
570-591: Scheduler CRD now correctly keyed offHyperShiftOnlyDynamicResourceAllocationThe feature-gated CRD manifest for
schedulers.config.openshift.iois wired to the newHyperShiftOnlyDynamicResourceAllocationgate, matching the type annotation and feature-gate definition. Looks consistent and safe.config/v1/zz_generated.swagger_doc_generated.go (1)
2901-2907: Swagger description correctly reflects deprecation ofprofileCustomizationsThe
SchedulerSpec.profileCustomizationsSwagger entry now clearly marks the field as deprecated and explains that DRA is GA and enabled by default, aligning with the struct comment and gating changes. No functional or schema issues.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml (1)
1-122: Hypershift DevPreviewNoUpgrade scheduler CRD shape matches gated API surfaceFor the Hypershift + DevPreviewNoUpgrade variant, exposing only
policy,profile,defaultNodeSelector, andmastersSchedulablewhile omittingprofileCustomizationsis consistent with moving that field behind the new feature gate. CRD metadata, schema, and subresources all look correct.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
1-143: CustomNoUpgrade scheduler CRD correctly exposes deprecatedprofileCustomizationsFor SelfManagedHA + CustomNoUpgrade, the CRD’s
profileCustomizationsblock (withdynamicResourceAllocationenum and deprecation text) matches the Go type and Swagger docs and is appropriately scoped to the unsupported/custom feature set. No structural or gating inconsistencies detected.features/features.go (1)
935-941: LGTM: Feature gate properly configured for Hypershift-only GA enablement.The new
HyperShiftOnlyDynamicResourceAllocationfeature gate is correctly configured to enable only for Hypershift clusters with the Default feature set, which aligns with the PR objective to decouple DRA from scheduler configuration.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml (1)
1-122: LGTM: CRD manifest correctly omits profileCustomizations for DevPreviewNoUpgrade.This generated CRD manifest appropriately excludes the
profileCustomizationsfield since theHyperShiftOnlyDynamicResourceAllocationfeature gate is not enabled in the DevPreviewNoUpgrade feature set.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml (1)
1-143: LGTM: CRD manifest consistent with Default variant.This CustomNoUpgrade variant correctly includes the
profileCustomizationsfield with the same deprecation notice as the Default variant, maintaining consistency across feature sets.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
1-122: LGTM: CRD manifest correctly omits profileCustomizations for TechPreviewNoUpgrade.This generated CRD manifest appropriately excludes the
profileCustomizationsfield, consistent with the feature gate not being enabled in the TechPreviewNoUpgrade feature set.config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (2)
9-9: LGTM: Feature gate annotation correctly updated.The feature gate annotation has been properly updated from
DynamicResourceAllocationtoHyperShiftOnlyDynamicResourceAllocation, which is the core change enabling the decoupling described in the PR objectives.
113-115: LGTM: Deprecation notice consistently applied.The deprecation notice is consistent with other CRD variants and clearly communicates that the field is no longer needed now that DRA is GA.
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (1)
1-122: LGTM: CRD manifest correctly omits profileCustomizations for SelfManagedHA.This generated CRD manifest correctly excludes the
profileCustomizationsfield since theHyperShiftOnlyDynamicResourceAllocationfeature gate is only enabled for the Hypershift cluster profile, not SelfManagedHA.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml (1)
112-114: Deprecation notice forprofileCustomizationsis clear; consider aligning wording with swagger/OpenAPI.The deprecation text correctly states that DRA is GA and enabled by default, and that this field is no longer needed. For consistency across tooling and docs, you may want to mirror the exact deprecation wording used in
zz_generated.swagger_doc_generated.go/ OpenAPI, including the planned removal release, but this is optional.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (29)
config/v1/types_scheduling.go(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(2 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.featuregated-crd-manifests.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(2 hunks)config/v1/zz_generated.swagger_doc_generated.go(1 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(2 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml(1 hunks)
💤 Files with no reviewable changes (4)
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (8)
- payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
- features.md
- payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- config/v1/types_scheduling.go
- config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
features/features.gopayload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlconfig/v1/zz_generated.featuregated-crd-manifests.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlopenapi/generated_openapi/zz_generated.openapi.gopayload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-Default.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-Default.yamlconfig/v1/zz_generated.swagger_doc_generated.go
🔇 Additional comments (15)
payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)
23-25: Correct feature gate placement for SelfManagedHA.The
HyperShiftOnlyDynamicResourceAllocationgate is correctly positioned in the disabled list for this environment profile.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
1-143: CRD deprecation and gating structure looks sound.The CRD properly documents the
profileCustomizationsfield as deprecated with a clear timeline, and the nesteddynamicResourceAllocationenum is correctly defined. The auto-generated manifest is consistent with the feature gate architecture.payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1)
123-125: Correct feature gate placement for SelfManagedHA Default.The
HyperShiftOnlyDynamicResourceAllocationgate is correctly added to the disabled list for this feature set.payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1)
275-277: Correct feature gate placement for Hypershift Default.The
HyperShiftOnlyDynamicResourceAllocationgate is correctly added to the enabled list, aligning with the PR objective to enable DRA by default for Hypershift environments.payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1)
23-25: Correct feature gate placement for Hypershift DevPreviewNoUpgrade.The
HyperShiftOnlyDynamicResourceAllocationgate is correctly added to the disabled list for this preview environment.config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
570-579: Scheduler CRD now correctly gated onHyperShiftOnlyDynamicResourceAllocation.Feature gate wiring for
schedulers.config.openshift.iolooks consistent with the new GA gate and PR intent.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml (1)
9-9: Feature-set annotation for Hypershift Default CRD looks correct.Using
release.openshift.io/feature-set: Defaulton theschedulers-Hypershift-DefaultCRD matches the intended Default feature set variant.features/features.go (1)
935-941: LGTM! Feature gate registration is correct.The new
HyperShiftOnlyDynamicResourceAllocationgate is properly configured with appropriate metadata and correctly scoped to Hypershift clusters in the Default feature set, aligning with the PR's intent to provide a GA gate for DRA in Hypershift environments.payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
112-114: LGTM! Deprecation notice is clear and appropriate.The deprecation message correctly communicates that DRA is GA and enabled by default, making this field unnecessary.
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml (1)
1-122: LGTM! CRD structure is correct for DevPreviewNoUpgrade.The intentional omission of
profileCustomizationsin this DevPreviewNoUpgrade variant is consistent with the feature gate being enabled only for the Hypershift Default profile.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml (1)
112-114: LGTM! Deprecation notice is consistent and appropriate.The deprecation message matches other CustomNoUpgrade variants and clearly indicates the field is no longer needed.
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
1-122: LGTM! CRD structure is correct for TechPreviewNoUpgrade.The intentional omission of
profileCustomizationsis consistent with the feature gate being enabled only for the Hypershift Default profile.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
1-122: LGTM! CRD structure is correct for TechPreviewNoUpgrade.The intentional omission of
profileCustomizationsaligns with the feature gate scope and is consistent with similar preview variants.payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (2)
4-9: Annotations are correct and properly aligned with deployment variant.Verification confirms the annotation pattern is intentional and consistent:
- SelfManagedHA variants consistently use
include.release.openshift.io/self-managed-high-availability: "true"- Hypershift variants consistently use
include.release.openshift.io/ibm-cloud-managed: "true"The removal of the
ibm-cloud-managedannotation from the SelfManagedHA variant is correct—each deployment model uses its own corresponding inclusion annotation. All other annotations (api-approved.openshift.io,api.openshift.io/merged-by-featuregates,release.openshift.io/bootstrap-required,release.openshift.io/feature-set) match the established pattern across all variants.
1-122: Removal of profileCustomizations from DevPreviewNoUpgrade variant is correct and consistent.Verification confirms the changes align with documented feature gate design. The
HyperShiftOnlyDynamicResourceAllocationfeature gate is enabled only in Hypershift-Default, meaning:
- Hypershift-Default: field present (gate enabled)
- SelfManagedHA-Default: field absent (gate disabled per features.md)
- All DevPreviewNoUpgrade/TechPreviewNoUpgrade variants: field absent (gate not enabled in preview variants)
The CRD variants correctly reflect this gating strategy, with deprecation properly documented in the dedicated
HyperShiftOnlyDynamicResourceAllocationmanifest. No inconsistencies found across payload-manifests and generated config/v1 copies.
| description: |- | ||
| Deprecated: no longer needed, since DRA is GA and is enabled by default in the | ||
| cluster, annd this field will be removed in 4.24. | ||
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. |
There was a problem hiding this comment.
Fix typo in deprecation message.
Line 113 contains "annd" which should be "and".
Apply this diff:
profileCustomizations:
description: |-
Deprecated: no longer needed, since DRA is GA and is enabled by default in the
- cluster, annd this field will be removed in 4.24.
+ cluster, and this field will be removed in 4.24.
profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: |- | |
| Deprecated: no longer needed, since DRA is GA and is enabled by default in the | |
| cluster, annd this field will be removed in 4.24. | |
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | |
| description: |- | |
| Deprecated: no longer needed, since DRA is GA and is enabled by default in the | |
| cluster, and this field will be removed in 4.24. | |
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. |
🤖 Prompt for AI Agents
In
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml
around lines 112 to 115, fix the typo in the deprecation description by
replacing "annd" with "and" so the sentence reads "...and this field will be
removed in 4.24."; update the YAML content accordingly with the corrected
spelling.
| Deprecated: no longer needed, since DRA is GA and is enabled by default in the | ||
| cluster, annd this field will be removed in 4.24. | ||
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. |
There was a problem hiding this comment.
Fix typo in deprecation message.
Line 114 contains "annd" which should be "and". This message appears in user-facing documentation.
- Deprecated: no longer needed, since DRA is GA and is enabled by default in the
- cluster, annd this field will be removed in 4.24.
+ Deprecated: no longer needed, since DRA is GA and is enabled by default in the
+ cluster, and this field will be removed in 4.24.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Deprecated: no longer needed, since DRA is GA and is enabled by default in the | |
| cluster, annd this field will be removed in 4.24. | |
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | |
| Deprecated: no longer needed, since DRA is GA and is enabled by default in the | |
| cluster, and this field will be removed in 4.24. | |
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. |
🤖 Prompt for AI Agents
In
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
around lines 113 to 115, fix the typo in the deprecation message by replacing
"annd" with "and" so the sentence reads "...since DRA is GA and is enabled by
default in the cluster, and this field will be removed in 4.24."; update only
that word to correct the user-facing documentation.
| "profileCustomizations": { | ||
| SchemaProps: spec.SchemaProps{ | ||
| Description: "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.", | ||
| Description: "Deprecated: no longer needed, since DRA is GA and is enabled by default in the cluster, annd this field will be removed in 4.24. profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.", |
There was a problem hiding this comment.
Fix typo in deprecation notice.
The deprecation notice contains a typo: "annd" should be "and".
Since this is a generated file, apply the fix to the source file (likely in config/v1/types_scheduling.go or related swagger doc files) and regenerate this OpenAPI file.
The corrected text should read:
"Deprecated: no longer needed, since DRA is GA and is enabled by default in the cluster, and this field will be removed in 4.24. profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles."
Note: The punctuation/formatting issue mentioned in previous review comments should also be addressed during the same fix.
🤖 Prompt for AI Agents
In openapi/generated_openapi/zz_generated.openapi.go around line 19980 the
deprecation string contains a typo ("annd"); update the source declaration
(likely in config/v1/types_scheduling.go or the corresponding swagger/doc
comment) to correct "annd" → "and" and adjust punctuation/formatting as
requested, then regenerate the OpenAPI file so the generated
zz_generated.openapi.go contains the corrected sentence: "Deprecated: no longer
needed, since DRA is GA and is enabled by default in the cluster, and this field
will be removed in 4.24. profileCustomizations contains configuration for
modifying the default behavior of existing scheduler profiles."
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
111-115: Fix recurring typo in deprecation message across multiple CRD manifests.Line 115 contains "annd" which should be "and". This typo appears in files 1, 4, and 5 and impacts user-facing documentation.
Apply this diff:
profileCustomizations: description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. - Deprecated: no longer needed, since DRA is GA and is enabled by default in the - cluster, annd this field will be removed in 4.24. + Deprecated: no longer needed, since DRA is GA and is enabled by default in the + cluster, and this field will be removed in 4.24.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml (1)
111-115: Fix typo in deprecation message and verify message consistency across variants.Line 115 contains "annd" which should be "and". Additionally, note that the deprecation message text differs between this file and HyperShiftOnlyDynamicResourceAllocation.yaml (file 4): this one says "is enabled by' default" (extra apostrophe?) while the other says "is enabled by' default" as well. Both should say "is enabled by default" (no apostrophe).
Apply this diff:
profileCustomizations: description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. - Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + Deprecated: no longer needed, since DRA is GA starting with 4.21, and + is enabled by default in the cluster, this field will be removed in 4.24.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (29)
config/v1/types_scheduling.go(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(2 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.featuregated-crd-manifests.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(2 hunks)config/v1/zz_generated.swagger_doc_generated.go(1 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(2 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml(1 hunks)
💤 Files with no reviewable changes (4)
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (9)
- payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
- config/v1/zz_generated.swagger_doc_generated.go
- payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
- openapi/generated_openapi/zz_generated.openapi.go
- payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
features.mdfeatures/features.gopayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-Default.yamlconfig/v1/types_scheduling.goconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-Hypershift-Default.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlconfig/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yamlconfig/v1/zz_generated.featuregated-crd-manifests.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml
🔇 Additional comments (15)
features.md (1)
8-8: LGTM! Feature gate documentation is consistent with the PR objectives.The new HyperShiftOnlyDynamicResourceAllocation gate is correctly documented as enabled only for Hypershift Default profile, aligning with the intent to decouple DRA configuration from scheduler settings in Hypershift environments.
payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1)
123-125: LGTM! Gate correctly disabled for SelfManagedHA.The HyperShiftOnlyDynamicResourceAllocation gate is appropriately added to the disabled list for the SelfManagedHA Default profile, consistent with the gate being Hypershift-only.
payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1)
32-34: LGTM! Gate correctly disabled for SelfManagedHA TechPreview.Consistent with the gate being Hypershift-only, it's appropriately disabled for the SelfManagedHA TechPreviewNoUpgrade profile.
payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1)
275-277: LGTM! Gate correctly enabled for Hypershift Default.The HyperShiftOnlyDynamicResourceAllocation gate is properly enabled for the Hypershift Default profile, aligning with the PR's intent to move ProfileCustomizations under this new GA gate for Hypershift environments.
config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
577-578: LGTM! CRD feature gate reference updated correctly.The schedulers.config.openshift.io CRD now correctly references HyperShiftOnlyDynamicResourceAllocation instead of DynamicResourceAllocation, consistent with moving ProfileCustomizations under the new gate.
config/v1/types_scheduling.go (1)
51-55: Deprecation notice and feature gate updated appropriately.The ProfileCustomizations field is correctly marked as deprecated with a clear timeline (GA in 4.21, removal in 4.24), and the feature gate annotation properly references the new HyperShiftOnlyDynamicResourceAllocation gate.
features/features.go (1)
935-941: Feature gate registration looks correct.The new HyperShiftOnlyDynamicResourceAllocation gate is properly registered with appropriate metadata and correctly restricted to Hypershift Default profile via
enableForClusterProfile(Hypershift, configv1.Default).Note: The existing FeatureGateDynamicResourceAllocation (lines 87-93) remains in place, which appears intentional to maintain the gate for DevPreview/TechPreview feature sets while the new gate serves Hypershift Default.
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml (1)
1-122: New CRD variant correctly excludes profileCustomizations.This DevPreviewNoUpgrade CRD variant appropriately excludes the profileCustomizations field, which is consistent with the HyperShiftOnlyDynamicResourceAllocation gate not being enabled for DevPreviewNoUpgrade profiles. The field is only present when the gate is active (Hypershift Default).
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml (1)
1-122: API surface correctly excludes profileCustomizations for DevPreviewNoUpgrade.The profileCustomizations field is appropriately omitted from this variant, consistent with the feature gating strategy to move the field under HyperShiftOnlyDynamicResourceAllocation (GA).
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
1-122: API surface correctly excludes profileCustomizations for TechPreviewNoUpgrade.The profileCustomizations field is appropriately omitted from this SelfManagedHA TechPreviewNoUpgrade variant, maintaining consistency with the feature gating strategy.
config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (2)
1-11: Feature gate annotation correctly updated to HyperShiftOnlyDynamicResourceAllocation.The metadata annotation properly reflects the new GA feature gate name, enabling proper CRD gating under the new gate.
112-116: Fix typo in profileCustomizations deprecation message (primary source file).Line 115 contains "annd" which should be "and". This file appears to be a source for generated CRD manifests, so fixing it here may cascade to other generated files. The message should read "...and this field will be removed in 4.24."
Apply this diff:
profileCustomizations: description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. - Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + Deprecated: no longer needed, since DRA is GA starting with 4.21, and + is enabled by default in the cluster, this field will be removed in 4.24.Likely an incorrect or invalid review comment.
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
1-122: API surface correctly excludes profileCustomizations for TechPreviewNoUpgrade.The profileCustomizations field is appropriately omitted from the TechPreviewNoUpgrade variant in payload manifests, maintaining API consistency with feature gating decisions.
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
1-122: API surface correctly excludes profileCustomizations for TechPreviewNoUpgrade variant.Proper alignment with feature gating strategy across both payload-manifests and config/v1 generated directories for this preview-tier feature set.
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (1)
1-122: API surface correctly excludes profileCustomizations for DevPreviewNoUpgrade.The profileCustomizations field is appropriately omitted from this SelfManagedHA DevPreviewNoUpgrade variant, completing the consistent feature gating strategy across all preview-tier manifests.
tkashem
force-pushed
the
fix-dra
branch
2 times, most recently
from
ee4823c to
d0e2999
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml (1)
112-115: Fix grammatical error in deprecation message.Line 115 contains "is enabled by' default" which should be "is enabled by default" (remove the apostrophe). This message is visible to users through API documentation.
This is the same grammatical issue flagged in the SelfManagedHA-CustomNoUpgrade file. Apply this diff:
profileCustomizations: description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + is enabled by default in the cluster, this field will be removed in 4.24.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (30)
config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(1 hunks)config/v1/types_scheduling.go(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(2 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.featuregated-crd-manifests.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(2 hunks)config/v1/zz_generated.swagger_doc_generated.go(1 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(2 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml(1 hunks)
💤 Files with no reviewable changes (4)
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (14)
- features.md
- payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
- openapi/generated_openapi/zz_generated.openapi.go
- config/v1/zz_generated.swagger_doc_generated.go
- payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
- config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml
- features/features.go
- config/v1/zz_generated.featuregated-crd-manifests.yaml
- config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
- config/v1/types_scheduling.go
- payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-Default.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml
🔇 Additional comments (9)
payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1)
23-25: Feature gate entry placement and structure look good.The HyperShiftOnlyDynamicResourceAllocation gate is properly added to the disabled list with correct JSON syntax and alphabetical ordering.
payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1)
23-25: Feature gate entry addition looks good.HyperShiftOnlyDynamicResourceAllocation is properly added to the disabled list with correct placement and syntax, consistent across the feature gate variants.
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml (1)
1-122: CRD manifest structure is correct.The Scheduler CRD is properly defined with correct metadata annotations, API schema, and feature-set gating for DevPreviewNoUpgrade. All standard scheduler fields and subresources are in place.
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
1-122: CRD manifest is properly configured.The Scheduler CRD correctly defines the SelfManagedHA variant with TechPreviewNoUpgrade gating. Annotations, schema, and subresources are all appropriately configured.
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
1-122: CRD manifest looks good.The Hypershift TechPreviewNoUpgrade CRD is properly structured with correct metadata, schema, and subresources for the payload manifests variant.
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (1)
46-111: Verify: profileCustomizations field missing from spec schema.This Scheduler CRD lacks the
profileCustomizationsfield that appears in the Hypershift-Default variant (File 4). Per the PR objectives,profileCustomizationsshould be deprecated and moved under the HyperShiftOnlyDynamicResourceAllocation gate, not removed entirely.Confirm whether this field should be included with a deprecation notice for consistency across all scheduler CRD variants, or if this variant intentionally excludes it.
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
46-111: Verify: profileCustomizations missing from generated CRD schema.Like File 1, this generated scheduler CRD omits the
profileCustomizationsfield. Since this is a zz_generated file, the source template/generator may need updating to include this field with proper deprecation markers across all CRD variants.Verify that the code generation pipeline was updated to produce CRDs with the deprecated
profileCustomizationsfield under the HyperShiftOnlyDynamicResourceAllocation gate.payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1)
238-280: Verify feature gate is consistently added across all variants.The addition of
HyperShiftOnlyDynamicResourceAllocationto the Hypershift-Default enabled list looks correct and properly structured. Confirm that this gate has been consistently added/configured in other feature gate manifests (DevPreviewNoUpgrade, TechPreviewNoUpgrade, etc.) to ensure uniform rollout.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml (1)
9-9: CRD annotation and feature-set updates align with GA promotion.Feature-set updated to "Default" and self-managed-high-availability annotation removed, appropriate for promoting HyperShiftOnlyDynamicResourceAllocation to GA for Hypershift clusters.
| profileCustomizations: | ||
| description: |- | ||
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | ||
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | ||
| is enabled by' default in the cluster, this field will be removed in 4.24. |
There was a problem hiding this comment.
Fix grammatical error in profileCustomizations deprecation message.
Line 115 has "is enabled by' default" which should be "is enabled by default" (remove apostrophe). This message appears in user-facing documentation.
Apply this diff:
profileCustomizations:
description: |-
profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.
Deprecated: no longer needed, since DRA is GA starting with 4.21, and
- is enabled by' default in the cluster, this field will be removed in 4.24.
+ is enabled by default in the cluster, this field will be removed in 4.24.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| profileCustomizations: | |
| description: |- | |
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | |
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | |
| is enabled by' default in the cluster, this field will be removed in 4.24. | |
| profileCustomizations: | |
| description: |- | |
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | |
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | |
| is enabled by default in the cluster, this field will be removed in 4.24. |
🤖 Prompt for AI Agents
In
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml
around lines 111 to 115, the deprecation message for profileCustomizations
contains a stray apostrophe in "is enabled by' default"; remove the apostrophe
so the phrase reads "is enabled by default" and update the line accordingly to
correct the user-facing documentation text.
| description: |- | ||
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | ||
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | ||
| is enabled by' default in the cluster, this field will be removed in 4.24. |
There was a problem hiding this comment.
Fix grammatical error in deprecation message.
Line 115 contains "is enabled by' default" which should be "is enabled by default" (remove the apostrophe after "by"). This user-facing documentation text needs correction.
Apply this diff:
profileCustomizations:
description: |-
profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.
Deprecated: no longer needed, since DRA is GA starting with 4.21, and
- is enabled by' default in the cluster, this field will be removed in 4.24.
+ is enabled by default in the cluster, this field will be removed in 4.24.🤖 Prompt for AI Agents
In
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
around lines 112 to 115, fix the grammatical error in the deprecation message by
removing the stray apostrophe after "by" so the phrase reads "is enabled by
default"; update the line to "is enabled by default in the cluster, this field
will be removed in 4.24." and ensure spacing and punctuation remain correct.
| description: |- | ||
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | ||
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | ||
| is enabled by' default in the cluster, this field will be removed in 4.24. |
There was a problem hiding this comment.
Fix typo in deprecation message.
Line 115 contains a syntax error: "enabled by' default" has an errant single quote. This appears in user-facing API documentation and should be corrected.
Apply this diff to fix the typo:
- Deprecated: no longer needed, since DRA is GA starting with 4.21, and
- is enabled by' default in the cluster, this field will be removed in 4.24.
+ Deprecated: no longer needed, since DRA is GA starting with 4.21, and
+ is enabled by default in the cluster, this field will be removed in 4.24.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| description: |- | |
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | |
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | |
| is enabled by' default in the cluster, this field will be removed in 4.24. | |
| description: |- | |
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | |
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | |
| is enabled by default in the cluster, this field will be removed in 4.24. |
🤖 Prompt for AI Agents
In
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml
around lines 112 to 115, the deprecation description contains a typo "enabled
by' default" — remove the stray single quote so the phrase reads "enabled by
default"; update the text accordingly and ensure YAML quoting/indentation
remains unchanged.
features/features.go
Outdated
| contactPerson("csrwng"). | ||
| productScope(kubernetes). | ||
| enhancementPR("https://github.com/kubernetes/enhancements/issues/4381"). | ||
| enableForClusterProfile(Hypershift, configv1.Default). |
There was a problem hiding this comment.
I think the configv1.Default is always accompanied by the other two cluster types. Moreover, I am not sure if this feature gets enabled in the DevPreviewNoUpgrade, TechPreviewNoUpgrade cluster types if not specified here.
| enableForClusterProfile(Hypershift, configv1.Default). | |
| enableForClusterProfile(Hypershift, configv1.Default, configv1.DevPreviewNoUpgrade, configv1.TechPreviewNoUpgrade). |
There was a problem hiding this comment.
Yes, please include DPNU and TPNU as well.
| // +optional | ||
| Profile SchedulerProfile `json:"profile,omitempty"` | ||
| // profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | ||
| // +openshift:enable:FeatureGate=DynamicResourceAllocation |
There was a problem hiding this comment.
I wonder if it would make the most sense to merge this in keeping the DRA gate in place so that we can avoid an accidental backwards-incompatible change in the hypershift APIs until we explicitly change the gates that are used in the hypershift repository?
openshift-ci
bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
112-115: Fix typo in deprecation message.Line 115 contains "enabled by' default" with an errant single quote that should be removed.
Apply this diff to fix the typo:
description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + is enabled by default in the cluster, this field will be removed in 4.24.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
111-114: Fix typo in deprecation message.Line 114 contains "enabled by' default" with an errant single quote that should be removed.
Apply this diff to fix the typo:
description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + is enabled by default in the cluster, this field will be removed in 4.24.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml (1)
112-115: Fix typo in deprecation message.Line 115 contains "enabled by' default" with an errant single quote that should be removed. This issue was previously flagged.
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml (1)
112-115: Fix typo in deprecation message.Line 115 contains "enabled by' default" with an errant single quote that should be removed.
Apply this diff to fix the typo:
description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + is enabled by default in the cluster, this field will be removed in 4.24.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (27)
config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(1 hunks)config/v1/types_scheduling.go(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.featuregated-crd-manifests.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(2 hunks)config/v1/zz_generated.swagger_doc_generated.go(1 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yaml(2 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml(1 hunks)
💤 Files with no reviewable changes (4)
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (12)
- openapi/generated_openapi/zz_generated.openapi.go
- config/v1/zz_generated.swagger_doc_generated.go
- payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-DevPreviewNoUpgrade.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
- config/v1/types_scheduling.go
- config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml
- payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlfeatures/features.goconfig/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlconfig/v1/zz_generated.featuregated-crd-manifests.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-CustomNoUpgrade.crd.yamlfeatures.mdpayload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-Default.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
🔇 Additional comments (7)
payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)
23-25: LGTM!The HyperShiftOnlyDynamicResourceAllocation feature gate is correctly added to the disabled list for SelfManagedHA clusters, which aligns with the gate being HyperShift-specific.
features/features.go (1)
943-949: LGTM!The new HyperShiftOnlyDynamicResourceAllocation feature gate is correctly configured with all required cluster profiles (Default, DevPreviewNoUpgrade, TechPreviewNoUpgrade) for the Hypershift profile, addressing feedback from previous reviews.
features.md (1)
18-18: LGTM!The feature gate documentation correctly reflects that HyperShiftOnlyDynamicResourceAllocation is enabled for Hypershift clusters (Default, DevPreviewNoUpgrade, TechPreviewNoUpgrade) and disabled for SelfManagedHA clusters.
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
46-111: The review comment is based on incorrect assumptions and should be dismissed.The
profileCustomizationsfield is intentionally omitted from TechPreviewNoUpgrade CRDs as part of the expected feature-gating strategy. The verification shows:
- In SelfManagedHA: only CustomNoUpgrade includes
profileCustomizations; DevPreviewNoUpgrade does not include it (contradicting the review's claim)- In Hypershift: only CustomNoUpgrade and Default include it; DevPreviewNoUpgrade does not
- All TechPreviewNoUpgrade variants consistently omit the field
The source code confirms this is intentional—
profileCustomizationsis gated behindHyperShiftOnlyDynamicResourceAllocationand marked deprecated (will be removed in 4.24). The field appears only in variants where this feature gate is enabled, making the omission in TechPreviewNoUpgrade correct and expected behavior, not a documentation or configuration gap.Likely an incorrect or invalid review comment.
config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
570-591: ✓ Feature gate registration updated correctly.The scheduler CRD metadata correctly references the new
HyperShiftOnlyDynamicResourceAllocationgate. This aligns with the feature gate transition outlined in the PR.config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
9-9: ✓ CRD annotation updated to reflect new feature gate.The
feature-gate.release.openshift.ioannotation is correctly updated to the new gate name.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift-TechPreviewNoUpgrade.crd.yaml (1)
1-122: Verify deprecation messaging and annotation consistency across manifest variants.This new Hypershift TechPreviewNoUpgrade CRD appears structurally sound, but two aspects warrant verification:
Deprecation messaging: Ensure that if this manifest includes the
profileCustomizationsfield (beyond the shown line range), it includes the same deprecation notice added in the generated CRD (HyperShiftOnlyDynamicResourceAllocation.yamllines 113-116).IBM Cloud annotation: Line 7 includes
include.release.openshift.io/ibm-cloud-managed: "true". The PR description mentions removing IBM Cloud annotations from self-managed HA manifests. Confirm this retention on Hypershift manifests is intentional and consistent across other Hypershift variant CRDs added in this PR.
| description: |- | ||
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | ||
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | ||
| is enabled by' default in the cluster, this field will be removed in 4.24. |
There was a problem hiding this comment.
Fix typo in deprecation message.
Line 116 contains a typo: "by'" should be "by". This will be user-visible in API documentation and generated client SDKs.
- Deprecated: no longer needed, since DRA is GA starting with 4.21, and
- is enabled by' default in the cluster, this field will be removed in 4.24.
+ Deprecated: no longer needed, since DRA is GA starting with 4.21, and
+ is enabled by default in the cluster, this field will be removed in 4.24.🤖 Prompt for AI Agents
In
config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml
around lines 113 to 116, the deprecation description contains a typo: the token
"by'" should be "by"; update the description string to remove the stray
apostrophe so it reads "...and is enabled by default in the cluster, this field
will be removed in 4.24." and ensure YAML quoting/indentation remains valid (run
a quick YAML lint or generate docs to verify).
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
111-115: Fix typo: remove stray apostrophe in deprecation message.Line 115 contains
"by'"which should be"by". This typo persists from previous review feedback and will be user-visible in API documentation.profileCustomizations: description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + is enabled by default in the cluster, this field will be removed in 4.24.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
111-114: Fix typo: remove stray apostrophe in deprecation message.Line 114 contains
"by'"which should be"by". This typo was flagged in previous review and persists.profileCustomizations: description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + is enabled by default in the cluster, this field will be removed in 4.24.config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
113-116: Fix typo: remove stray apostrophe in deprecation message.Line 116 contains
"by'"which should be"by". This typo was flagged in previous review for this file and persists.profileCustomizations: description: |- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + is enabled by default in the cluster, this field will be removed in 4.24.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (24)
config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(1 hunks)config/v1/types_scheduling.go(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.featuregated-crd-manifests.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(2 hunks)config/v1/zz_generated.swagger_doc_generated.go(1 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml(1 hunks)
💤 Files with no reviewable changes (4)
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (10)
- openapi/generated_openapi/zz_generated.openapi.go
- config/v1/zz_generated.featuregated-crd-manifests.yaml
- payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
- features/features.go
- payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
- config/v1/types_scheduling.go
- payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
config/v1/zz_generated.swagger_doc_generated.goconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlfeatures.mdpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlconfig/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlconfig/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
🔇 Additional comments (6)
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
46-111: No action required—profileCustomizations field omission is intentional and correct.Verification confirms the field is strategically omitted from this variant. The
profileCustomizationsfield appears only in Hypershift and SelfManagedHA-CustomNoUpgrade variants (with deprecation notices), while TechPreviewNoUpgrade, DevPreviewNoUpgrade, and Default variants intentionally exclude it. This reflects a deliberate design where modern/simplified profile variants do not include the deprecated field, while compatibility-focused variants retain it with deprecation notices for removal in 4.24.payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1)
32-34: Disabled HyperShift-only gate on SelfManagedHA looks correctAdding
HyperShiftOnlyDynamicResourceAllocationto thedisabledlist for SelfManagedHA TechPreviewNoUpgrade correctly keeps this Hypershift-only gate off in SelfManagedHA clusters and matches the intended split between global DRA and Hypershift-only behavior.features.md (1)
18-18: Feature gate matrix entry matches intended Hypershift-only scopeThe
HyperShiftOnlyDynamicResourceAllocationrow correctly shows the gate Enabled only for Hypershift profiles and not for SelfManagedHA, aligning with the new gate’s semantics and the manifest changes.config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
1-16: Test file structure looks good.The test correctly validates that a minimal Scheduler resource can be created under the HyperShiftOnlyDynamicResourceAllocation feature gate. Structure follows the established pattern for feature-gated CRD tests.
config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
9-9: Feature gate annotation is correctly formatted.The new feature gate annotation on line 9 properly marks this CRD as gated by
HyperShiftOnlyDynamicResourceAllocation.payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (1)
98-111: Verify profileCustomizations field is intentionally absent.The new SelfManagedHA-DevPreviewNoUpgrade CRD manifest does not include a
profileCustomizationsfield in the spec, unlike other Scheduler CRDs in the PR. Given that the PR movesprofileCustomizationsunder theHyperShiftOnlyDynamicResourceAllocationgate, confirm whether this omission is intentional (feature-gated exclusion) or an oversight. If intentional, it should be documented in the commit message or a comment.
| description: |- | ||
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | ||
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | ||
| is enabled by' default in the cluster, this field will be removed in 4.24. |
There was a problem hiding this comment.
Fix typo in deprecation notice.
Line 114 contains "by' default" which should be "by default". This is a user-facing text in the CRD documentation.
profileCustomizations:
description: |-
profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.
Deprecated: no longer needed, since DRA is GA starting with 4.21, and
- is enabled by' default in the cluster, this field will be removed in 4.24.
+ is enabled by default in the cluster, this field will be removed in 4.24.Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml
around lines 111 to 114, the deprecation sentence contains a typo: "by' default"
should be "by default"; edit the description string to remove the stray
apostrophe so the user-facing CRD doc reads "...is enabled by default in the
cluster, this field will be removed in 4.24." preserving surrounding punctuation
and line breaks.
| var map_SchedulerSpec = map[string]string{ | ||
| "policy": "DEPRECATED: the scheduler Policy API has been deprecated and will be removed in a future release. policy is a reference to a ConfigMap containing scheduler policy which has user specified predicates and priorities. If this ConfigMap is not available scheduler will default to use DefaultAlgorithmProvider. The namespace for this configmap is openshift-config.", | ||
| "profile": "profile sets which scheduling profile should be set in order to configure scheduling decisions for new pods.\n\nValid values are \"LowNodeUtilization\", \"HighNodeUtilization\", \"NoScoring\" Defaults to \"LowNodeUtilization\"", | ||
| "profileCustomizations": "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.", | ||
| "profileCustomizations": "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and is enabled by' default in the cluster, this field will be removed in 4.24.", | ||
| "defaultNodeSelector": "defaultNodeSelector helps set the cluster-wide default node selector to restrict pod placement to specific nodes. This is applied to the pods created in all namespaces and creates an intersection with any existing nodeSelectors already set on a pod, additionally constraining that pod's selector. For example, defaultNodeSelector: \"type=user-node,region=east\" would set nodeSelector field in pod spec to \"type=user-node,region=east\" to all pods created in all namespaces. Namespaces having project-wide node selectors won't be impacted even if this field is set. This adds an annotation section to the namespace. For example, if a new namespace is created with node-selector='type=user-node,region=east', the annotation openshift.io/node-selector: type=user-node,region=east gets added to the project. When the openshift.io/node-selector annotation is set on the project the value is used in preference to the value we are setting for defaultNodeSelector field. For instance, openshift.io/node-selector: \"type=user-node,region=west\" means that the default of \"type=user-node,region=east\" set in defaultNodeSelector would not be applied.", | ||
| "mastersSchedulable": "mastersSchedulable allows masters nodes to be schedulable. When this flag is turned on, all the master nodes in the cluster will be made schedulable, so that workload pods can run on them. The default value for this field is false, meaning none of the master nodes are schedulable. Important Note: Once the workload pods start running on the master nodes, extreme care must be taken to ensure that cluster-critical control plane components are not impacted. Please turn on this field after doing due diligence.", | ||
| } |
There was a problem hiding this comment.
Fix typo and punctuation in profileCustomizations deprecation text
The deprecation string for profileCustomizations has a small typo and awkward punctuation:
- Typo:
by' defaultcontains an extraneous'. - The clause after “Deprecated:” is a comma splice; splitting into two sentences reads better.
Suggest updating the source comment (then regenerating this file) to something like:
- "profileCustomizations": "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and is enabled by' default in the cluster, this field will be removed in 4.24.",
+ "profileCustomizations": "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21 and is enabled by default in the cluster. This field will be removed in 4.24.",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| var map_SchedulerSpec = map[string]string{ | |
| "policy": "DEPRECATED: the scheduler Policy API has been deprecated and will be removed in a future release. policy is a reference to a ConfigMap containing scheduler policy which has user specified predicates and priorities. If this ConfigMap is not available scheduler will default to use DefaultAlgorithmProvider. The namespace for this configmap is openshift-config.", | |
| "profile": "profile sets which scheduling profile should be set in order to configure scheduling decisions for new pods.\n\nValid values are \"LowNodeUtilization\", \"HighNodeUtilization\", \"NoScoring\" Defaults to \"LowNodeUtilization\"", | |
| "profileCustomizations": "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.", | |
| "profileCustomizations": "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21, and is enabled by' default in the cluster, this field will be removed in 4.24.", | |
| "defaultNodeSelector": "defaultNodeSelector helps set the cluster-wide default node selector to restrict pod placement to specific nodes. This is applied to the pods created in all namespaces and creates an intersection with any existing nodeSelectors already set on a pod, additionally constraining that pod's selector. For example, defaultNodeSelector: \"type=user-node,region=east\" would set nodeSelector field in pod spec to \"type=user-node,region=east\" to all pods created in all namespaces. Namespaces having project-wide node selectors won't be impacted even if this field is set. This adds an annotation section to the namespace. For example, if a new namespace is created with node-selector='type=user-node,region=east', the annotation openshift.io/node-selector: type=user-node,region=east gets added to the project. When the openshift.io/node-selector annotation is set on the project the value is used in preference to the value we are setting for defaultNodeSelector field. For instance, openshift.io/node-selector: \"type=user-node,region=west\" means that the default of \"type=user-node,region=east\" set in defaultNodeSelector would not be applied.", | |
| "mastersSchedulable": "mastersSchedulable allows masters nodes to be schedulable. When this flag is turned on, all the master nodes in the cluster will be made schedulable, so that workload pods can run on them. The default value for this field is false, meaning none of the master nodes are schedulable. Important Note: Once the workload pods start running on the master nodes, extreme care must be taken to ensure that cluster-critical control plane components are not impacted. Please turn on this field after doing due diligence.", | |
| } | |
| var map_SchedulerSpec = map[string]string{ | |
| "policy": "DEPRECATED: the scheduler Policy API has been deprecated and will be removed in a future release. policy is a reference to a ConfigMap containing scheduler policy which has user specified predicates and priorities. If this ConfigMap is not available scheduler will default to use DefaultAlgorithmProvider. The namespace for this configmap is openshift-config.", | |
| "profile": "profile sets which scheduling profile should be set in order to configure scheduling decisions for new pods.\n\nValid values are \"LowNodeUtilization\", \"HighNodeUtilization\", \"NoScoring\" Defaults to \"LowNodeUtilization\"", | |
| "profileCustomizations": "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. Deprecated: no longer needed, since DRA is GA starting with 4.21 and is enabled by default in the cluster. This field will be removed in 4.24.", | |
| "defaultNodeSelector": "defaultNodeSelector helps set the cluster-wide default node selector to restrict pod placement to specific nodes. This is applied to the pods created in all namespaces and creates an intersection with any existing nodeSelectors already set on a pod, additionally constraining that pod's selector. For example, defaultNodeSelector: \"type=user-node,region=east\" would set nodeSelector field in pod spec to \"type=user-node,region=east\" to all pods created in all namespaces. Namespaces having project-wide node selectors won't be impacted even if this field is set. This adds an annotation section to the namespace. For example, if a new namespace is created with node-selector='type=user-node,region=east', the annotation openshift.io/node-selector: type=user-node,region=east gets added to the project. When the openshift.io/node-selector annotation is set on the project the value is used in preference to the value we are setting for defaultNodeSelector field. For instance, openshift.io/node-selector: \"type=user-node,region=west\" means that the default of \"type=user-node,region=east\" set in defaultNodeSelector would not be applied.", | |
| "mastersSchedulable": "mastersSchedulable allows masters nodes to be schedulable. When this flag is turned on, all the master nodes in the cluster will be made schedulable, so that workload pods can run on them. The default value for this field is false, meaning none of the master nodes are schedulable. Important Note: Once the workload pods start running on the master nodes, extreme care must be taken to ensure that cluster-critical control plane components are not impacted. Please turn on this field after doing due diligence.", | |
| } |
🤖 Prompt for AI Agents
In config/v1/zz_generated.swagger_doc_generated.go around lines 2901 to 2907,
the deprecation string for profileCustomizations contains a typo and awkward
punctuation ("by' default" and a comma splice). Fix the source comment that
generates this text: remove the stray apostrophe from "by' default" to read "by
default" and split or rephrase the sentence after "Deprecated:" into two
sentences (or use proper conjunction) so it no longer forms a comma splice; then
regenerate the swagger doc file so the corrected string appears here.
| // +openshift:enable:FeatureGate=DynamicResourceAllocation | ||
| // +openshift:enable:FeatureGate=HyperShiftOnlyDynamicResourceAllocation |
There was a problem hiding this comment.
Looks like the verify check in CI isn't happy because of the ordering in which we are applying feature gate configurations :(
Because HyperShiftOnlyDynamicResourceAllocation isn't enabled at all on standalone, and is alphabetically after the DynamicResourceAllocation it looks like we might be applying and then subsequently removing the fields from the standalone DPNU and TPNU payload manifests....
Maybe we make this new gate also DPNU and TPNU in standalone just for the transition period so we don't permanently break the verify CI job until the transition is complete.
- Add a new gate HyperShiftOnlyDynamicResourceAllocation - make it GA - move the field ProfileCustomizations to the new gate - deprecate the ProfileCustomizations field DRA is GA and is enabled by default, the scheduler enables the DRA plugin if the upstream feature gate is enabled, so the field ProfileCustomizations is defunct. This will remove the coupling between the DynamicResourceAllocation gate and the scheduler config to pave the way for removal of the DynamicResourceAllocation gate
openshift-ci
bot
added
size/XL
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
config/v1/types_scheduling.go (1)
51-54: Fix typo in deprecation comment.Line 52 contains a typo:
"is enabled by' default"should be"is enabled by default"(remove the apostrophe after "by"). This typo appears in the API documentation and should be corrected.Apply this diff:
// profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. // Deprecated: no longer needed, since DRA is GA starting with 4.21, and - // is enabled by' default in the cluster, this field will be removed in 4.24. + // is enabled by default in the cluster, this field will be removed in 4.24. // +openshift:enable:FeatureGate=DynamicResourceAllocationNote: A past review comment indicated this was addressed in commits 79edc2a to 5a73fad, but the typo still appears in the current code.
config/v1/zz_generated.swagger_doc_generated.go (1)
2901-2904: Fix typo and clean upprofileCustomizationsdeprecation text.The deprecation string still contains
by' default; this should beby default, and the long clause would read more clearly as two sentences (e.g., “…enabled by default in the cluster. This field will be removed in 4.24.”). Please fix the source doc comment forSchedulerSpec.profileCustomizations(inconfig/v1/types_scheduling.go) and regenerate swagger/OpenAPI/CRD manifests so all generated artifacts pick up the corrected text.config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
112-116: Fix typo in deprecation message (previously flagged).Line 116 contains the typo
"by'"that was identified in an earlier review. The stray apostrophe must be removed.- Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + Deprecated: no longer needed, since DRA is GA starting with 4.21, and + is enabled by default in the cluster, this field will be removed in 4.24.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
110-114: Fix typo in deprecation message (previously flagged).Line 114 contains the same typo
"by'"that needs correction. This is a duplicate of the issue in the prior file.- Deprecated: no longer needed, since DRA is GA starting with 4.21, and - is enabled by' default in the cluster, this field will be removed in 4.24. + Deprecated: no longer needed, since DRA is GA starting with 4.21, and + is enabled by default in the cluster, this field will be removed in 4.24.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge base: Disabled due to Reviews -> Disable Knowledge Base setting
📒 Files selected for processing (25)
config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(1 hunks)config/v1/types_scheduling.go(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/DynamicResourceAllocation.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml(1 hunks)config/v1/zz_generated.swagger_doc_generated.go(1 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml(1 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml(1 hunks)
💤 Files with no reviewable changes (2)
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-Default.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (10)
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
- openapi/generated_openapi/zz_generated.openapi.go
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
- payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
- config/v1/zz_generated.featuregated-crd-manifests.yaml
- payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
- features.md
- payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
🧰 Additional context used
📓 Path-based instructions (1)
**
⚙️ CodeRabbit configuration file
-Focus on major issues impacting performance, readability, maintainability and security. Avoid nitpicks and avoid verbosity.
Files:
config/v1/zz_generated.swagger_doc_generated.goconfig/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlconfig/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/DynamicResourceAllocation.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlfeatures/features.goconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlconfig/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlconfig/v1/types_scheduling.gopayload-manifests/featuregates/featureGate-Hypershift-Default.yaml
🔇 Additional comments (9)
payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)
23-25: LGTM!The
HyperShiftOnlyDynamicResourceAllocationgate is correctly disabled for the SelfManagedHA DevPreviewNoUpgrade profile, which is appropriate since this gate is specific to HyperShift clusters.payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1)
278-280: LGTM!The
HyperShiftOnlyDynamicResourceAllocationgate is correctly enabled for the Hypershift Default profile, which aligns with the PR objective to move ProfileCustomizations under this new HyperShift-specific gate.features/features.go (1)
943-949: LGTM!The new
FeatureGateHyperShiftOnlyDynamicResourceAllocationis properly defined with all required metadata and correctly scoped to the Hypershift cluster profile with all three feature sets (DevPreviewNoUpgrade, TechPreviewNoUpgrade, Default) as requested in past review comments.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (1)
111-132: SchedulerprofileCustomizationsCRD schema and deprecation wiring look correct.The OpenAPI schema for
spec.profileCustomizations(including the nesteddynamicResourceAllocationenum) matches the existing type and correctly marks the field as deprecated with a removal timeline; no structural or validation issues here. Once the underlying doc comment for this field is fixed and generators are rerun, this description will automatically pick up the corrected wording.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
111-132: Consistent deprecation schema forprofileCustomizationson SelfManagedHA-CustomNoUpgrade.This CRD’s
spec.profileCustomizationsblock matches the other Scheduler CRDs and the Go type (including thedynamicResourceAllocationenum) and correctly marks the field as deprecated with a clear removal version; no structural or validation concerns. Text clean‑up can follow automatically from updating the shared Go doc comment.config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
1-16: New HyperShift-only Scheduler feature-gate test looks correct.The test correctly wires
crdNameandfeatureGates: [HyperShiftOnlyDynamicResourceAllocation]and asserts that a minimalScheduler(spec: {}) remains valid; this is a good safety net for the new gate without over-constraining the API.config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/DynamicResourceAllocation.yaml (1)
112-133: Feature-gated DynamicResourceAllocation slice remains structurally sound.The
profileCustomizationsschema and its deprecation note are consistent with the main Scheduler CRD, and the existingfeature-gate.release.openshift.io/DynamicResourceAllocationannotation is preserved; no structural or gating issues are apparent here. Text fixes will flow from updating the shared Go doc comment and regenerating.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
1-143: Hypershift Scheduler CRD definition is consistent and valid.The new
schedulers.config.openshift.ioHypershift CRD mirrors the existing Scheduler schema (includingprofileCustomizationsanddynamicResourceAllocationenum), uses the status subresource, and has appropriate annotations for Hypershift/IBM-managed environments; no structural, validation, or compatibility issues are apparent.payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
4-8: Verify ibm-cloud-managed annotation presence aligns with PR objectives.The PR objectives state "removes select
include.release.openshift.io/ibm-cloud-managedannotations," but line 7 still includes this annotation. Confirm whether this Hypershift-specific variant should retain this annotation while other variants have it removed, or if it should be removed here as well.
| description: |- | ||
| profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. | ||
| Deprecated: no longer needed, since DRA is GA starting with 4.21, and | ||
| is enabled by' default in the cluster, this field will be removed in 4.24. |
There was a problem hiding this comment.
🧩 Analysis chain
Typo in generated CRD manifest.
Line 115 contains the same typo as in the source file: "is enabled by' default" should be "is enabled by default". Since this is a generated file (indicated by the zz_generated prefix), it will be automatically corrected when you fix the source in config/v1/types_scheduling.go and run the code generation.
Please ensure you run the code generation after fixing the typo in the source file to update all generated manifests.
I'll verify the review comment by checking the source file and code generation setup.
🏁 Script executed:
#!/bin/bash
# Check if source file exists and contains the typo
if [ -f "config/v1/types_scheduling.go" ]; then
echo "=== Source file found ==="
grep -n "is enabled by" config/v1/types_scheduling.go
else
echo "=== Source file not found at config/v1/types_scheduling.go ==="
# Search for the file in case path differs
find . -name "types_scheduling.go" -type f 2>/dev/null | head -5
fi
# Look for code generation scripts or Makefile targets
echo -e "\n=== Checking for code generation mechanism ==="
if [ -f "Makefile" ]; then
grep -n "generate\|crd" Makefile | head -10
fi
# Check for go:generate directives
echo -e "\n=== Checking for go:generate directives ==="
rg "go:generate" --type go -n | grep -i "crd\|manifest" | head -5Length of output: 846
Typo in source file and generated CRD manifest confirmed.
The typo exists in the source file config/v1/types_scheduling.go at line 52: "is enabled by' default" should be "is enabled by default". Since the generated CRD file reflects this typo, the established code generation process (make update-codegen-crds) will correct it when you fix the source file.
🤖 Prompt for AI Agents
In
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
around lines 112-115, the generated description contains a typo "is enabled by'
default"; fix the root source at config/v1/types_scheduling.go (line ~52) by
removing the stray apostrophe so the string reads "is enabled by default", then
re-run the codegen/CRD generation (make update-codegen-crds) to regenerate the
CRD manifest so the typo is corrected in the generated file.
|
Scheduling tests matching the |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: everettraven The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-ci
bot
added
the
approved
|
/verified by @tkashem |
openshift-ci-robot
added
the
verified
|
@tkashem: This PR has been marked as verified by In 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 |
|
/retest-required |
|
/retest-required |
2 similar comments
|
/retest-required |
|
/retest-required |
|
/test e2e-aws-ovn-hypershift-conformance |
|
The e2e-aws-ovn-hypershift-conformance failure seems to be unrelated and is also seen in other recent job(as part of different PRs) runs as well Ref:
|
|
/retest-required |
|
@tkashem: all tests passed! Full PR test history. Your PR dashboard. 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 kubernetes-sigs/prow repository. I understand the commands that are listed here. |
4 participants
move
ProfileCustomizationsunder a new GA gateDRA is GA and is enabled by default, the scheduler enables the DRA plugin if the upstream feature gate is enabled, so the field
ProfileCustomizationsis defunct. This will remove the coupling between theDynamicResourceAllocationgate and the scheduler config to pave the way for removal of theDynamicResourceAllocationgate