Skip to content

Commit 473a1a8

Browse files
committed
CA: remove Clear from ClusterSnapshot
It's now redundant - SetClusterState with empty arguments does the same thing.
1 parent f67db62 commit 473a1a8

File tree

6 files changed

+25
-16
lines changed

6 files changed

+25
-16
lines changed

cluster-autoscaler/simulator/clustersnapshot/basic.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ func (data *internalBasicSnapshotData) removePod(namespace, podName, nodeName st
196196
// NewBasicClusterSnapshot creates instances of BasicClusterSnapshot.
197197
func NewBasicClusterSnapshot() *BasicClusterSnapshot {
198198
snapshot := &BasicClusterSnapshot{}
199-
snapshot.Clear()
199+
snapshot.clear()
200200
return snapshot
201201
}
202202

@@ -234,7 +234,7 @@ func (snapshot *BasicClusterSnapshot) AddNodeInfo(nodeInfo *framework.NodeInfo)
234234

235235
// SetClusterState sets the cluster state.
236236
func (snapshot *BasicClusterSnapshot) SetClusterState(nodes []*apiv1.Node, scheduledPods []*apiv1.Pod) error {
237-
snapshot.Clear()
237+
snapshot.clear()
238238

239239
knownNodes := make(map[string]bool)
240240
for _, node := range nodes {
@@ -297,8 +297,8 @@ func (snapshot *BasicClusterSnapshot) Commit() error {
297297
return nil
298298
}
299299

300-
// Clear reset cluster snapshot to empty, unforked state
301-
func (snapshot *BasicClusterSnapshot) Clear() {
300+
// clear reset cluster snapshot to empty, unforked state
301+
func (snapshot *BasicClusterSnapshot) clear() {
302302
baseData := newInternalBasicSnapshotData()
303303
snapshot.data = []*internalBasicSnapshotData{baseData}
304304
}

cluster-autoscaler/simulator/clustersnapshot/clustersnapshot.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ type ClusterSnapshot interface {
5959
Revert()
6060
// Commit commits changes done after forking.
6161
Commit() error
62-
// Clear reset cluster snapshot to empty, unforked state.
63-
Clear()
6462
}
6563

6664
// ErrNodeNotFound means that a node wasn't found in the snapshot.

cluster-autoscaler/simulator/clustersnapshot/clustersnapshot_benchmark_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ func BenchmarkAddNodeInfo(b *testing.B) {
8080
b.Run(fmt.Sprintf("%s: AddNodeInfo() %d", snapshotName, tc), func(b *testing.B) {
8181
for i := 0; i < b.N; i++ {
8282
b.StopTimer()
83-
clusterSnapshot.Clear()
83+
assert.NoError(b, clusterSnapshot.SetClusterState(nil, nil))
8484
b.StartTimer()
8585
for _, node := range nodes {
8686
err := clusterSnapshot.AddNodeInfo(framework.NewTestNodeInfo(node))

cluster-autoscaler/simulator/clustersnapshot/clustersnapshot_test.go

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ func TestForking(t *testing.T) {
275275
}
276276
}
277277

278-
func TestClear(t *testing.T) {
278+
func TestSetClusterState(t *testing.T) {
279279
// Run with -count=1 to avoid caching.
280280
localRand := rand.New(rand.NewSource(time.Now().Unix()))
281281

@@ -309,10 +309,21 @@ func TestClear(t *testing.T) {
309309
snapshot := startSnapshot(t, snapshotFactory, state)
310310
compareStates(t, state, getSnapshotState(t, snapshot))
311311

312-
snapshot.Clear()
312+
assert.NoError(t, snapshot.SetClusterState(nil, nil))
313313

314314
compareStates(t, snapshotState{}, getSnapshotState(t, snapshot))
315315
})
316+
t.Run(fmt.Sprintf("%s: clear base %d nodes %d pods and set a new state", name, nodeCount, podCount),
317+
func(t *testing.T) {
318+
snapshot := startSnapshot(t, snapshotFactory, state)
319+
compareStates(t, state, getSnapshotState(t, snapshot))
320+
321+
newNodes, newPods := createTestNodes(13), createTestPods(37)
322+
assignPodsToNodes(newPods, newNodes)
323+
assert.NoError(t, snapshot.SetClusterState(newNodes, newPods))
324+
325+
compareStates(t, snapshotState{nodes: newNodes, pods: newPods}, getSnapshotState(t, snapshot))
326+
})
316327
t.Run(fmt.Sprintf("%s: clear fork %d nodes %d pods %d extra nodes %d extra pods", name, nodeCount, podCount, extraNodeCount, extraPodCount),
317328
func(t *testing.T) {
318329
snapshot := startSnapshot(t, snapshotFactory, state)
@@ -332,11 +343,11 @@ func TestClear(t *testing.T) {
332343

333344
compareStates(t, snapshotState{allNodes, allPods}, getSnapshotState(t, snapshot))
334345

335-
snapshot.Clear()
346+
assert.NoError(t, snapshot.SetClusterState(nil, nil))
336347

337348
compareStates(t, snapshotState{}, getSnapshotState(t, snapshot))
338349

339-
// Clear() should break out of forked state.
350+
// SetClusterState() should break out of forked state.
340351
snapshot.Fork()
341352
})
342353
}
@@ -718,7 +729,7 @@ func TestPVCClearAndFork(t *testing.T) {
718729
volumeExists := snapshot.StorageInfos().IsPVCUsedByPods(schedulerframework.GetNamespacedName("default", "claim1"))
719730
assert.Equal(t, true, volumeExists)
720731

721-
snapshot.Clear()
732+
assert.NoError(t, snapshot.SetClusterState(nil, nil))
722733
volumeExists = snapshot.StorageInfos().IsPVCUsedByPods(schedulerframework.GetNamespacedName("default", "claim1"))
723734
assert.Equal(t, false, volumeExists)
724735

cluster-autoscaler/simulator/clustersnapshot/delta.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -389,7 +389,7 @@ func (snapshot *DeltaClusterSnapshot) StorageInfos() schedulerframework.StorageI
389389
// NewDeltaClusterSnapshot creates instances of DeltaClusterSnapshot.
390390
func NewDeltaClusterSnapshot() *DeltaClusterSnapshot {
391391
snapshot := &DeltaClusterSnapshot{}
392-
snapshot.Clear()
392+
snapshot.clear()
393393
return snapshot
394394
}
395395

@@ -423,7 +423,7 @@ func (snapshot *DeltaClusterSnapshot) AddNodeInfo(nodeInfo *framework.NodeInfo)
423423

424424
// SetClusterState sets the cluster state.
425425
func (snapshot *DeltaClusterSnapshot) SetClusterState(nodes []*apiv1.Node, scheduledPods []*apiv1.Pod) error {
426-
snapshot.Clear()
426+
snapshot.clear()
427427

428428
knownNodes := make(map[string]bool)
429429
for _, node := range nodes {
@@ -489,6 +489,6 @@ func (snapshot *DeltaClusterSnapshot) Commit() error {
489489

490490
// Clear reset cluster snapshot to empty, unforked state
491491
// Time: O(1)
492-
func (snapshot *DeltaClusterSnapshot) Clear() {
492+
func (snapshot *DeltaClusterSnapshot) clear() {
493493
snapshot.data = newInternalDeltaSnapshotData()
494494
}

cluster-autoscaler/simulator/clustersnapshot/test_utils.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ func InitializeClusterSnapshotOrDie(
3434
pods []*apiv1.Pod) {
3535
var err error
3636

37-
snapshot.Clear()
37+
assert.NoError(t, snapshot.SetClusterState(nil, nil))
3838

3939
for _, node := range nodes {
4040
err = snapshot.AddNodeInfo(framework.NewTestNodeInfo(node))

0 commit comments

Comments
 (0)