-
Notifications
You must be signed in to change notification settings - Fork 580
Move ProfileCustomizations under a new GA gate #2590
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| apiVersion: apiextensions.k8s.io/v1 # Hack because controller-gen complains if we don't have this | ||
| name: "Scheduler" | ||
| crdName: schedulers.config.openshift.io | ||
| featureGates: | ||
| - HyperShiftOnlyDynamicResourceAllocation | ||
| tests: | ||
| onCreate: | ||
| - name: Should be able to create a minimal Scheduler | ||
| initial: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Scheduler | ||
| spec: {} # No spec is required for a Scheduler | ||
| expected: | | ||
| apiVersion: config.openshift.io/v1 | ||
| kind: Scheduler | ||
| spec: {} |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,7 +48,10 @@ type SchedulerSpec struct { | |
| // +optional | ||
| Profile SchedulerProfile `json:"profile,omitempty"` | ||
| // 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. | ||
| // +openshift:enable:FeatureGate=DynamicResourceAllocation | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Keep this around so we have both gates at the same time please. Once we get hypershift updated to use the new gate, we can remove this marker. |
||
| // +openshift:enable:FeatureGate=HyperShiftOnlyDynamicResourceAllocation | ||
|
Comment on lines
51
to
+54
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like the Because Maybe we make this new gate also DPNU and TPNU in standalone just for the transition period so we don't permanently break the |
||
| // +optional | ||
| ProfileCustomizations ProfileCustomizations `json:"profileCustomizations"` | ||
| // defaultNodeSelector helps set the cluster-wide default node selector to | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| apiVersion: apiextensions.k8s.io/v1 | ||
| kind: CustomResourceDefinition | ||
| metadata: | ||
| annotations: | ||
| api-approved.openshift.io: https://github.com/openshift/api/pull/470 | ||
| api.openshift.io/merged-by-featuregates: "true" | ||
| include.release.openshift.io/ibm-cloud-managed: "true" | ||
| release.openshift.io/bootstrap-required: "true" | ||
| name: schedulers.config.openshift.io | ||
| spec: | ||
| group: config.openshift.io | ||
| names: | ||
| kind: Scheduler | ||
| listKind: SchedulerList | ||
| plural: schedulers | ||
| singular: scheduler | ||
| scope: Cluster | ||
| versions: | ||
| - name: v1 | ||
| schema: | ||
| openAPIV3Schema: | ||
| description: |- | ||
| Scheduler holds cluster-wide config information to run the Kubernetes Scheduler | ||
| and influence its placement decisions. The canonical name for this config is `cluster`. | ||
|
|
||
| Compatibility level 1: Stable within a major release for a minimum of 12 months or 3 minor releases (whichever is longer). | ||
| properties: | ||
| apiVersion: | ||
| description: |- | ||
| APIVersion defines the versioned schema of this representation of an object. | ||
| Servers should convert recognized schemas to the latest internal value, and | ||
| may reject unrecognized values. | ||
| More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#resources | ||
| type: string | ||
| kind: | ||
| description: |- | ||
| Kind is a string value representing the REST resource this object represents. | ||
| Servers may infer this from the endpoint the client submits requests to. | ||
| Cannot be updated. | ||
| In CamelCase. | ||
| More info: https://git.k8s.io/community/contributors/devel/sig-architecture/api-conventions.md#types-kinds | ||
| type: string | ||
| metadata: | ||
| type: object | ||
| spec: | ||
| description: spec holds user settable values for configuration | ||
| properties: | ||
| defaultNodeSelector: | ||
| description: |- | ||
| 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. | ||
| type: string | ||
| mastersSchedulable: | ||
| description: |- | ||
| 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. | ||
| type: boolean | ||
| policy: | ||
| description: |- | ||
| 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. | ||
| properties: | ||
| name: | ||
| description: name is the metadata.name of the referenced config | ||
| map | ||
| type: string | ||
| required: | ||
| - name | ||
| type: object | ||
| profile: | ||
| description: |- | ||
| profile sets which scheduling profile should be set in order to configure scheduling | ||
| decisions for new pods. | ||
|
|
||
| Valid values are "LowNodeUtilization", "HighNodeUtilization", "NoScoring" | ||
| Defaults to "LowNodeUtilization" | ||
| enum: | ||
| - "" | ||
| - LowNodeUtilization | ||
| - HighNodeUtilization | ||
| - NoScoring | ||
| type: string | ||
| 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. | ||
|
Comment on lines
+111
to
+114
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fix typo in deprecation notice. Line 114 contains 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 |
||
| properties: | ||
| dynamicResourceAllocation: | ||
| description: |- | ||
| dynamicResourceAllocation allows to enable or disable dynamic resource allocation within the scheduler. | ||
| Dynamic resource allocation is an API for requesting and sharing resources between pods and containers inside a pod. | ||
| Third-party resource drivers are responsible for tracking and allocating resources. | ||
| Different kinds of resources support arbitrary parameters for defining requirements and initialization. | ||
| Valid values are Enabled, Disabled and omitted. | ||
| When omitted, this means no opinion and the platform is left to choose a reasonable default, | ||
| which is subject to change over time. | ||
| The current default is Disabled. | ||
| enum: | ||
| - "" | ||
| - Enabled | ||
| - Disabled | ||
| type: string | ||
| type: object | ||
| type: object | ||
| status: | ||
| description: status holds observed values from the cluster. They may not | ||
| be overridden. | ||
| type: object | ||
| required: | ||
| - spec | ||
| type: object | ||
| served: true | ||
| storage: true | ||
| subresources: | ||
| status: {} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ metadata: | |
| annotations: | ||
| api-approved.openshift.io: https://github.com/openshift/api/pull/470 | ||
| api.openshift.io/merged-by-featuregates: "true" | ||
| include.release.openshift.io/ibm-cloud-managed: "true" | ||
| include.release.openshift.io/self-managed-high-availability: "true" | ||
| release.openshift.io/bootstrap-required: "true" | ||
| release.openshift.io/feature-set: CustomNoUpgrade | ||
|
|
@@ -110,8 +109,10 @@ spec: | |
| - NoScoring | ||
| type: string | ||
| 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. | ||
|
Comment on lines
+112
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
| properties: | ||
| dynamicResourceAllocation: | ||
| description: |- | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,7 +4,6 @@ metadata: | |
| annotations: | ||
| api-approved.openshift.io: https://github.com/openshift/api/pull/470 | ||
| api.openshift.io/merged-by-featuregates: "true" | ||
| include.release.openshift.io/ibm-cloud-managed: "true" | ||
| include.release.openshift.io/self-managed-high-availability: "true" | ||
| release.openshift.io/bootstrap-required: "true" | ||
| release.openshift.io/feature-set: TechPreviewNoUpgrade | ||
|
|
@@ -110,8 +109,10 @@ spec: | |
| - NoScoring | ||
| type: string | ||
| 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. | ||
|
Comment on lines
+112
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainTypo in generated CRD manifest. Line 115 contains the same typo as in the source file: 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 🤖 Prompt for AI Agents |
||
| properties: | ||
| dynamicResourceAllocation: | ||
| description: |- | ||
|
|
||
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.
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?