Skip to content

Commit

Permalink
Merge pull request #251 from RadekManak/exposeIgnoredLabelsInCRD
Browse files Browse the repository at this point in the history
OCPCLOUD-1427: Expose autoscaler "--balancing-ignore-label" flag through CRD
  • Loading branch information
openshift-merge-robot authored Sep 29, 2022
2 parents 9587e0b + 12ff9c1 commit a733b3f
Show file tree
Hide file tree
Showing 5 changed files with 33 additions and 2 deletions.
13 changes: 12 additions & 1 deletion install/01_clusterautoscaler.crd.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,10 +41,21 @@ spec:
properties:
balanceSimilarNodeGroups:
description: BalanceSimilarNodeGroups enables/disables the `--balance-similar-node-groups`
cluster-autocaler feature. This feature will automatically identify
cluster-autoscaler feature. This feature will automatically identify
node groups with the same instance type and the same set of labels
and try to keep the respective sizes of those node groups balanced.
type: boolean
balancingIgnoredLabels:
description: BalancingIgnoredLabels sets "--balancing-ignore-label
<label name>" flag on cluster-autoscaler for each listed label.
This option specifies labels that cluster autoscaler should ignore
when considering node group similarity. For example, if you have
nodes with "topology.ebs.csi.aws.com/zone" label, you can add name
of this label here to prevent cluster autoscaler from spliting nodes
into different node groups based on its value.
items:
type: string
type: array
ignoreDaemonsetsUtilization:
description: Enables/Disables `--ignore-daemonsets-utilization` CA
feature flag. Should CA ignore DaemonSet pods when calculating resource
Expand Down
8 changes: 7 additions & 1 deletion pkg/apis/autoscaling/v1/clusterautoscaler_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,18 @@ type ClusterAutoscalerSpec struct {
PodPriorityThreshold *int32 `json:"podPriorityThreshold,omitempty"`

// BalanceSimilarNodeGroups enables/disables the
// `--balance-similar-node-groups` cluster-autocaler feature.
// `--balance-similar-node-groups` cluster-autoscaler feature.
// This feature will automatically identify node groups with
// the same instance type and the same set of labels and try
// to keep the respective sizes of those node groups balanced.
BalanceSimilarNodeGroups *bool `json:"balanceSimilarNodeGroups,omitempty"`

// BalancingIgnoredLabels sets "--balancing-ignore-label <label name>" flag on cluster-autoscaler for each listed label.
// This option specifies labels that cluster autoscaler should ignore when considering node group similarity.
// For example, if you have nodes with "topology.ebs.csi.aws.com/zone" label, you can add name of this label here
// to prevent cluster autoscaler from spliting nodes into different node groups based on its value.
BalancingIgnoredLabels []string `json:"balancingIgnoredLabels,omitempty"`

// Enables/Disables `--ignore-daemonsets-utilization` CA feature flag. Should CA ignore DaemonSet pods when calculating resource utilization for scaling down. false by default
IgnoreDaemonsetsUtilization *bool `json:"ignoreDaemonsetsUtilization,omitempty"`

Expand Down
5 changes: 5 additions & 0 deletions pkg/apis/autoscaling/v1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 5 additions & 0 deletions pkg/controller/clusterautoscaler/clusterautoscaler.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ const (
GPUTotalArg AutoscalerArg = "--gpu-total"
VerbosityArg AutoscalerArg = "--v"
BalanceSimilarNodeGroupsArg AutoscalerArg = "--balance-similar-node-groups"
BalancingIgnoreLabelArg AutoscalerArg = "--balancing-ignore-label"
IgnoreDaemonsetsUtilization AutoscalerArg = "--ignore-daemonsets-utilization"
SkipNodesWithLocalStorage AutoscalerArg = "--skip-nodes-with-local-storage"
LeaderElectLeaseDurationArg AutoscalerArg = "--leader-elect-lease-duration"
Expand Down Expand Up @@ -116,6 +117,10 @@ func AutoscalerArgs(ca *v1.ClusterAutoscaler, cfg *Config) []string {
args = append(args, SkipNodesWithLocalStorage.Value(*ca.Spec.SkipNodesWithLocalStorage))
}

for _, ignoredLabel := range ca.Spec.BalancingIgnoredLabels {
args = append(args, BalancingIgnoreLabelArg.Value(ignoredLabel))
}

// Prefer log level set from ClousterAutoscaler resource
if ca.Spec.LogVerbosity != nil {
args = append(args, VerbosityArg.Value(*ca.Spec.LogVerbosity))
Expand Down
4 changes: 4 additions & 0 deletions pkg/controller/clusterautoscaler/clusterautoscaler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,7 @@ func TestAutoscalerArgs(t *testing.T) {
"--balance-similar-node-groups",
"--ignore-daemonsets-utilization",
"--skip-nodes-with-local-storage",
"--balancing-ignore-label",
}

for _, e := range expectedMissing {
Expand All @@ -191,6 +192,7 @@ func TestAutoscalerArgEnabled(t *testing.T) {
ca.Spec.IgnoreDaemonsetsUtilization = pointer.BoolPtr(true)
ca.Spec.SkipNodesWithLocalStorage = pointer.BoolPtr(true)
ca.Spec.MaxNodeProvisionTime = MaxNodeProvisionTime
ca.Spec.BalancingIgnoredLabels = []string{"test/ignoredLabel", "test/anotherIgnoredLabel"}

args := AutoscalerArgs(ca, &Config{CloudProvider: TestCloudProvider, Namespace: TestNamespace})

Expand All @@ -199,6 +201,8 @@ func TestAutoscalerArgEnabled(t *testing.T) {
fmt.Sprintf("--ignore-daemonsets-utilization=true"),
fmt.Sprintf("--skip-nodes-with-local-storage=true"),
fmt.Sprintf("--max-node-provision-time=%s", MaxNodeProvisionTime),
fmt.Sprintf("--balancing-ignore-label=test/ignoredLabel"),
fmt.Sprintf("--balancing-ignore-label=test/anotherIgnoredLabel"),
}

for _, e := range expected {
Expand Down

0 comments on commit a733b3f

Please sign in to comment.