Skip to content

Commit adf59d4

Browse files
k8s-ci-robotk8s-ci-robot
authored
Merge pull request #8156 from BigDarkClown/master
Rewrite TestCloudProvider to use builder pattern
2 parents 0d8f587 + 2c7d8dc commit adf59d4

27 files changed

+242
-227
lines changed

‎cluster-autoscaler/cloudprovider/test/test_cloud_provider.go

Lines changed: 70 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -61,45 +61,84 @@ type TestCloudProvider struct {
6161
resourceLimiter *cloudprovider.ResourceLimiter
6262
}
6363

64-
// NewTestCloudProvider builds new TestCloudProvider
65-
func NewTestCloudProvider(onScaleUp OnScaleUpFunc, onScaleDown OnScaleDownFunc) *TestCloudProvider {
66-
return &TestCloudProvider{
67-
nodes: make(map[string]string),
68-
groups: make(map[string]cloudprovider.NodeGroup),
69-
onScaleUp: onScaleUp,
70-
onScaleDown: onScaleDown,
71-
resourceLimiter: cloudprovider.NewResourceLimiter(make(map[string]int64), make(map[string]int64)),
72-
}
64+
// TestCloudProviderBuilder is used to create CloudProvider
65+
type TestCloudProviderBuilder struct {
66+
builders []func(p *TestCloudProvider)
7367
}
7468

75-
// NewTestAutoprovisioningCloudProvider builds new TestCloudProvider with autoprovisioning support
76-
func NewTestAutoprovisioningCloudProvider(onScaleUp OnScaleUpFunc, onScaleDown OnScaleDownFunc,
77-
onNodeGroupCreate OnNodeGroupCreateFunc, onNodeGroupDelete OnNodeGroupDeleteFunc,
78-
machineTypes []string, machineTemplates map[string]*framework.NodeInfo) *TestCloudProvider {
79-
return &TestCloudProvider{
80-
nodes: make(map[string]string),
81-
groups: make(map[string]cloudprovider.NodeGroup),
82-
onScaleUp: onScaleUp,
83-
onScaleDown: onScaleDown,
84-
onNodeGroupCreate: onNodeGroupCreate,
85-
onNodeGroupDelete: onNodeGroupDelete,
86-
machineTypes: machineTypes,
87-
machineTemplates: machineTemplates,
88-
resourceLimiter: cloudprovider.NewResourceLimiter(make(map[string]int64), make(map[string]int64)),
89-
}
69+
// NewTestCloudProviderBuilder returns a new test cloud provider builder
70+
func NewTestCloudProviderBuilder() *TestCloudProviderBuilder {
71+
return &TestCloudProviderBuilder{}
72+
}
73+
74+
// WithOnScaleUp adds scale-up handle function to provider
75+
func (b *TestCloudProviderBuilder) WithOnScaleUp(onScaleUp OnScaleUpFunc) *TestCloudProviderBuilder {
76+
b.builders = append(b.builders, func(p *TestCloudProvider) {
77+
p.onScaleUp = onScaleUp
78+
})
79+
return b
80+
}
81+
82+
// WithOnScaleDown adds scale-down handle function to provider
83+
func (b *TestCloudProviderBuilder) WithOnScaleDown(onScaleDown OnScaleDownFunc) *TestCloudProviderBuilder {
84+
b.builders = append(b.builders, func(p *TestCloudProvider) {
85+
p.onScaleDown = onScaleDown
86+
})
87+
return b
88+
}
89+
90+
// WithOnNodeGroupCreate adds node group creation handle function to provider
91+
func (b *TestCloudProviderBuilder) WithOnNodeGroupCreate(onNodeGroupCreate OnNodeGroupCreateFunc) *TestCloudProviderBuilder {
92+
b.builders = append(b.builders, func(p *TestCloudProvider) {
93+
p.onNodeGroupCreate = onNodeGroupCreate
94+
})
95+
return b
9096
}
9197

92-
// NewTestNodeDeletionDetectionCloudProvider builds new TestCloudProvider with deletion detection support
93-
func NewTestNodeDeletionDetectionCloudProvider(onScaleUp OnScaleUpFunc, onScaleDown OnScaleDownFunc,
94-
hasInstance HasInstance) *TestCloudProvider {
95-
return &TestCloudProvider{
98+
// WithOnNodeGroupDelete adds node group deletion handle function to provider
99+
func (b *TestCloudProviderBuilder) WithOnNodeGroupDelete(onNodeGroupDelete OnNodeGroupDeleteFunc) *TestCloudProviderBuilder {
100+
b.builders = append(b.builders, func(p *TestCloudProvider) {
101+
p.onNodeGroupDelete = onNodeGroupDelete
102+
})
103+
return b
104+
}
105+
106+
// WithMachineTypes adds machine types to provider
107+
func (b *TestCloudProviderBuilder) WithMachineTypes(machineTypes []string) *TestCloudProviderBuilder {
108+
b.builders = append(b.builders, func(p *TestCloudProvider) {
109+
p.machineTypes = machineTypes
110+
})
111+
return b
112+
}
113+
114+
// WithMachineTemplates adds machine templates for provider
115+
func (b *TestCloudProviderBuilder) WithMachineTemplates(machineTemplates map[string]*framework.NodeInfo) *TestCloudProviderBuilder {
116+
b.builders = append(b.builders, func(p *TestCloudProvider) {
117+
p.machineTemplates = machineTemplates
118+
})
119+
return b
120+
}
121+
122+
// WithHasInstance adds has instance handler to provider
123+
func (b *TestCloudProviderBuilder) WithHasInstance(hasInstance HasInstance) *TestCloudProviderBuilder {
124+
b.builders = append(b.builders, func(p *TestCloudProvider) {
125+
p.hasInstance = hasInstance
126+
})
127+
return b
128+
}
129+
130+
// Build returns a built test cloud provider
131+
func (b *TestCloudProviderBuilder) Build() *TestCloudProvider {
132+
p := &TestCloudProvider{
96133
nodes: make(map[string]string),
97134
groups: make(map[string]cloudprovider.NodeGroup),
98-
onScaleUp: onScaleUp,
99-
onScaleDown: onScaleDown,
100-
hasInstance: hasInstance,
101135
resourceLimiter: cloudprovider.NewResourceLimiter(make(map[string]int64), make(map[string]int64)),
102136
}
137+
138+
for _, builder := range b.builders {
139+
builder(p)
140+
}
141+
return p
103142
}
104143

105144
// Name returns name of the cloud provider.

‎cluster-autoscaler/clusterstate/clusterstate_test.go

Lines changed: 26 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ func TestOKWithScaleUp(t *testing.T) {
6262
ng2_1 := BuildTestNode("ng2-1", 1000, 1000)
6363
SetNodeReadyState(ng2_1, true, now.Add(-time.Minute))
6464

65-
provider := testprovider.NewTestCloudProvider(nil, nil)
65+
provider := testprovider.NewTestCloudProviderBuilder().Build()
6666
provider.AddNodeGroup("ng1", 1, 10, 5)
6767
provider.AddNodeGroup("ng2", 1, 10, 1)
6868

@@ -104,7 +104,7 @@ func TestOKWithScaleUp(t *testing.T) {
104104
func TestEmptyOK(t *testing.T) {
105105
now := time.Now()
106106

107-
provider := testprovider.NewTestCloudProvider(nil, nil)
107+
provider := testprovider.NewTestCloudProviderBuilder().Build()
108108
provider.AddNodeGroup("ng1", 0, 10, 0)
109109
assert.NotNil(t, provider)
110110

@@ -148,7 +148,7 @@ func TestHasNodeGroupStartedScaleUp(t *testing.T) {
148148
for tn, tc := range tests {
149149
t.Run(tn, func(t *testing.T) {
150150
now := time.Now()
151-
provider := testprovider.NewTestCloudProvider(nil, nil)
151+
provider := testprovider.NewTestCloudProviderBuilder().Build()
152152
provider.AddNodeGroup("ng1", 0, 5, tc.initialSize)
153153
fakeClient := &fake.Clientset{}
154154
fakeLogRecorder, _ := utils.NewStatusMapRecorder(fakeClient, "kube-system", kube_record.NewFakeRecorder(5), false, "my-cool-configmap")
@@ -225,7 +225,7 @@ func TestRecalculateStateAfterNodeGroupSizeChanged(t *testing.T) {
225225
}
226226
for _, tc := range testCases {
227227
t.Run(tc.name, func(t *testing.T) {
228-
provider := testprovider.NewTestCloudProvider(nil, nil)
228+
provider := testprovider.NewTestCloudProviderBuilder().Build()
229229
provider.AddNodeGroup(ngName, 0, 1000, tc.newTarget)
230230

231231
fakeLogRecorder, _ := utils.NewStatusMapRecorder(&fake.Clientset{}, "kube-system", kube_record.NewFakeRecorder(5), false, "my-cool-configmap")
@@ -255,7 +255,7 @@ func TestOKOneUnreadyNode(t *testing.T) {
255255
ng2_1 := BuildTestNode("ng2-1", 1000, 1000)
256256
SetNodeReadyState(ng2_1, false, now.Add(-time.Minute))
257257

258-
provider := testprovider.NewTestCloudProvider(nil, nil)
258+
provider := testprovider.NewTestCloudProviderBuilder().Build()
259259
provider.AddNodeGroup("ng1", 1, 10, 1)
260260
provider.AddNodeGroup("ng2", 1, 10, 1)
261261
provider.AddNode("ng1", ng1_1)
@@ -294,7 +294,7 @@ func TestNodeWithoutNodeGroupDontCrash(t *testing.T) {
294294

295295
noNgNode := BuildTestNode("no_ng", 1000, 1000)
296296
SetNodeReadyState(noNgNode, true, now.Add(-time.Minute))
297-
provider := testprovider.NewTestCloudProvider(nil, nil)
297+
provider := testprovider.NewTestCloudProviderBuilder().Build()
298298
provider.AddNode("no_ng", noNgNode)
299299

300300
fakeClient := &fake.Clientset{}
@@ -317,7 +317,7 @@ func TestOKOneUnreadyNodeWithScaleDownCandidate(t *testing.T) {
317317
ng2_1 := BuildTestNode("ng2-1", 1000, 1000)
318318
SetNodeReadyState(ng2_1, false, now.Add(-time.Minute))
319319

320-
provider := testprovider.NewTestCloudProvider(nil, nil)
320+
provider := testprovider.NewTestCloudProviderBuilder().Build()
321321
provider.AddNodeGroup("ng1", 1, 10, 1)
322322
provider.AddNodeGroup("ng2", 1, 10, 1)
323323
provider.AddNode("ng1", ng1_1)
@@ -370,7 +370,7 @@ func TestMissingNodes(t *testing.T) {
370370
ng2_1 := BuildTestNode("ng2-1", 1000, 1000)
371371
SetNodeReadyState(ng2_1, true, now.Add(-time.Minute))
372372

373-
provider := testprovider.NewTestCloudProvider(nil, nil)
373+
provider := testprovider.NewTestCloudProviderBuilder().Build()
374374
provider.AddNodeGroup("ng1", 1, 10, 5)
375375
provider.AddNodeGroup("ng2", 1, 10, 1)
376376

@@ -411,7 +411,7 @@ func TestTooManyUnready(t *testing.T) {
411411
ng2_1 := BuildTestNode("ng2-1", 1000, 1000)
412412
SetNodeReadyState(ng2_1, false, now.Add(-time.Minute))
413413

414-
provider := testprovider.NewTestCloudProvider(nil, nil)
414+
provider := testprovider.NewTestCloudProviderBuilder().Build()
415415
provider.AddNodeGroup("ng1", 1, 10, 1)
416416
provider.AddNodeGroup("ng2", 1, 10, 1)
417417
provider.AddNode("ng1", ng1_1)
@@ -440,7 +440,7 @@ func TestUnreadyLongAfterCreation(t *testing.T) {
440440
SetNodeReadyState(ng2_1, false, now.Add(-time.Minute))
441441
ng2_1.CreationTimestamp = metav1.Time{Time: now.Add(-30 * time.Minute)}
442442

443-
provider := testprovider.NewTestCloudProvider(nil, nil)
443+
provider := testprovider.NewTestCloudProviderBuilder().Build()
444444
provider.AddNodeGroup("ng1", 1, 10, 1)
445445
provider.AddNodeGroup("ng2", 1, 10, 1)
446446
provider.AddNode("ng1", ng1_1)
@@ -472,7 +472,7 @@ func TestNotStarted(t *testing.T) {
472472
SetNodeNotReadyTaint(ng2_1)
473473
ng2_1.CreationTimestamp = metav1.Time{Time: now.Add(-10 * time.Minute)}
474474

475-
provider := testprovider.NewTestCloudProvider(nil, nil)
475+
provider := testprovider.NewTestCloudProviderBuilder().Build()
476476
provider.AddNodeGroup("ng1", 1, 10, 1)
477477
provider.AddNodeGroup("ng2", 1, 10, 1)
478478
provider.AddNode("ng1", ng1_1)
@@ -511,7 +511,7 @@ func TestExpiredScaleUp(t *testing.T) {
511511
ng1_1 := BuildTestNode("ng1-1", 1000, 1000)
512512
SetNodeReadyState(ng1_1, true, now.Add(-time.Minute))
513513

514-
provider := testprovider.NewTestCloudProvider(nil, nil)
514+
provider := testprovider.NewTestCloudProviderBuilder().Build()
515515
provider.AddNodeGroup("ng1", 1, 10, 5)
516516
provider.AddNode("ng1", ng1_1)
517517
assert.NotNil(t, provider)
@@ -536,7 +536,7 @@ func TestExpiredScaleUp(t *testing.T) {
536536

537537
func TestRegisterScaleDown(t *testing.T) {
538538
ng1_1 := BuildTestNode("ng1-1", 1000, 1000)
539-
provider := testprovider.NewTestCloudProvider(nil, nil)
539+
provider := testprovider.NewTestCloudProviderBuilder().Build()
540540
provider.AddNodeGroup("ng1", 1, 10, 1)
541541
provider.AddNode("ng1", ng1_1)
542542
assert.NotNil(t, provider)
@@ -556,7 +556,7 @@ func TestRegisterScaleDown(t *testing.T) {
556556
}
557557

558558
func TestUpcomingNodes(t *testing.T) {
559-
provider := testprovider.NewTestCloudProvider(nil, nil)
559+
provider := testprovider.NewTestCloudProviderBuilder().Build()
560560
now := time.Now()
561561

562562
// 6 nodes are expected to come.
@@ -629,8 +629,7 @@ func TestUpcomingNodes(t *testing.T) {
629629
func TestTaintBasedNodeDeletion(t *testing.T) {
630630
// Create a new Cloud Provider that does not implement the HasInstance check
631631
// it will return the ErrNotImplemented error instead.
632-
provider := testprovider.NewTestNodeDeletionDetectionCloudProvider(nil, nil,
633-
func(string) (bool, error) { return false, cloudprovider.ErrNotImplemented })
632+
provider := testprovider.NewTestCloudProviderBuilder().WithHasInstance(func(string) (bool, error) { return false, cloudprovider.ErrNotImplemented }).Build()
634633
now := time.Now()
635634

636635
// One node is already there, for a second nde deletion / draining was already started.
@@ -667,7 +666,7 @@ func TestTaintBasedNodeDeletion(t *testing.T) {
667666

668667
func TestIncorrectSize(t *testing.T) {
669668
ng1_1 := BuildTestNode("ng1-1", 1000, 1000)
670-
provider := testprovider.NewTestCloudProvider(nil, nil)
669+
provider := testprovider.NewTestCloudProviderBuilder().Build()
671670
provider.AddNodeGroup("ng1", 1, 10, 5)
672671
provider.AddNode("ng1", ng1_1)
673672
assert.NotNil(t, provider)
@@ -702,7 +701,7 @@ func TestUnregisteredNodes(t *testing.T) {
702701
ng1_1.Spec.ProviderID = "ng1-1"
703702
ng1_2 := BuildTestNode("ng1-2", 1000, 1000)
704703
ng1_2.Spec.ProviderID = "ng1-2"
705-
provider := testprovider.NewTestCloudProvider(nil, nil)
704+
provider := testprovider.NewTestCloudProviderBuilder().Build()
706705
provider.AddNodeGroup("ng1", 1, 10, 2)
707706
provider.AddNode("ng1", ng1_1)
708707
provider.AddNode("ng1", ng1_2)
@@ -750,7 +749,7 @@ func TestCloudProviderDeletedNodes(t *testing.T) {
750749
SetNodeReadyState(noNgNode, true, now.Add(-time.Minute))
751750

752751
noNgNode.Spec.ProviderID = "no-ng"
753-
provider := testprovider.NewTestCloudProvider(nil, nil)
752+
provider := testprovider.NewTestCloudProviderBuilder().Build()
754753
provider.AddNodeGroup("ng1", 1, 10, 2)
755754
provider.AddNode("ng1", ng1_1)
756755
provider.AddNode("ng1", ng1_2)
@@ -844,7 +843,7 @@ func TestScaleUpBackoff(t *testing.T) {
844843
ng1_3 := BuildTestNode("ng1-3", 1000, 1000)
845844
SetNodeReadyState(ng1_3, true, now.Add(-time.Minute))
846845

847-
provider := testprovider.NewTestCloudProvider(nil, nil)
846+
provider := testprovider.NewTestCloudProviderBuilder().Build()
848847
provider.AddNodeGroup("ng1", 1, 10, 4)
849848
ng1 := provider.GetNodeGroup("ng1")
850849
provider.AddNode("ng1", ng1_1)
@@ -967,7 +966,7 @@ func TestGetClusterSize(t *testing.T) {
967966
notAutoscaledNode := BuildTestNode("notAutoscaledNode", 1000, 1000)
968967
SetNodeReadyState(notAutoscaledNode, true, now.Add(-time.Minute))
969968

970-
provider := testprovider.NewTestCloudProvider(nil, nil)
969+
provider := testprovider.NewTestCloudProviderBuilder().Build()
971970
provider.AddNodeGroup("ng1", 1, 10, 5)
972971
provider.AddNodeGroup("ng2", 1, 10, 1)
973972

@@ -1018,7 +1017,7 @@ func TestUpdateScaleUp(t *testing.T) {
10181017
now := time.Now()
10191018
later := now.Add(time.Minute)
10201019

1021-
provider := testprovider.NewTestCloudProvider(nil, nil)
1020+
provider := testprovider.NewTestCloudProviderBuilder().Build()
10221021
provider.AddNodeGroup("ng1", 1, 10, 5)
10231022
provider.AddNodeGroup("ng2", 1, 10, 5)
10241023
fakeClient := &fake.Clientset{}
@@ -1065,7 +1064,7 @@ func TestUpdateScaleUp(t *testing.T) {
10651064
func TestScaleUpFailures(t *testing.T) {
10661065
now := time.Now()
10671066

1068-
provider := testprovider.NewTestCloudProvider(nil, nil)
1067+
provider := testprovider.NewTestCloudProviderBuilder().Build()
10691068
provider.AddNodeGroup("ng1", 0, 10, 0)
10701069
provider.AddNodeGroup("ng2", 0, 10, 0)
10711070
assert.NotNil(t, provider)
@@ -1213,7 +1212,7 @@ func TestUpdateAcceptableRanges(t *testing.T) {
12131212

12141213
for _, tc := range testCases {
12151214
t.Run(tc.name, func(t *testing.T) {
1216-
provider := testprovider.NewTestCloudProvider(nil, nil)
1215+
provider := testprovider.NewTestCloudProviderBuilder().Build()
12171216
for nodeGroupName, targetSize := range tc.targetSizes {
12181217
provider.AddNodeGroup(nodeGroupName, 0, 1000, targetSize)
12191218
}
@@ -1397,7 +1396,7 @@ func TestUpdateIncorrectNodeGroupSizes(t *testing.T) {
13971396

13981397
for _, tc := range testCases {
13991398
t.Run(tc.name, func(t *testing.T) {
1400-
provider := testprovider.NewTestCloudProvider(nil, nil)
1399+
provider := testprovider.NewTestCloudProviderBuilder().Build()
14011400
for nodeGroupName, acceptableRange := range tc.acceptableRanges {
14021401
provider.AddNodeGroup(nodeGroupName, 0, 1000, acceptableRange.CurrentTarget)
14031402
}
@@ -1458,7 +1457,7 @@ func TestTruncateIfExceedMaxSize(t *testing.T) {
14581457
}
14591458

14601459
func TestIsNodeGroupRegistered(t *testing.T) {
1461-
provider := testprovider.NewTestCloudProvider(nil, nil)
1460+
provider := testprovider.NewTestCloudProviderBuilder().Build()
14621461
registeredNodeGroupName := "registered-node-group"
14631462
provider.AddNodeGroup(registeredNodeGroupName, 1, 10, 1)
14641463
fakeClient := &fake.Clientset{}
@@ -1537,7 +1536,7 @@ func TestUpcomingNodesFromUpcomingNodeGroups(t *testing.T) {
15371536
for _, tc := range testCases {
15381537

15391538
now := time.Now()
1540-
provider := testprovider.NewTestCloudProvider(nil, nil)
1539+
provider := testprovider.NewTestCloudProviderBuilder().Build()
15411540
for groupName, groupSize := range tc.nodeGroups {
15421541
provider.AddUpcomingNodeGroup(groupName, 1, 10, groupSize)
15431542
}

‎cluster-autoscaler/clusterstate/utils/node_instances_cache_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ func TestCloudProviderNodeInstancesCache(t *testing.T) {
4444
nodeNg4_1 := BuildTestNode("ng4-1", 1000, 1000)
4545
instanceNg4_1 := buildRunningInstance(nodeNg4_1.Name)
4646

47-
provider := testprovider.NewTestCloudProvider(nil, nil)
47+
provider := testprovider.NewTestCloudProviderBuilder().Build()
4848
provider.AddNodeGroup("ng1", 1, 10, 1)
4949
provider.AddNodeGroup("ng2", 1, 10, 1)
5050
provider.AddNodeGroup("ng3", 1, 10, 1)

‎cluster-autoscaler/core/scaledown/actuation/actuator_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1185,13 +1185,13 @@ func runStartDeletionTest(t *testing.T, tc startDeletionTestCase, force bool) {
11851185

11861186
// Hook node deletion at the level of cloud provider, to gather which nodes were deleted, and to fail the deletion for
11871187
// certain nodes to simulate errors.
1188-
provider := testprovider.NewTestCloudProvider(nil, func(nodeGroup string, node string) error {
1188+
provider := testprovider.NewTestCloudProviderBuilder().WithOnScaleDown(func(nodeGroup string, node string) error {
11891189
if tc.failedNodeDeletion[node] {
11901190
return fmt.Errorf("SIMULATED ERROR: won't remove node")
11911191
}
11921192
deletedNodes <- node
11931193
return nil
1194-
})
1194+
}).Build()
11951195
for _, bucket := range emptyNodeGroupViews {
11961196
bucket.Group.(*testprovider.TestNodeGroup).SetCloudProvider(provider)
11971197
provider.InsertNodeGroup(bucket.Group)
@@ -1497,13 +1497,13 @@ func TestStartDeletionInBatchBasic(t *testing.T) {
14971497
}
14981498
deletedResult := make(chan string)
14991499
fakeClient := &fake.Clientset{}
1500-
provider := testprovider.NewTestCloudProvider(nil, func(nodeGroupId string, node string) error {
1500+
provider := testprovider.NewTestCloudProviderBuilder().WithOnScaleDown(func(nodeGroupId string, node string) error {
15011501
if gotFailedRequest(nodeGroupId) {
15021502
return fmt.Errorf("SIMULATED ERROR: won't remove node")
15031503
}
15041504
deletedResult <- nodeGroupId
15051505
return nil
1506-
})
1506+
}).Build()
15071507
// 2d array represent the waves of pushing nodes to delete.
15081508
deleteNodes := [][]*apiv1.Node{}
15091509

0 commit comments

Comments
 (0)