OCPNODE-3895,OCPNODE-3894,OCPNODE-3779: Enable DRA(DynamicResourceAllocation) featuregate by default #2498
Conversation
openshift-ci-robot
added
jira/valid-reference
|
@sairameshv: This pull request references Jira Issue OCPBUGS-61381, which is valid. The bug has been moved to the POST state. 3 validation(s) were run on this bug
Requesting review from QA contact: The bug has been updated to refer to the pull request using the external bug tracker. 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. |
openshift-ci
bot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
|
Hello @sairameshv! Some important instructions when contributing to openshift/api: |
openshift-ci
bot
added
the
size/XL
|
/test |
|
@sairameshv: The The following commands are available to trigger optional jobs: Use 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 kubernetes-sigs/prow repository. |
|
/test e2e-gcp |
sairameshv
force-pushed
the
enable_DRA
branch
from
82843d4 to
fad9674
Compare
openshift-ci
bot
added
size/L
sairameshv
changed the title
openshift-ci
bot
removed
the
do-not-merge/work-in-progress
sairameshv
force-pushed
the
enable_DRA
branch
from
fad9674 to
6650897
Compare
openshift-ci
bot
added
size/XL
|
Just FYI,
Following is the output of DRA jobs passing for reference from the output log of the techpreview job I'm also monitoring the o/k PR's techpreview jobs to get the results of the |
|
Regarding the multiple different naming conventions, can we use anything similar to the logic outlined in https://github.com/openshift-eng/openshift-tests-extension/blob/d81c090588352f8cd80934dc61e10b443feca7d7/cmd/example-tests/main.go#L89-L120 to do some labelling/renaming so that all the tests follow enablement/disablement based on the state of the feature gate? Our test tooling should already know how to filter tests based on enabled/disabled feature gates as long as the test spec names are correctly annotated with the feature gate. Having the correct annotations injected would also make it so that the tests report into Sippy in a way that |
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Default.crd.yaml
Show resolved
Hide resolved
|
/hold Please don't merge this until we merge @tkashem's cluster-kube-apiserver-operator PRs. We could end up in another situation where OpenShift serves default-disabled APIs in a GA build. |
openshift-merge-robot
added
the
needs-rebase
openshift-ci
bot
added
the
do-not-merge/hold
|
lgtm, cc @JoelSpeed for an approval |
|
/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. |
WalkthroughThis pull request removes the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Cache: Disabled due to data retention organization setting Knowledge base: Disabled due to 📒 Files selected for processing (14)
💤 Files with no reviewable changes (14)
Comment |
![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
📜 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/DynamicResourceAllocation.yaml(0 hunks)config/v1/types_scheduling.go(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-CustomNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-DevPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-TechPreviewNoUpgrade.crd.yaml(0 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers.crd.yaml(0 hunks)config/v1/zz_generated.deepcopy.go(0 hunks)config/v1/zz_generated.featuregated-crd-manifests.yaml(1 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/DynamicResourceAllocation.yaml(0 hunks)config/v1/zz_generated.swagger_doc_generated.go(1 hunks)features.md(0 hunks)features/features.go(0 hunks)openapi/generated_openapi/zz_generated.openapi.go(1 hunks)openapi/openapi.json(5 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-CustomNoUpgrade.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-DevPreviewNoUpgrade.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-TechPreviewNoUpgrade.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers.crd.yaml(0 hunks)payload-manifests/featuregates/featureGate-Hypershift-Default.yaml(0 hunks)payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml(0 hunks)payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml(0 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml(0 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml(0 hunks)payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml(0 hunks)
💤 Files with no reviewable changes (20)
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-TechPreviewNoUpgrade.crd.yaml
- config/v1/types_scheduling.go
- payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
- features/features.go
- payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
- config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/DynamicResourceAllocation.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers.crd.yaml
- config/v1/tests/schedulers.config.openshift.io/DynamicResourceAllocation.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-DevPreviewNoUpgrade.crd.yaml
- features.md
- payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
- config/v1/zz_generated.deepcopy.go
- payload-manifests/crds/0000_10_config-operator_01_schedulers-TechPreviewNoUpgrade.crd.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-DevPreviewNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-CustomNoUpgrade.crd.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-CustomNoUpgrade.crd.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers.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.featuregated-crd-manifests.yamlopenapi/generated_openapi/zz_generated.openapi.goopenapi/openapi.jsonconfig/v1/zz_generated.swagger_doc_generated.go
🔇 Additional comments (3)
config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
577-577: Approve the DRA feature gate removal from schedulers CRD metadata.The change correctly removes
DynamicResourceAllocationfrom the feature gates list, promoting the feature to GA availability. Formatting is consistent with other CRD entries in the file (e.g., lines 54, 182, 271, 399 showing emptyFeatureGates: []).Since this file is auto-generated (prefix
zz_generated), verify that the upstream generation source (code generation, manifests, or templates) has been updated accordingly to ensure this change persists across future regenerations.openapi/generated_openapi/zz_generated.openapi.go (1)
19972-19976: Updated dependency list looks correctRestricting the
Dependenciesarray here to onlygithub.com/openshift/api/config/v1.ConfigMapNameReferenceis consistent with the removal ofProfileCustomizations-related types and keeps the generated OpenAPI wiring minimal. No issues from a correctness or maintainability perspective, assuming this file is regenerated from the updated source types.openapi/openapi.json (1)
4780-4789:⚠️ Major inconsistency: provided code hunks do not match PR objectives.The hunks show repeated changes to
dnsRecordsTypefield schema (adding descriptions, type constraints, and enum values), but the PR objectives describe removal of DRA feature gate and ProfileCustomizations types from Scheduler config API. These hunks appear unrelated to DynamicResourceAllocation or ProfileCustomizations.The schema improvements themselves are sound—adding explicit descriptions, type declarations, and enum constraints improves API documentation and validation. However, this appears to be either a regeneration side-effect or incorrect hunks were provided.
Please confirm:
- Are these hunks intended for this PR, or are they collateral regeneration from changes in other files?
- Should this file contain actual DRA-related schema removals (e.g., ProfileCustomizations definitions being deleted)?
- If these dnsRecordsType changes are intentional, clarify their connection to the DRA feature gate removal objective.
You can verify by checking whether
ProfileCustomizationsorDynamicResourceAllocationschema definitions were removed elsewhere in this file.Also applies to: 8933-8942, 9551-9560, 9749-9758, 11655-11660
| "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\"", | ||
| "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.
🧩 Analysis chain
Generated docs look fine; please verify source-of-truth and enum alignment.
Since this file is auto-generated, ensure these doc updates come from comments in config/v1/types_scheduling.go and that the listed valid values for profile exactly match any kubebuilder Enum or server-side validation to avoid drift. Also confirm no manual edits to generated artifacts.
Run to verify the upstream source and validation:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "1) Locate SchedulerSpec and Profile field definition/comments"
rg -nC3 -g 'config/v1/**' $'type\\s+SchedulerSpec\\b'
rg -nC3 -g 'config/v1/**' $'\\bProfile\\b.*`json:"profile' || true
echo -e "\n2) Check for explicit Enum validation on Profile"
rg -nC2 -g 'config/v1/**' 'kubebuilder:validation:Enum' || echo "No Enum tag found for Profile"
echo -e "\n3) Sanity: confirm this file is generated (header marker)"
rg -n 'AUTO-GENERATED FUNCTIONS START HERE' config/v1/zz_generated.swagger_doc_generated.goLength of output: 20407
Let me verify one more thing about the empty string in the Enum pattern:
Fix the generated swagger docs to include all valid enum values for the profile field.
The verification found a discrepancy: the source validation defines +kubebuilder:validation:Enum="";LowNodeUtilization;HighNodeUtilization;NoScoring, which includes an empty string as valid. However, the generated swagger documentation at lines 2893-2896 lists only "LowNodeUtilization", "HighNodeUtilization", "NoScoring" and omits the empty string value.
The empty string is intentional in the Enum (allowing the field to remain unset), so the swagger doc must be updated to reflect this. Either regenerate the docs from the source comments by running hack update scripts, or update the source field comment in config/v1/types_scheduling.go (around line 46-47) to explicitly document the empty string as a valid value.
🤖 Prompt for AI Agents
In config/v1/zz_generated.swagger_doc_generated.go around lines 2893-2896 the
swagger enum for the `profile` field omits the empty-string value allowed by the
kubebuilder validation; fix by either regenerating the swagger docs so the
generated string list includes the empty string (run the repo's
hack/update-generated / hack/update-swagger-docs script per project
instructions) or update the source comment in config/v1/types_scheduling.go
(around lines 46-47) to explicitly document the empty string as a valid value
and then re-run the doc generation so the generated swagger contains
"";LowNodeUtilization;HighNodeUtilization;NoScoring.
60ab84d to
6819d79
Compare
openshift-ci-robot
removed
the
verified
|
New changes are detected. LGTM label has been removed. |
openshift-ci
bot
added
size/L
|
/hold |
openshift-ci
bot
added
the
do-not-merge/hold
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
116-130: Consider propagating deprecation to nested enum field.The
dynamicResourceAllocationenum nested under the deprecatedprofileCustomizationsshould either explicitly inherit the deprecation marker or have its own deprecation notice if it will continue to be relevant. Current state may confuse users about the field's lifecycle.
📜 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(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)openapi/openapi.json(2 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-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
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
✅ Files skipped from review due to trivial changes (1)
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- features.md
- payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
- config/v1/zz_generated.swagger_doc_generated.go
- payload-manifests/featuregates/featureGate-Hypershift-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:
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlfeatures/features.goconfig/v1/zz_generated.featuregated-crd-manifests.yamlconfig/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlconfig/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlconfig/v1/types_scheduling.goopenapi/openapi.jsonopenapi/generated_openapi/zz_generated.openapi.gopayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
🔇 Additional comments (15)
payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1)
32-34: HyperShiftOnlyDynamicResourceAllocation disabled wiring looks correctGate name and placement in the disabled list for SelfManagedHA TechPreviewNoUpgrade are consistent with the new feature-gate definition and intent.
config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
578-578: Scheduler CRD feature-gate registration is consistentAdding
HyperShiftOnlyDynamicResourceAllocationto the Scheduler CRD’sFeatureGateslist correctly aligns the CRD metadata with the new gate.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
112-115: CRD schema deprecation text matches API typeThe updated
profileCustomizationsdescription mirrors the Go type deprecation note, which is good for consistency and for clearly signaling removal in 4.24. Any refinement to the DRA default wording can follow whatever is decided inconfig/v1/types_scheduling.go.features/features.go (1)
935-941: New HyperShiftOnlyDynamicResourceAllocation gate is wired correctlyThe new feature gate follows existing patterns (metadata, enhancement link, cluster-profile scoping via
enableForClusterProfile, andmustRegister()), and its name matches all manifest uses.payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)
23-25: SelfManagedHA DevPreviewNoUpgrade correctly disables HyperShiftOnlyDynamicResourceAllocationExplicitly listing the new gate as disabled for this profile keeps behavior clear and matches its “HyperShiftOnly” scope.
payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1)
199-201: Hypershift TechPreviewNoUpgrade enabling of HyperShiftOnlyDynamicResourceAllocation is consistentThe gate is enabled for the Hypershift TechPreviewNoUpgrade profile as defined in
features.go, and the placement in the enabled list is consistent with surrounding entries.payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml (1)
193-195: Hypershift DevPreviewNoUpgrade manifest correctly enables the new gateAdding
HyperShiftOnlyDynamicResourceAllocationto the enabled list aligns this manifest with the new feature-gate definition and Hypershift DevPreviewNoUpgrade behavior.config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
1-16: Minimal test appropriately validates feature gate applicability.The test correctly creates and verifies a minimal Scheduler object under the HyperShiftOnlyDynamicResourceAllocation feature gate. The structure and expectations are sound.
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
110-114: Clarify deprecation scope in the description.The deprecation message states "DRA is GA and enabled by default in the cluster," but the feature gate is named
HyperShiftOnlyDynamicResourceAllocation, implying HyperShift-only scope. Clarify whether this deprecation applies universally or only to HyperShift deployments to avoid user confusion.openapi/generated_openapi/zz_generated.openapi.go (1)
19979-19982: OpenAPI deprecation description looks correct and aligned with DRA promotionThe updated
profileCustomizationsdescription clearly marks the field as deprecated, ties it to DRA GA in 4.21 and default enablement, and states the planned removal in 4.24. Assuming this was regenerated from the Go types/Swagger docs via the usualmake updateflow, this OpenAPI change looks good and maintainable.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
111-114: Deprecation message is clear and well‑specified.The updated description properly deprecates the
profileCustomizationsfield with rationale (DRA GA in 4.21) and removal timeline (4.24). This follows good API deprecation practices—the field remains functional while communicating its planned removal.payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (1)
1-122: Consistency with variant pattern confirmed.This new SelfManagedHA DevPreviewNoUpgrade manifest consistently excludes
profileCustomizations, matching the TechPreviewNoUpgrade variant (File 2). This confirms the intentional schema differentiation for SelfManagedHA deployments.config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
9-9: Feature gate annotation and deprecation messaging are correctly sourced.The source file
config/v1/types_scheduling.gocontains the correct annotation+openshift:enable:FeatureGate=HyperShiftOnlyDynamicResourceAllocationand deprecation comment. The generated CRD manifest file properly reflects this source code state, with deprecation messaging clearly indicating DRA is GA starting 4.21 and removal in 4.24.payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
1-122: No issues found—field exclusion is consistent and correct.The
profileCustomizationsfield exclusion in the TechPreviewNoUpgrade variant is intentional and follows a consistent pattern across all deployment profiles. Verification confirms:
- CustomNoUpgrade includes
profileCustomizations(both payload-manifests and generated)- Default, DevPreviewNoUpgrade, and TechPreviewNoUpgrade exclude it (both payload-manifests and generated)
The payload-manifests file matches the corresponding generated CRD exactly, confirming the schema is correct for this deployment variant.
openapi/openapi.json (1)
10813-10816: Deprecation messaging is clear, but verify removal timeline coordination.The deprecation note for
profileCustomizationsclearly documents why the field is no longer needed (DRA GA in 4.21, enabled by default) and sets an explicit removal version (4.24). This is consistent with the PR objectives.Ensure the 4.24 removal timeline is coordinated with release planning and that dependent code paths have sufficient notice.
openapi/openapi.json
Outdated
| "closedClientConnectionPolicy": { | ||
| "description": "closedClientConnectionPolicy controls how the IngressController behaves when the client closes the TCP connection while the TLS handshake or HTTP request is in progress. This option maps directly to HAProxy’s \"abortonclose\" option.\n\nValid values are: \"Abort\" and \"Continue\". The default value is \"Continue\".\n\nWhen set to \"Abort\", the router will stop processing the TLS handshake if it is in progress, and it will not send an HTTP request to the backend server if the request has not yet been sent when the client closes the connection.\n\nWhen set to \"Continue\", the router will complete the TLS handshake if it is in progress, or send an HTTP request to the backend server and wait for the backend server's response, regardless of whether the client has closed the connection.\n\nSetting \"Abort\" can help free CPU resources otherwise spent on TLS computation for connections the client has already closed, and can reduce request queue size, thereby reducing the load on saturated backend servers.\n\nImportant Considerations:\n\n - The default policy (\"Continue\") is HTTP-compliant, and requests\n for aborted client connections will still be served.\n Use the \"Continue\" policy to allow a client to send a request\n and then immediately close its side of the connection while\n still receiving a response on the half-closed connection.\n\n - When clients use keep-alive connections, the most common case for premature\n closure is when the user wants to cancel the transfer or when a timeout\n occurs. In that case, the \"Abort\" policy may be used to reduce resource consumption.\n\n - Using RSA keys larger than 2048 bits can significantly slow down\n TLS computations. Consider using the \"Abort\" policy to reduce CPU usage.", | ||
| "type": "string", | ||
| "default": "Continue" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
This hunk introduces closedClientConnectionPolicy for IngressController HAProxy configuration. However, the PR is scoped to enabling the DynamicResourceAllocation feature gate and promoting it to GA (as stated in PR title and objectives).
This change:
- Is unrelated to DRA feature gate promotion
- Is missing from the AI-generated summary
- Introduces HAProxy/ingress-specific behavior unrelated to scheduler resource allocation
Recommendation: Extract this change to a separate PR to maintain focused review and reduce risk of unintended interactions with the DRA promotion.
🏁 Script executed:
#!/bin/bash
# Locate openapi.json and check if the file exists
find . -name "openapi.json" -type f 2>/dev/null | head -20Length of output: 22
🏁 Script executed:
#!/bin/bash
# Search for closedClientConnectionPolicy in the codebase to understand context
rg -n "closedClientConnectionPolicy" --type json -C 5 2>/dev/null || echo "Pattern not found"Length of output: 3665
🏁 Script executed:
#!/bin/bash
# Check for DynamicResourceAllocation references in openapi.json or nearby changes
rg -n "DynamicResourceAllocation" --type json -C 3 2>/dev/null | head -50 || echo "No DRA references found"Length of output: 1286
🏁 Script executed:
#!/bin/bash
# Search for IngressController definitions in openapi.json to understand schema context
rg -n '"IngressController"' --type json -C 3 2>/dev/null | head -50 || echo "Pattern not found"Length of output: 0
🏁 Script executed:
#!/bin/bash
# Check git log or PR description if available
if git rev-parse --git-dir > /dev/null 2>&1; then
echo "=== Current branch ===" && git rev-parse --abbrev-ref HEAD
echo "=== Recent commits ===" && git log --oneline -10
echo "=== PR/Branch info ===" && git log -1 --format="%B"
fiLength of output: 1227
🏁 Script executed:
#!/bin/bash
# Show changes to openapi.json in the current PR
git diff HEAD~1 openapi/openapi.json | grep -A 10 -B 10 "closedClientConnectionPolicy" | head -50 || echo "No changes found in diff"Length of output: 3831
🏁 Script executed:
#!/bin/bash
# Check git blame for the specific lines to see when closedClientConnectionPolicy was added
git blame -L 30630,30645 openapi/openapi.json 2>/dev/null | grep -E "closedClientConnectionPolicy|^[a-f0-9]+"Length of output: 4448
🏁 Script executed:
#!/bin/bash
# Check if closedClientConnectionPolicy existed in the parent commit
git show HEAD~1:openapi/openapi.json 2>/dev/null | grep -n "closedClientConnectionPolicy" || echo "Not found in parent commit"Length of output: 26
🏁 Script executed:
#!/bin/bash
# Get list of all files changed in this PR
git diff --name-only HEAD~1Length of output: 1771
Extract closedClientConnectionPolicy changes to a separate PR—scope mismatch confirmed.
Verification confirms the review comment is valid. The closedClientConnectionPolicy field (IngressController HAProxy setting) was added in this PR but is unrelated to DRA GA promotion. Git blame shows this change originated in the current PR (commit 6819d79) and did not exist in the parent commit. While the openapi.json file is auto-generated via make update, the IngressController enhancement is completely separate from the scheduler resource allocation changes that define this PR's scope. All other changed files focus on DRA promotion, CRD manifests, and feature gate updates.
Extracting this to a separate PR maintains focused review and prevents unintended interactions between unrelated feature areas.
🤖 Prompt for AI Agents
In openapi/openapi.json around lines 30637-30641, the new
closedClientConnectionPolicy schema entry was introduced in this PR but is
unrelated to the DRA GA promotion scope; extract this change into a separate PR
by removing the closedClientConnectionPolicy addition from this branch (revert
the specific auto-generated change or regenerate openapi.json from the parent
commit), then create a new focused PR that contains only the IngressController
HAProxy closedClientConnectionPolicy addition (including any code/CRD changes
that necessitate the schema update) with its own commit and description so
reviews remain scoped to DRA GA promotion here.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
111-114: Deprecation message scope issue.Same concern as flagged in
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml: the deprecation message states DRA is "enabled by default in the cluster," but the feature gate is scoped to HyperShift only.config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
113-116: Deprecation message scope inconsistency.The deprecation message claims DRA is "enabled by default in the cluster," but this manifest is specifically for the
HyperShiftOnlyDynamicResourceAllocationfeature gate, which is not enabled for SelfManagedHA clusters per the features.md table.
📜 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(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)openapi/openapi.json(2 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-DevPreviewNoUpgrade.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-TechPreviewNoUpgrade.crd.yaml
🚧 Files skipped from review as they are similar to previous changes (3)
- payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
- payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
- 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:
features/features.goopenapi/generated_openapi/zz_generated.openapi.gopayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.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-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlconfig/v1/types_scheduling.goconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yamlconfig/v1/zz_generated.featuregated-crd-manifests.yamlconfig/v1/zz_generated.swagger_doc_generated.goconfig/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlfeatures.mdpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlopenapi/openapi.json
🔇 Additional comments (14)
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (1)
1-122: Inconsistency between AI summary and actual file content.The summary states "Removed the profileCustomizations field from the CRD schema," but this file does not contain a
profileCustomizationsfield at all (spec ends at theprofilefield on line 110). Clarify whether this omission is intentional for the DevPreviewNoUpgrade feature set or if the summary conflated this file with the CustomNoUpgrade variant (which does contain and deprecate the field).payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
112-115: Deprecation message is clear and properly formatted.The multi-line description correctly documents deprecation reason, GA status, and removal timeline (4.24). Structure follows best practices for API deprecation.
However, verify that the message accurately reflects the actual scope: the feature gate changes show renaming to
HyperShiftOnlyDynamicResourceAllocation, suggesting the feature is scoped to HyperShift deployments rather than enabled globally by default across all clusters. Confirm the deprecation message is not misleading regarding cluster-wide default enablement.payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml (1)
32-34: Feature gate addition is well-placed and properly formatted.
HyperShiftOnlyDynamicResourceAllocationcorrectly added to the disabled list with proper JSON structure and alphabetical ordering preserved.payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)
23-25: Feature gate addition is consistent with related manifests and properly positioned.
HyperShiftOnlyDynamicResourceAllocationcorrectly added to disabled list with proper formatting and alphabetical ordering, consistent with the TechPreviewNoUpgrade variant.payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1)
199-201: Feature gate correctly enabled in HyperShift context.
HyperShiftOnlyDynamicResourceAllocationappropriately added to the enabled list for Hypershift-TechPreviewNoUpgrade, with proper formatting and alphabetical ordering.Verify that this gate rename/scoping strategy aligns with PR objectives. The current implementation enables a HyperShift-specific DRA variant only in HyperShift profiles, whereas the PR description suggests enabling DRA globally by default for OCP-4.21. Confirm whether the original
DynamicResourceAllocationgate has been completely replaced byHyperShiftOnlyDynamicResourceAllocationacross all manifests, or if it exists elsewhere with different semantics.openapi/generated_openapi/zz_generated.openapi.go (1)
19980-19980: LGTM: Deprecation properly documented in generated OpenAPI schema.The description correctly marks
profileCustomizationsas deprecated, explains the reason (DRA is GA and enabled by default starting with 4.21), and specifies the removal timeline (4.24). Since this is generated code (zz_generatedprefix), the change appropriately reflects updates to the source types.config/v1/types_scheduling.go (1)
51-53: Clarify deprecation approach vs past review feedback.Past review comments indicated
ProfileCustomizationsshould be removed entirely since it has no use without DRA (per ingvagabund's feedback). The author acknowledged this with "Ack, Removed the field from the API." However, the current changes deprecate the field rather than remove it, with a planned removal in 4.24.Additionally, the deprecation message states DRA "is enabled by default in the cluster," but the feature gate annotation
HyperShiftOnlyDynamicResourceAllocationscopes this to HyperShift clusters only, not all cluster types.Please clarify:
- Why deprecation was chosen over removal, given the past review feedback?
- Whether the "enabled by default in the cluster" language is accurate given the HyperShift-only scope?
Based on past review comments.
config/v1/tests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
1-16: LGTM - Test structure is correct.The test configuration follows the standard pattern for validating minimal Scheduler objects with the feature gate enabled. The test structure itself is appropriate.
config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
578-578: Generated file reflects feature gate rename.This change is consistent with the feature gate rename from
DynamicResourceAllocationtoHyperShiftOnlyDynamicResourceAllocation. As this is a generated file, any concerns about the feature gate scope should be addressed in the source definitions (already flagged in previous comments).config/v1/zz_generated.swagger_doc_generated.go (1)
2902-2905: Deprecation text forprofileCustomizationslooks correct; verify source comment and version timeline are in sync.The new description clearly marks
profileCustomizationsas deprecated and ties it to DRA GA in 4.21 with removal in 4.24, which matches the PR intent. Since this file is generated, please double‑check that:
- The same wording (including 4.21/4.24) is present in
config/v1/types_scheduling.goand other generated artifacts (OpenAPI, CRDs), and- The 4.21 GA and 4.24 removal versions are the agreed product timeline across docs and feature‑gate definitions.
As long as those sources are aligned and this was produced via the normal
make update/codegen path, this change is good to go.features/features.go (1)
935-941: LGTM! Feature gate correctly scoped to HyperShift.The new
FeatureGateHyperShiftOnlyDynamicResourceAllocationproperly restricts the downstream-specificprofileCustomizationsfield to HyperShift cluster profiles only. The configuration changes are appropriate:
enableForClusterProfile(Hypershift, ...)correctly limits activation to HyperShift clustersproductScope(ocpSpecific)correctly reflects that this gate controls OpenShift-specific behavior, even though the underlying DRA feature is upstream GA- Jira component and contact person updated to reflect HyperShift ownership
The enhancement PR link pointing to the upstream Kubernetes issue is acceptable since it documents the underlying DRA feature.
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
1-122: LGTM! CRD correctly omits profileCustomizations for SelfManagedHA.This CRD manifest for the
SelfManagedHA-TechPreviewNoUpgradefeature set correctly excludes theprofileCustomizationsfield, which is consistent with the feature gate being scoped to HyperShift cluster profiles only.The spec includes only the core scheduler configuration fields (
defaultNodeSelector,mastersSchedulable,policy,profile), and the CRD structure is complete and valid.openapi/openapi.json (2)
10813-10816: Deprecation message aligns with PR objectives and provides clear removal timeline.The updated description for
profileCustomizationsclearly communicates that DRA is now GA (in 4.21) and the field will be removed in 4.24. The rationale is explicit for users.
30637-30641: Scope mismatch: closedClientConnectionPolicy is unrelated to DRA feature gate changes.The PR objectives focus on enabling the DRA feature gate by default for Kubernetes v1.34 GA promotion. This field (IngressController TCP connection handling) appears orthogonal to DRA and scheduler-related changes.
The field is well-documented (enum values, defaults, rationale), but its inclusion needs clarification: is this an intentional multi-faceted change, or was it accidentally included during a rebase/merge?
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml
Outdated
Show resolved
Hide resolved
| 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.
Critical inconsistency for SelfManagedHA clusters.
This CRD manifest is for SelfManagedHA clusters (per the filename and metadata annotation release.openshift.io/feature-set: CustomNoUpgrade), but the deprecation message states DRA is "enabled by default in the cluster." However, the features.md table (line 18) shows HyperShiftOnlyDynamicResourceAllocation is not enabled for SelfManagedHA clusters.
This creates a misleading message for SelfManagedHA users: they're being told a field is deprecated because the feature is enabled by default, when it's actually not enabled for their cluster type at all.
The deprecation message should either:
- Be conditional/variant-specific based on cluster type, or
- The feature gate should be enabled for SelfManagedHA clusters if the intent is universal DRA enablement
🤖 Prompt for AI Agents
In
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
around lines 112-115, the deprecation message incorrectly states DRA is "enabled
by default in the cluster" while SelfManagedHA does not have
HyperShiftOnlyDynamicResourceAllocation enabled; update the manifest to either
(A) make the deprecation message variant-specific (explicitly state that DRA is
not enabled for SelfManagedHA and therefore the deprecation does not apply) or
(B) if the intent is universal DRA enablement, flip the feature gate for
SelfManagedHA so HyperShiftOnlyDynamicResourceAllocation is enabled and then
keep the original message; ensure the chosen approach is reflected in the CRD
description text and update any related metadata/comments to avoid misleading
SelfManagedHA users.
| | NewOLMPreflightPermissionChecks| | | | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | | ||
| | NoRegistryClusterInstall| | | | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | | ||
| | ProvisioningRequestAvailable| | | <span style="background-color: #519450">Enabled</span> | <span style="background-color: #519450">Enabled</span> | | | | ||
| | HyperShiftOnlyDynamicResourceAllocation| <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | | <span style="background-color: #519450">Enabled</span> | | |
There was a problem hiding this comment.
Feature gate scope contradicts PR objective.
The PR description states the goal is to "Enable the DynamicResourceAllocation (DRA) feature gate by default because DRA graduated to GA in upstream Kubernetes v1.34." However, this row shows HyperShiftOnlyDynamicResourceAllocation is enabled only for HyperShift cluster profiles and remains disabled for SelfManagedHA clusters.
If the intent is to enable DRA by default for all clusters (per the PR description and the upstream GA status), the feature gate should be enabled for both HyperShift and SelfManagedHA columns. If the intent is to scope this to HyperShift only, the PR description and deprecation messages claiming "enabled by default in the cluster" need clarification.
🤖 Prompt for AI Agents
In features.md around line 18, the table row for
HyperShiftOnlyDynamicResourceAllocation conflicts with the PR description:
either enable DynamicResourceAllocation for SelfManagedHA as well or clarify the
PR scope; to fix, update the table so both HyperShift and SelfManagedHA columns
show Enabled if the intent is to enable DRA cluster-wide, or else change the PR
description and any deprecation/notification text to state the feature is being
enabled only for HyperShift and not for SelfManagedHA.
6819d79 to
60bb094
Compare
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-Hypershift.crd.yaml (1)
111-115: Clarify deprecation scope for HyperShift deployments.The deprecation message states DRA "is enabled by default in the cluster," but this CRD is specific to HyperShift deployments (per filename and removed annotations). Since the feature gate is named
HyperShiftOnlyDynamicResourceAllocation, consider clarifying whether "the cluster" refers to all cluster types or specifically HyperShift clusters to avoid user confusion.config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
112-126: Deprecation message may mislead SelfManagedHA users.This CRD is for SelfManagedHA clusters (per metadata annotation on line 7 and feature-set on line 9), but the deprecation message states DRA "is enabled by default in the cluster." Given the gate is named
HyperShiftOnlyDynamicResourceAllocation, this creates confusion about actual DRA availability in SelfManagedHA deployments. Past reviews flagged this as critical.openapi/openapi.json (1)
30637-30641: Duplicate: IngressController closedClientConnectionPolicy is out of scope.This addition of
closedClientConnectionPolicy(lines 30637–30641) for IngressController HAProxy configuration was already flagged in a prior review as unrelated to the DRA GA promotion scope. The field pertains to ingress-controller-specific behavior, not scheduler resource allocation.As confirmed in the prior review, this change should be extracted to a separate PR to maintain focused review scope and reduce risk of unintended feature interactions.
📜 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 (22)
config/v1/types_scheduling.go(2 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(2 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(2 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(3 hunks)config/v1/zz_generated.swagger_doc_generated.go(2 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(2 hunks)openapi/openapi.json(3 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(2 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(2 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)
- 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
🚧 Files skipped from review as they are similar to previous changes (8)
- payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
- config/v1/zz_generated.swagger_doc_generated.go
- payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
- features.md
- features/features.go
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.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/types_scheduling.goconfig/v1/zz_generated.featuregated-crd-manifests.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-Default.yamlconfig/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yamlopenapi/generated_openapi/zz_generated.openapi.goopenapi/openapi.jsonpayload-manifests/featuregates/featureGate-Hypershift-Default.yaml
🔇 Additional comments (12)
payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1)
120-122: Manifest update looks good—verify feature definition alignment.The addition of
HyperShiftOnlyDynamicResourceAllocationto the disabled list for self-managed HA deployments is logically correct and maintains alphabetical ordering. Ensure that the corresponding feature definition (likely infeatures.goor related feature declaration file) includes this gate and that the oldDynamicResourceAllocationgate has been removed from all relevant manifests across the codebase.payload-manifests/featuregates/featureGate-Hypershift-Default.yaml (1)
275-277: Manifest update looks good—verify consistency with feature declarations.The addition of
HyperShiftOnlyDynamicResourceAllocationto the enabled list for HyperShift deployments is logical and maintains alphabetical ordering. Verify that this gate is:
- Properly declared in the upstream feature definitions
- Removed from all other relevant manifest variants (e.g., ensure the old
DynamicResourceAllocationentry is gone from all deployment profiles)- Consistent with the corresponding CRD annotations and OpenAPI descriptions that reference this gate
openapi/generated_openapi/zz_generated.openapi.go (2)
19006-19012: LGTM: Description accurately reflects DRA GA status.The updated description correctly states that DRA is now enabled by default following its graduation to GA in Kubernetes v1.34.
19977-19983: LGTM: Clear deprecation notice with appropriate timeline.The deprecation message clearly explains that profileCustomizations is no longer needed since DRA is GA and enabled by default in 4.21, with removal scheduled for 4.24 (3 releases away).
config/v1/types_scheduling.go (1)
116-116: LGTM.The updated default description accurately reflects the GA status of DRA in upstream Kubernetes 1.34.
config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
578-578: LGTM.The feature gate rename from
DynamicResourceAllocationtoHyperShiftOnlyDynamicResourceAllocationis correctly reflected in the CRD metadata.config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (2)
9-9: LGTM.The feature gate annotation correctly references
HyperShiftOnlyDynamicResourceAllocation.
113-127: LGTM.The deprecation notice and updated default description are consistent with the DRA GA status and the feature gate scope.
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
111-125: LGTM with prior comment.The deprecation notice and default description updates are consistent with the DRA GA status. Note the scope ambiguity flagged in the similar config/v1 CRD manifest.
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
1-122: No issues found — CRD structure is complete and intentional for TechPreviewNoUpgrade.Verification confirms the absence of
profileCustomizationsis intentional. WhileCustomNoUpgradeincludes this field (marked deprecated, removal planned for 4.24), it is correctly omitted fromTechPreviewNoUpgrade, consistent withDefaultandDevPreviewNoUpgradevariants. This represents deliberate feature-set differentiation rather than a missing field.openapi/openapi.json (2)
10238-10241: Verify feature gate name consistency in description.The description at line 10239 states "Dynamic resource allocation is...enabled by default in the cluster" and mentions "the related feature gate is graduated to GA in k8s 1.34 upstream." However, according to the AI-generated summary, the feature gate was replaced with
HyperShiftOnlyDynamicResourceAllocation.Confirm whether:
- The field name should remain
dynamicResourceAllocation(for upstream Kubernetes GA behavior).- The description should reference the correct gate name if OpenShift-specific behavior is documented here.
Consistency with the feature gate definitions in CRD manifests and config/v1 is critical for documentation and tooling accuracy.
10813-10816: Deprecation note is well-aligned with PR objectives.The deprecation message correctly notes that
profileCustomizationsis no longer needed since DRA is GA and enabled by default, with a removal target of 4.24. This aligns with the PR goal of enabling DRA by default and the referenced changes to CRD manifests and feature gate definitions.
| 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
Verify deprecation message appropriateness for SelfManagedHA.
This CRD manifest is for SelfManagedHA clusters (CustomNoUpgrade feature set). The deprecation message states DRA "is enabled by default in the cluster," but the associated feature gate is HyperShiftOnlyDynamicResourceAllocation, which by name suggests it's not enabled for SelfManagedHA.
If the profileCustomizations field is present in SelfManagedHA CRDs despite the HyperShiftOnly gate, the deprecation message may confuse users about whether DRA is actually enabled in their cluster type.
Run this script to verify whether HyperShiftOnlyDynamicResourceAllocation is enabled for SelfManagedHA feature sets:
🏁 Script executed:
#!/bin/bash
# Check which feature sets enable HyperShiftOnlyDynamicResourceAllocation
echo "=== Checking feature gate enablement across feature sets ==="
fd -e yaml . payload-manifests/featuregates/ --exec grep -l "HyperShiftOnlyDynamicResourceAllocation" {} \; | while read file; do
echo ""
echo "File: $file"
echo "Enabled gates:"
gawk '/featureSet:/{fs=$2} /enabled:/{flag=1; next} flag && /^[^ ]/{flag=0} flag && /HyperShiftOnlyDynamicResourceAllocation/{print " Feature set: " fs " - Gate enabled"}' "$file"
echo "Disabled gates:"
gawk '/featureSet:/{fs=$2} /disabled:/{flag=1; next} flag && /^[^ ]/{flag=0} flag && /HyperShiftOnlyDynamicResourceAllocation/{print " Feature set: " fs " - Gate disabled"}' "$file"
doneLength of output: 749
Running more detailed inspection to understand the structure and content:
🏁 Script executed:
#!/bin/bash
# Inspect the actual structure and content of feature gate files
echo "=== Inspecting SelfManagedHA feature gate files ==="
for file in payload-manifests/featuregates/featureGate-SelfManagedHA-*.yaml; do
if [ -f "$file" ]; then
echo ""
echo "File: $file"
echo "---"
# Check if HyperShiftOnlyDynamicResourceAllocation is mentioned at all
if grep -q "HyperShiftOnlyDynamicResourceAllocation" "$file"; then
echo "Found HyperShiftOnlyDynamicResourceAllocation in this file"
# Show context around the match
grep -n -B5 -A5 "HyperShiftOnlyDynamicResourceAllocation" "$file" | head -20
else
echo "HyperShiftOnlyDynamicResourceAllocation NOT found in this file"
fi
fi
done
echo ""
echo "=== Checking if profileCustomizations exists in SelfManagedHA CRD ==="
if grep -q "profileCustomizations" "payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml"; then
echo "profileCustomizations field FOUND in SelfManagedHA CRD"
grep -n -B3 -A5 "profileCustomizations" "payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml" | head -25
else
echo "profileCustomizations field NOT found in SelfManagedHA CRD"
fiLength of output: 2506
Need to clarify whether these gates are in the enabled or disabled lists:
🏁 Script executed:
#!/bin/bash
# Check the full structure and whether HyperShiftOnlyDynamicResourceAllocation is enabled or disabled
echo "=== Full inspection of SelfManagedHA-Default feature gate structure ==="
cat payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml | head -150
echo ""
echo "=== Checking context for HyperShiftOnlyDynamicResourceAllocation ==="
grep -n "enabled\|disabled" payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml | head -10Length of output: 5217
Verifying this pattern across all SelfManagedHA feature sets:
🏁 Script executed:
#!/bin/bash
# Check if HyperShiftOnlyDynamicResourceAllocation is in disabled list for all SelfManagedHA feature sets
echo "=== Checking enabled/disabled status in all SelfManagedHA feature gates ==="
for file in payload-manifests/featuregates/featureGate-SelfManagedHA-*.yaml; do
if [ -f "$file" ]; then
echo ""
echo "File: $(basename $file)"
# Extract the section containing disabled and enabled blocks with gates
python3 << 'PYTHON_EOF'
import json
import sys
file_path = sys.argv[1]
try:
with open(file_path, 'r') as f:
data = json.load(f)
for fg in data.get('status', {}).get('featureGates', []):
disabled_gates = [g['name'] for g in fg.get('disabled', [])]
enabled_gates = [g['name'] for g in fg.get('enabled', [])]
if 'HyperShiftOnlyDynamicResourceAllocation' in disabled_gates:
print(" Status: DISABLED")
elif 'HyperShiftOnlyDynamicResourceAllocation' in enabled_gates:
print(" Status: ENABLED")
else:
print(" Status: NOT FOUND")
except Exception as e:
print(f" Error: {e}")
PYTHON_EOF
python3 -c "
import json
with open('$file') as f:
data = json.load(f)
for fg in data.get('status', {}).get('featureGates', []):
disabled = [g['name'] for g in fg.get('disabled', [])]
enabled = [g['name'] for g in fg.get('enabled', [])]
if 'HyperShiftOnlyDynamicResourceAllocation' in disabled:
print(' Status: DISABLED')
elif 'HyperShiftOnlyDynamicResourceAllocation' in enabled:
print(' Status: ENABLED')
else:
print(' Status: NOT FOUND')
"
fi
doneLength of output: 622
Update the deprecation message to clarify DRA status for SelfManagedHA clusters.
The HyperShiftOnlyDynamicResourceAllocation feature gate is explicitly disabled in all SelfManagedHA feature sets (Default, DevPreviewNoUpgrade, TechPreviewNoUpgrade), contradicting the deprecation message that states DRA "is enabled by default in the cluster." The current message will mislead SelfManagedHA users about whether DRA is actually enabled in their environment.
The message should be clarified to either:
- Specify that the statement applies only to Hypershift clusters, or
- Accurately reflect that DRA is disabled/not available for SelfManagedHA
Update the description at lines 112-115 of payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml to provide accurate deprecation guidance for SelfManagedHA users.
sairameshv
force-pushed
the
enable_DRA
branch
from
60bb094 to
5ac3982
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
111-132:⚠️ Misleading deprecation message for SelfManagedHA cluster type.This file targets SelfManagedHA clusters (CustomNoUpgrade feature set), but the deprecation message states DRA is "enabled by default across all clusters." However,
HyperShiftOnlyDynamicResourceAllocationis explicitly disabled infeatureGate-SelfManagedHA-Default.yaml, meaning DRA is not enabled for SelfManagedHA environments. This creates confusion for SelfManagedHA users who will read that a field is deprecated because a feature is enabled by default, when it actually isn't in their cluster type.Consider clarifying the deprecation message to reflect SelfManagedHA reality:
- profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. - Deprecated: DRA is GA and enabled by default across all clusters starting with 4.21 - this configuration field is maintained only for HyperShift compatibility and will be removed in 4.24. + profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles. + Deprecated: DRA is GA and enabled by default in HyperShift clusters starting with 4.21. + This configuration field is maintained for compatibility and will be removed in 4.24.Alternatively, remove the
profileCustomizationsfield entirely from SelfManagedHA CRDs if DRA is not applicable (as shown in file 8 for DevPreviewNoUpgrade).config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml (1)
111-132:⚠️ Identical misleading message as in file 6.This generated CRD manifest has the same inconsistency: misleading deprecation message stating DRA is "enabled by default across all clusters" when
HyperShiftOnlyDynamicResourceAllocationis actually disabled for SelfManagedHA. Apply the same clarification as needed for file 6.The fix should mirror the resolution for
payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml. Clarify the message to "enabled by default in HyperShift clusters" or remove the field if not applicable to SelfManagedHA.openapi/openapi.json (1)
30634-30641:⚠️ closedClientConnectionPolicy addition is out of scope—duplicate concern from prior review.This hunk introduces
closedClientConnectionPolicyfor IngressController HAProxy configuration, which is unrelated to the PR's scope of enabling the DRA feature gate by default. This concern was already identified in a prior review comment with verification that this change originated in the current PR but is unrelated to DRA promotion.Recommendation: Extract this IngressController change to a separate PR. The
openapi.jsonfile is auto-generated viamake update, so regenerate it without theclosedClientConnectionPolicyaddition by reverting the underlying IngressController CRD/code changes from this branch, then create a focused PR for the HAProxy configuration enhancement. This keeps the DRA promotion review focused and reduces risk of unintended interactions between unrelated feature areas.
📜 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 (22)
config/v1/types_scheduling.go(2 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(2 hunks)config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(2 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(3 hunks)config/v1/zz_generated.swagger_doc_generated.go(2 hunks)features.md(1 hunks)features/features.go(1 hunks)openapi/generated_openapi/zz_generated.openapi.go(2 hunks)openapi/openapi.json(3 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml(2 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml(2 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)
- 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
🚧 Files skipped from review as they are similar to previous changes (8)
- config/v1/zz_generated.swagger_doc_generated.go
- config/v1/types_scheduling.go
- openapi/generated_openapi/zz_generated.openapi.go
- features/features.go
- features.md
- payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
- payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.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:
payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yamlconfig/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yamlpayload-manifests/featuregates/featureGate-SelfManagedHA-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.yamlconfig/v1/zz_generated.featuregated-crd-manifests.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yamlpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yamlopenapi/openapi.jsonpayload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-CustomNoUpgrade.crd.yaml
🔇 Additional comments (10)
config/v1/zz_generated.featuregated-crd-manifests.yaml (1)
570-591: Straightforward feature gate rename.The CRD metadata correctly reflects the gate rename from
DynamicResourceAllocationtoHyperShiftOnlyDynamicResourceAllocation, maintaining proper YAML structure and alphabetical ordering.payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml (1)
14-35: Correctly disables HyperShiftOnly gate for SelfManagedHA profile.The gate is appropriately placed in the disabled list for this SelfManagedHA variant, ensuring it does not apply to non-HyperShift clusters.
payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml (1)
69-358: Correctly enables HyperShiftOnly gate for HyperShift TechPreviewNoUpgrade profile.The gate is appropriately added to the enabled list for HyperShift clusters at this feature set level.
payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml (1)
14-230: Correctly disables HyperShiftOnly gate for SelfManagedHA default profile.The gate is appropriately placed in the disabled list, preventing it from applying to non-HyperShift cluster types.
config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml (1)
110-132: Deprecation messaging is accurate for HyperShift CRD.For the HyperShift variant, the deprecation note correctly reflects that DRA (via
HyperShiftOnlyDynamicResourceAllocation) is enabled by default and the field will be removed in 4.24. The updated default description aligns with GA status.payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml (1)
1-122: The review comment's inconsistency claim is factually incorrect.Verification confirms that
profileCustomizationsis present only in the CustomNoUpgrade variant. The current DevPreviewNoUpgrade file is consistent with two other SelfManagedHA variants (Default and TechPreviewNoUpgrade), all three of which correctly omit the field. CustomNoUpgrade is the outlier, not the standard.The current file follows the established pattern across the majority of SelfManagedHA variants. No changes are required.
Likely an incorrect or invalid review comment.
config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml (1)
9-9: Verify feature gate rename is applied consistently across the codebase.Line 9 shows the annotation key has been renamed from
feature-gate.release.openshift.io/DynamicResourceAllocationtofeature-gate.release.openshift.io/HyperShiftOnlyDynamicResourceAllocation. Please confirm this rename is reflected consistently across all related feature-gate definitions, feature lists, and any downstream code that references the old gate name.payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml (1)
1-122: Verify intentional omission ofprofileCustomizationsfield in TechPreviewNoUpgrade variant.This new file omits the
profileCustomizationsfield that is present in the Hypershift variant (File 2). Please confirm this is intentional. If TechPreviewNoUpgrade should support the same configuration options, theprofileCustomizationsfield should be added for consistency. If this omission is intentional (e.g., the field is not exposed in tech preview), please document the design rationale.openapi/openapi.json (2)
10236-10242: ✓ DRA GA promotion description update aligns with PR objectives.The revised description for
dynamicResourceAllocationaccurately reflects the GA status in Kubernetes 1.34 and clarifies that the default is nowEnabled. This is consistent with the PR's goal to enable DRA by default in OpenShift.
10811-10817: ✓ Deprecation notice for profileCustomizations is clear and forward-compatible.The updated description correctly marks
profileCustomizationsas deprecated for HyperShift compatibility only, with removal planned for 4.24. This aligns with the PR's promotion of DRA to GA and provides clear migration guidance.
...ed-crd-manifests/schedulers.config.openshift.io/HyperShiftOnlyDynamicResourceAllocation.yaml
Outdated
Show resolved
Hide resolved
payload-manifests/crds/0000_10_config-operator_01_schedulers-Hypershift.crd.yaml
Outdated
Show resolved
Hide resolved
sairameshv
force-pushed
the
enable_DRA
branch
from
5ac3982 to
a1d5082
Compare
|
/hold cancel |
openshift-ci
bot
removed
the
do-not-merge/hold
- DynamicResourceAllocation feature has been graduated to GA in the upstream i.e. k8s 1.34 - Removed the downstream featuregate DynamicResourceAllocation so that this is enabled by default for all the cluster types - Remove the downstream featuregate dependency from the scheduler.config's ProfileCustomizations field Signed-off-by: Sai Ramesh Vanka <[email protected]>
Signed-off-by: Sai Ramesh Vanka <[email protected]>
sairameshv
force-pushed
the
enable_DRA
branch
from
a1d5082 to
bb45b13
Compare
|
/retest |
|
@sairameshv: The following tests failed, say
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. |
11 participants
DRA has been graduated to GA in upstream 1.34[1] and is required to be enabled for OCP-4.21 rebase to unblock the DRA based e2e test failures[2]
[1] - https://kubernetes.io/blog/2025/09/01/kubernetes-v1-34-dra-updates/
[2] - https://issues.redhat.com/browse/OCPBUGS-61381