-
Notifications
You must be signed in to change notification settings - Fork 582
Remove the DynamicResourceAllocation feature gate #2601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
- DRA is GA, and is enable by default - no downstream component refers to the gate any longer With the above, it is safe to delete the gate
|
Pipeline controller notification For optional jobs, comment This repository is configured in: LGTM mode |
WalkthroughThis change removes the DynamicResourceAllocation feature gate from OpenShift scheduler configuration. The feature gate annotation is removed from Go type definitions, the feature gate declaration is removed from the features package, CRD manifests are updated to remove the profileCustomizations field across multiple feature gate levels, feature gate configuration files are updated to remove DynamicResourceAllocation entries, and the OpenAPI schema is updated with a deprecation notice. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
|
Hello @tkashem! Some important instructions when contributing to openshift/api: |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (16)
config/v1/types_scheduling.go(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(0 hunks)config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/DynamicResourceAllocation.yaml(0 hunks)features.md(0 hunks)features/features.go(0 hunks)openapi/openapi.json(1 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml(0 hunks)payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.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 (15)
- payload-manifests/featuregates/featureGate-SelfManagedHA-DevPreviewNoUpgrade.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- payload-manifests/featuregates/featureGate-Hypershift-TechPreviewNoUpgrade.yaml
- config/v1/zz_generated.crd-manifests/0000_10_config-operator_01_schedulers-SelfManagedHA-DevPreviewNoUpgrade.crd.yaml
- config/v1/types_scheduling.go
- payload-manifests/featuregates/featureGate-Hypershift-DevPreviewNoUpgrade.yaml
- config/v1/zz_generated.featuregated-crd-manifests/schedulers.config.openshift.io/DynamicResourceAllocation.yaml
- features.md
- payload-manifests/featuregates/featureGate-SelfManagedHA-TechPreviewNoUpgrade.yaml
- features/features.go
- config/v1/zz_generated.featuregated-crd-manifests.yaml
- payload-manifests/featuregates/featureGate-Hypershift-Default.yaml
- payload-manifests/crds/0000_10_config-operator_01_schedulers-SelfManagedHA-TechPreviewNoUpgrade.crd.yaml
- payload-manifests/featuregates/featureGate-SelfManagedHA-Default.yaml
- payload-manifests/crds/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:
openapi/openapi.json
| }, | ||
| "profileCustomizations": { | ||
| "description": "profileCustomizations contains configuration for modifying the default behavior of existing scheduler profiles.", | ||
| "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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix typo in deprecation description.
The description string contains a syntax error: "by' default" should be "by default" (the errant quote character 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.",
+ "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.",📝 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 openapi/openapi.json around line 10814, the deprecation description contains
a typo: the substring "by' default" includes an errant single quote; remove the
stray quote so it reads "by default" and update the JSON string accordingly,
ensuring proper JSON escaping and that the surrounding punctuation/spacing
remains unchanged.
|
@tkashem: 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. |
JoelSpeed
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please fix the typo coderabbit picked up and then we can get this merged
Remove the DynamicResourceAllocation feature gate