Skip to content

Commit 8313e96

Browse files
adammwadammw
committed
Add support for passing in custom ignore labels
1 parent 5476125 commit 8313e96

File tree

12 files changed

+123
-51
lines changed

12 files changed

+123
-51
lines changed

‎cluster-autoscaler/config/autoscaling_options.go‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,9 @@ type AutoscalingOptions struct {
135135
MaxBulkSoftTaintTime time.Duration
136136
// IgnoredTaints is a list of taints to ignore when considering a node template for scheduling.
137137
IgnoredTaints []string
138+
// BalancingExtraIgnoredLabels is a list of labels to additionally ignore when comparing if two node groups are similar.
139+
// Labels in BasicIgnoredLabels and the cloud provider-specific ignored labels are always ignored.
140+
BalancingExtraIgnoredLabels []string
138141
// AWSUseStaticInstanceList tells if AWS cloud provider use static instance type list or dynamically fetch from remote APIs.
139142
AWSUseStaticInstanceList bool
140143
// Path to kube configuration if available

‎cluster-autoscaler/core/scale_test_common.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ func NewTestProcessors() *processors.AutoscalingProcessors {
132132
return &processors.AutoscalingProcessors{
133133
PodListProcessor: NewFilterOutSchedulablePodListProcessor(),
134134
NodeGroupListProcessor: &nodegroups.NoOpNodeGroupListProcessor{},
135-
NodeGroupSetProcessor: &nodegroupset.BalancingNodeGroupSetProcessor{},
135+
NodeGroupSetProcessor: nodegroupset.NewDefaultNodeGroupSetProcessor([]string{}),
136136
// TODO(bskiba): change scale up test so that this can be a NoOpProcessor
137137
ScaleUpStatusProcessor: &status.EventingScaleUpStatusProcessor{},
138138
ScaleDownStatusProcessor: &status.NoOpScaleDownStatusProcessor{},

‎cluster-autoscaler/main.go‎

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -168,9 +168,10 @@ var (
168168
regional = flag.Bool("regional", false, "Cluster is regional.")
169169
newPodScaleUpDelay = flag.Duration("new-pod-scale-up-delay", 0*time.Second, "Pods less than this old will not be considered for scale-up.")
170170

171-
ignoreTaintsFlag = multiStringFlag("ignore-taint", "Specifies a taint to ignore in node templates when considering to scale a node group")
172-
awsUseStaticInstanceList = flag.Bool("aws-use-static-instance-list", false, "Should CA fetch instance types in runtime or use a static list. AWS only")
173-
enableProfiling = flag.Bool("profiling", false, "Is debug/pprof endpoint enabled")
171+
ignoreTaintsFlag = multiStringFlag("ignore-taint", "Specifies a taint to ignore in node templates when considering to scale a node group")
172+
balancingIgnoreLabelsFlag = multiStringFlag("balancing-ignore-label", "Specifies a label to ignore in addition to the basic and cloud-provider set of labels when comparing if two node groups are similar")
173+
awsUseStaticInstanceList = flag.Bool("aws-use-static-instance-list", false, "Should CA fetch instance types in runtime or use a static list. AWS only")
174+
enableProfiling = flag.Bool("profiling", false, "Is debug/pprof endpoint enabled")
174175
)
175176

176177
func createAutoscalingOptions() config.AutoscalingOptions {
@@ -235,6 +236,7 @@ func createAutoscalingOptions() config.AutoscalingOptions {
235236
Regional: *regional,
236237
NewPodScaleUpDelay: *newPodScaleUpDelay,
237238
IgnoredTaints: *ignoreTaintsFlag,
239+
BalancingExtraIgnoredLabels: *balancingIgnoreLabelsFlag,
238240
KubeConfigPath: *kubeConfigFile,
239241
NodeDeletionDelayTimeout: *nodeDeletionDelayTimeout,
240242
AWSUseStaticInstanceList: *awsUseStaticInstanceList,
@@ -289,21 +291,24 @@ func buildAutoscaler() (core.Autoscaler, error) {
289291
kubeClient := createKubeClient(getKubeConfig())
290292
eventsKubeClient := createKubeClient(getKubeConfig())
291293

292-
processors := ca_processors.DefaultProcessors()
293-
processors.PodListProcessor = core.NewFilterOutSchedulablePodListProcessor()
294-
if autoscalingOptions.CloudProviderName == cloudprovider.AzureProviderName {
295-
processors.NodeGroupSetProcessor = &nodegroupset.BalancingNodeGroupSetProcessor{
296-
Comparator: nodegroupset.CreateAzureNodeInfoComparator()}
297-
} else if autoscalingOptions.CloudProviderName == cloudprovider.AwsProviderName {
298-
processors.NodeGroupSetProcessor = &nodegroupset.BalancingNodeGroupSetProcessor{
299-
Comparator: nodegroupset.CreateAwsNodeInfoComparator()}
300-
}
301-
302294
opts := core.AutoscalerOptions{
303295
AutoscalingOptions: autoscalingOptions,
304296
KubeClient: kubeClient,
305297
EventsKubeClient: eventsKubeClient,
306-
Processors: processors,
298+
}
299+
300+
opts.Processors = ca_processors.DefaultProcessors()
301+
opts.Processors.PodListProcessor = core.NewFilterOutSchedulablePodListProcessor()
302+
303+
nodeInfoComparatorBuilder := nodegroupset.CreateGenericNodeInfoComparator
304+
if autoscalingOptions.CloudProviderName == cloudprovider.AzureProviderName {
305+
nodeInfoComparatorBuilder = nodegroupset.CreateAzureNodeInfoComparator
306+
} else if autoscalingOptions.CloudProviderName == cloudprovider.AwsProviderName {
307+
nodeInfoComparatorBuilder = nodegroupset.CreateAwsNodeInfoComparator
308+
}
309+
310+
opts.Processors.NodeGroupSetProcessor = &nodegroupset.BalancingNodeGroupSetProcessor{
311+
Comparator: nodeInfoComparatorBuilder(autoscalingOptions.BalancingExtraIgnoredLabels),
307312
}
308313

309314
// This metric should be published only once.

‎cluster-autoscaler/processors/nodegroupset/aws_nodegroups.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,10 @@ import (
2020
schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
2121
)
2222

23-
func CreateAwsNodeInfoComparator() NodeInfoComparator {
23+
// CreateAwsNodeInfoComparator returns a comparator that checks if two nodes should be considered
24+
// part of the same NodeGroupSet. This is true if they match usual conditions checked by IsCloudProviderNodeInfoSimilar,
25+
// even if they have different AWS-specific labels.
26+
func CreateAwsNodeInfoComparator(extraIgnoredLabels []string) NodeInfoComparator {
2427
awsIgnoredLabels := map[string]bool{
2528
"alpha.eksctl.io/instance-id": true, // this is a label used by eksctl to identify instances.
2629
"alpha.eksctl.io/nodegroup-name": true, // this is a label used by eksctl to identify "node group" names.
@@ -33,6 +36,10 @@ func CreateAwsNodeInfoComparator() NodeInfoComparator {
3336
awsIgnoredLabels[k] = v
3437
}
3538

39+
for _, k := range extraIgnoredLabels {
40+
awsIgnoredLabels[k] = true
41+
}
42+
3643
return func(n1, n2 *schedulernodeinfo.NodeInfo) bool {
3744
return IsCloudProviderNodeInfoSimilar(n1, n2, awsIgnoredLabels)
3845
}

‎cluster-autoscaler/processors/nodegroupset/azure_nodegroups.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,18 @@ func nodesFromSameAzureNodePool(n1, n2 *schedulernodeinfo.NodeInfo) bool {
2929
return n1AzureNodePool != "" && n1AzureNodePool == n2AzureNodePool
3030
}
3131

32-
// Returned NodeInfoComparator compares if two nodes should be considered part of the
33-
// same NodeGroupSet. This is true if they either belong to the same Azure agentpool
32+
// CreateAzureNodeInfoComparator returns a comparator that checks if two nodes should be considered
33+
// part of the same NodeGroupSet. This is true if they either belong to the same Azure agentpool
3434
// or match usual conditions checked by IsCloudProviderNodeInfoSimilar, even if they have different agentpool labels.
35-
func CreateAzureNodeInfoComparator() NodeInfoComparator {
35+
func CreateAzureNodeInfoComparator(extraIgnoredLabels []string) NodeInfoComparator {
3636
azureIgnoredLabels := make(map[string]bool)
3737
for k, v := range BasicIgnoredLabels {
3838
azureIgnoredLabels[k] = v
3939
}
4040
azureIgnoredLabels[AzureNodepoolLabel] = true
41+
for _, k := range extraIgnoredLabels {
42+
azureIgnoredLabels[k] = true
43+
}
4144

4245
return func(n1, n2 *schedulernodeinfo.NodeInfo) bool {
4346
if nodesFromSameAzureNodePool(n1, n2) {

‎cluster-autoscaler/processors/nodegroupset/azure_nodegroups_test.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import (
2929
)
3030

3131
func TestIsAzureNodeInfoSimilar(t *testing.T) {
32-
comparator := CreateAzureNodeInfoComparator()
32+
comparator := CreateAzureNodeInfoComparator([]string{"example.com/ready"})
3333
n1 := BuildTestNode("node1", 1000, 2000)
3434
n1.ObjectMeta.Labels["test-label"] = "test-value"
3535
n1.ObjectMeta.Labels["character"] = "thing"
@@ -62,15 +62,21 @@ func TestIsAzureNodeInfoSimilar(t *testing.T) {
6262
n1.ObjectMeta.Labels["agentpool"] = "foo"
6363
n2.ObjectMeta.Labels["agentpool"] = "bar"
6464
checkNodesSimilar(t, n1, n2, comparator, true)
65+
// Custom label
66+
n1.ObjectMeta.Labels["example.com/ready"] = "true"
67+
n2.ObjectMeta.Labels["example.com/ready"] = "false"
68+
checkNodesSimilar(t, n1, n2, comparator, true)
6569
}
6670

6771
func TestFindSimilarNodeGroupsAzureBasic(t *testing.T) {
68-
processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAzureNodeInfoComparator()}
69-
basicSimilarNodeGroupsTest(t, processor)
72+
context := &context.AutoscalingContext{}
73+
ni1, ni2, ni3 := buildBasicNodeGroups(context)
74+
processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAzureNodeInfoComparator([]string{})}
75+
basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3)
7076
}
7177

7278
func TestFindSimilarNodeGroupsAzureByLabel(t *testing.T) {
73-
processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAzureNodeInfoComparator()}
79+
processor := &BalancingNodeGroupSetProcessor{Comparator: CreateAzureNodeInfoComparator([]string{})}
7480
context := &context.AutoscalingContext{}
7581

7682
n1 := BuildTestNode("n1", 1000, 1000)

‎cluster-autoscaler/processors/nodegroupset/balancing_processor.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func (b *BalancingNodeGroupSetProcessor) FindSimilarNodeGroups(context *context.
5858
}
5959
comparator := b.Comparator
6060
if comparator == nil {
61-
panic("BalancingNodeGroupSetProcessor comparator not set")
61+
klog.Fatal("BalancingNodeGroupSetProcessor comparator not set")
6262
}
6363
if comparator(nodeInfo, ngNodeInfo) {
6464
result = append(result, ng)

‎cluster-autoscaler/processors/nodegroupset/balancing_processor_test.go‎

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,17 @@ package nodegroupset
1919
import (
2020
"testing"
2121

22+
schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
23+
2224
"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
2325
testprovider "k8s.io/autoscaler/cluster-autoscaler/cloudprovider/test"
2426
"k8s.io/autoscaler/cluster-autoscaler/context"
2527
. "k8s.io/autoscaler/cluster-autoscaler/utils/test"
26-
schedulernodeinfo "k8s.io/kubernetes/pkg/scheduler/nodeinfo"
2728

2829
"github.com/stretchr/testify/assert"
2930
)
3031

31-
func basicSimilarNodeGroupsTest(t *testing.T, processor NodeGroupSetProcessor) {
32-
context := &context.AutoscalingContext{}
33-
32+
func buildBasicNodeGroups(context *context.AutoscalingContext) (*schedulernodeinfo.NodeInfo, *schedulernodeinfo.NodeInfo, *schedulernodeinfo.NodeInfo) {
3433
n1 := BuildTestNode("n1", 1000, 1000)
3534
n2 := BuildTestNode("n2", 1000, 1000)
3635
n3 := BuildTestNode("n3", 2000, 2000)
@@ -49,35 +48,71 @@ func basicSimilarNodeGroupsTest(t *testing.T, processor NodeGroupSetProcessor) {
4948
ni3 := schedulernodeinfo.NewNodeInfo()
5049
ni3.SetNode(n3)
5150

51+
context.CloudProvider = provider
52+
return ni1, ni2, ni3
53+
}
54+
55+
func basicSimilarNodeGroupsTest(
56+
t *testing.T,
57+
context *context.AutoscalingContext,
58+
processor NodeGroupSetProcessor,
59+
ni1 *schedulernodeinfo.NodeInfo,
60+
ni2 *schedulernodeinfo.NodeInfo,
61+
ni3 *schedulernodeinfo.NodeInfo,
62+
) {
5263
nodeInfosForGroups := map[string]*schedulernodeinfo.NodeInfo{
5364
"ng1": ni1, "ng2": ni2, "ng3": ni3,
5465
}
5566

56-
ng1, _ := provider.NodeGroupForNode(n1)
57-
ng2, _ := provider.NodeGroupForNode(n2)
58-
ng3, _ := provider.NodeGroupForNode(n3)
59-
context.CloudProvider = provider
67+
ng1, _ := context.CloudProvider.NodeGroupForNode(ni1.Node())
68+
ng2, _ := context.CloudProvider.NodeGroupForNode(ni2.Node())
69+
ng3, _ := context.CloudProvider.NodeGroupForNode(ni3.Node())
6070

6171
similar, err := processor.FindSimilarNodeGroups(context, ng1, nodeInfosForGroups)
6272
assert.NoError(t, err)
63-
assert.Equal(t, similar, []cloudprovider.NodeGroup{ng2})
73+
assert.Equal(t, []cloudprovider.NodeGroup{ng2}, similar)
6474

6575
similar, err = processor.FindSimilarNodeGroups(context, ng2, nodeInfosForGroups)
6676
assert.NoError(t, err)
67-
assert.Equal(t, similar, []cloudprovider.NodeGroup{ng1})
77+
assert.Equal(t, []cloudprovider.NodeGroup{ng1}, similar)
6878

6979
similar, err = processor.FindSimilarNodeGroups(context, ng3, nodeInfosForGroups)
7080
assert.NoError(t, err)
71-
assert.Equal(t, similar, []cloudprovider.NodeGroup{})
81+
assert.Equal(t, []cloudprovider.NodeGroup{}, similar)
7282
}
7383

7484
func TestFindSimilarNodeGroups(t *testing.T) {
75-
processor := NewDefaultNodeGroupSetProcessor()
76-
basicSimilarNodeGroupsTest(t, processor)
85+
context := &context.AutoscalingContext{}
86+
ni1, ni2, ni3 := buildBasicNodeGroups(context)
87+
processor := NewDefaultNodeGroupSetProcessor([]string{})
88+
basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3)
89+
}
90+
91+
func TestFindSimilarNodeGroupsCustomLabels(t *testing.T) {
92+
context := &context.AutoscalingContext{}
93+
ni1, ni2, ni3 := buildBasicNodeGroups(context)
94+
ni1.Node().Labels["example.com/ready"] = "true"
95+
ni2.Node().Labels["example.com/ready"] = "false"
96+
97+
processor := NewDefaultNodeGroupSetProcessor([]string{"example.com/ready"})
98+
basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3)
99+
}
100+
101+
func TestFindSimilarNodeGroupsCustomComparator(t *testing.T) {
102+
context := &context.AutoscalingContext{}
103+
ni1, ni2, ni3 := buildBasicNodeGroups(context)
104+
105+
processor := &BalancingNodeGroupSetProcessor{
106+
Comparator: func(n1, n2 *schedulernodeinfo.NodeInfo) bool {
107+
return (n1.Node().Name == "n1" && n2.Node().Name == "n2") ||
108+
(n1.Node().Name == "n2" && n2.Node().Name == "n1")
109+
},
110+
}
111+
basicSimilarNodeGroupsTest(t, context, processor, ni1, ni2, ni3)
77112
}
78113

79114
func TestBalanceSingleGroup(t *testing.T) {
80-
processor := NewDefaultNodeGroupSetProcessor()
115+
processor := NewDefaultNodeGroupSetProcessor([]string{})
81116
context := &context.AutoscalingContext{}
82117

83118
provider := testprovider.NewTestCloudProvider(nil, nil)
@@ -97,7 +132,7 @@ func TestBalanceSingleGroup(t *testing.T) {
97132
}
98133

99134
func TestBalanceUnderMaxSize(t *testing.T) {
100-
processor := NewDefaultNodeGroupSetProcessor()
135+
processor := NewDefaultNodeGroupSetProcessor([]string{})
101136
context := &context.AutoscalingContext{}
102137

103138
provider := testprovider.NewTestCloudProvider(nil, nil)
@@ -147,7 +182,7 @@ func TestBalanceUnderMaxSize(t *testing.T) {
147182
}
148183

149184
func TestBalanceHittingMaxSize(t *testing.T) {
150-
processor := NewDefaultNodeGroupSetProcessor()
185+
processor := NewDefaultNodeGroupSetProcessor([]string{})
151186
context := &context.AutoscalingContext{}
152187

153188
provider := testprovider.NewTestCloudProvider(nil, nil)

‎cluster-autoscaler/processors/nodegroupset/compare_nodegroups.go‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,19 @@ func compareLabels(nodes []*schedulernodeinfo.NodeInfo, ignoredLabels map[string
8585
}
8686
return true
8787
}
88+
8889
// CreateGenericNodeInfoComparator returns a generic comparator that checks for node group similarity
89-
// based on a standard set of widely-applicable ignore labels
90-
func CreateGenericNodeInfoComparator() NodeInfoComparator {
90+
func CreateGenericNodeInfoComparator(extraIgnoredLabels []string) NodeInfoComparator {
91+
genericIgnoredLabels := make(map[string]bool)
92+
for k, v := range BasicIgnoredLabels {
93+
genericIgnoredLabels[k] = v
94+
}
95+
for _, k := range extraIgnoredLabels {
96+
genericIgnoredLabels[k] = true
97+
}
98+
9199
return func(n1, n2 *schedulernodeinfo.NodeInfo) bool {
92-
return IsCloudProviderNodeInfoSimilar(n1, n2, BasicIgnoredLabels)
100+
return IsCloudProviderNodeInfoSimilar(n1, n2, genericIgnoredLabels)
93101
}
94102
}
95103

‎cluster-autoscaler/processors/nodegroupset/compare_nodegroups_test.go‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -41,14 +41,14 @@ func checkNodesSimilarWithPods(t *testing.T, n1, n2 *apiv1.Node, pods1, pods2 []
4141
}
4242

4343
func TestIdenticalNodesSimilar(t *testing.T) {
44-
comparator := CreateGenericNodeInfoComparator()
44+
comparator := CreateGenericNodeInfoComparator([]string{})
4545
n1 := BuildTestNode("node1", 1000, 2000)
4646
n2 := BuildTestNode("node2", 1000, 2000)
4747
checkNodesSimilar(t, n1, n2, comparator, true)
4848
}
4949

5050
func TestNodesSimilarVariousRequirements(t *testing.T) {
51-
comparator := CreateGenericNodeInfoComparator()
51+
comparator := CreateGenericNodeInfoComparator([]string{})
5252
n1 := BuildTestNode("node1", 1000, 2000)
5353

5454
// Different CPU capacity
@@ -74,7 +74,7 @@ func TestNodesSimilarVariousRequirements(t *testing.T) {
7474
}
7575

7676
func TestNodesSimilarVariousRequirementsAndPods(t *testing.T) {
77-
comparator := CreateGenericNodeInfoComparator()
77+
comparator := CreateGenericNodeInfoComparator([]string{})
7878
n1 := BuildTestNode("node1", 1000, 2000)
7979
p1 := BuildTestPod("pod1", 500, 1000)
8080
p1.Spec.NodeName = "node1"
@@ -100,7 +100,7 @@ func TestNodesSimilarVariousRequirementsAndPods(t *testing.T) {
100100
}
101101

102102
func TestNodesSimilarVariousMemoryRequirements(t *testing.T) {
103-
comparator := CreateGenericNodeInfoComparator()
103+
comparator := CreateGenericNodeInfoComparator([]string{})
104104
n1 := BuildTestNode("node1", 1000, MaxMemoryDifferenceInKiloBytes)
105105

106106
// Different memory capacity within tolerance
@@ -115,7 +115,7 @@ func TestNodesSimilarVariousMemoryRequirements(t *testing.T) {
115115
}
116116

117117
func TestNodesSimilarVariousLabels(t *testing.T) {
118-
comparator := CreateGenericNodeInfoComparator()
118+
comparator := CreateGenericNodeInfoComparator([]string{"example.com/ready"})
119119
n1 := BuildTestNode("node1", 1000, 2000)
120120
n1.ObjectMeta.Labels["test-label"] = "test-value"
121121
n1.ObjectMeta.Labels["character"] = "winnie the pooh"
@@ -147,4 +147,9 @@ func TestNodesSimilarVariousLabels(t *testing.T) {
147147
n1.ObjectMeta.Labels["beta.kubernetes.io/fluentd-ds-ready"] = "true"
148148
delete(n2.ObjectMeta.Labels, "beta.kubernetes.io/fluentd-ds-ready")
149149
checkNodesSimilar(t, n1, n2, comparator, true)
150+
151+
// Different custom labels should not matter
152+
n1.ObjectMeta.Labels["example.com/ready"] = "true"
153+
n2.ObjectMeta.Labels["example.com/ready"] = "false"
154+
checkNodesSimilar(t, n1, n2, comparator, true)
150155
}

0 commit comments

Comments
 (0)