Skip to content

Commit

Permalink
CA: remove Clear from ClusterSnapshot
Browse files Browse the repository at this point in the history
It's now redundant - SetClusterState with empty arguments does the same
thing.
  • Loading branch information
towca committed Nov 18, 2024
1 parent e960edf commit 00e304b
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 16 deletions.
8 changes: 4 additions & 4 deletions cluster-autoscaler/simulator/clustersnapshot/basic.go
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ func (data *internalBasicSnapshotData) removePod(namespace, podName, nodeName st
// NewBasicClusterSnapshot creates instances of BasicClusterSnapshot.
func NewBasicClusterSnapshot() *BasicClusterSnapshot {
snapshot := &BasicClusterSnapshot{}
snapshot.Clear()
snapshot.clear()
return snapshot
}

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

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

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

// Clear reset cluster snapshot to empty, unforked state
func (snapshot *BasicClusterSnapshot) Clear() {
// clear reset cluster snapshot to empty, unforked state
func (snapshot *BasicClusterSnapshot) clear() {
baseData := newInternalBasicSnapshotData()
snapshot.data = []*internalBasicSnapshotData{baseData}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,6 @@ type ClusterSnapshot interface {
Revert()
// Commit commits changes done after forking.
Commit() error
// Clear reset cluster snapshot to empty, unforked state.
Clear()
}

// ErrNodeNotFound means that a node wasn't found in the snapshot.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ func BenchmarkAddNodeInfo(b *testing.B) {
b.Run(fmt.Sprintf("%s: AddNodeInfo() %d", snapshotName, tc), func(b *testing.B) {
for i := 0; i < b.N; i++ {
b.StopTimer()
clusterSnapshot.Clear()
assert.NoError(b, clusterSnapshot.SetClusterState(nil, nil))
b.StartTimer()
for _, node := range nodes {
err := clusterSnapshot.AddNodeInfo(framework.NewTestNodeInfo(node))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ func TestForking(t *testing.T) {
}
}

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

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

snapshot.Clear()
assert.NoError(t, snapshot.SetClusterState(nil, nil))

compareStates(t, snapshotState{}, getSnapshotState(t, snapshot))
})
t.Run(fmt.Sprintf("%s: clear base %d nodes %d pods and set a new state", name, nodeCount, podCount),
func(t *testing.T) {
snapshot := startSnapshot(t, snapshotFactory, state)
compareStates(t, state, getSnapshotState(t, snapshot))

newNodes, newPods := createTestNodes(13), createTestPods(37)
assignPodsToNodes(newPods, newNodes)
assert.NoError(t, snapshot.SetClusterState(newNodes, newPods))

compareStates(t, snapshotState{nodes: newNodes, pods: newPods}, getSnapshotState(t, snapshot))
})
t.Run(fmt.Sprintf("%s: clear fork %d nodes %d pods %d extra nodes %d extra pods", name, nodeCount, podCount, extraNodeCount, extraPodCount),
func(t *testing.T) {
snapshot := startSnapshot(t, snapshotFactory, state)
Expand All @@ -332,11 +343,11 @@ func TestClear(t *testing.T) {

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

snapshot.Clear()
assert.NoError(t, snapshot.SetClusterState(nil, nil))

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

// Clear() should break out of forked state.
// SetClusterState() should break out of forked state.
snapshot.Fork()
})
}
Expand Down Expand Up @@ -718,7 +729,7 @@ func TestPVCClearAndFork(t *testing.T) {
volumeExists := snapshot.StorageInfos().IsPVCUsedByPods(schedulerframework.GetNamespacedName("default", "claim1"))
assert.Equal(t, true, volumeExists)

snapshot.Clear()
assert.NoError(t, snapshot.SetClusterState(nil, nil))
volumeExists = snapshot.StorageInfos().IsPVCUsedByPods(schedulerframework.GetNamespacedName("default", "claim1"))
assert.Equal(t, false, volumeExists)

Expand Down
6 changes: 3 additions & 3 deletions cluster-autoscaler/simulator/clustersnapshot/delta.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,7 @@ func (snapshot *DeltaClusterSnapshot) StorageInfos() schedulerframework.StorageI
// NewDeltaClusterSnapshot creates instances of DeltaClusterSnapshot.
func NewDeltaClusterSnapshot() *DeltaClusterSnapshot {
snapshot := &DeltaClusterSnapshot{}
snapshot.Clear()
snapshot.clear()
return snapshot
}

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

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

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

// Clear reset cluster snapshot to empty, unforked state
// Time: O(1)
func (snapshot *DeltaClusterSnapshot) Clear() {
func (snapshot *DeltaClusterSnapshot) clear() {
snapshot.data = newInternalDeltaSnapshotData()
}
2 changes: 1 addition & 1 deletion cluster-autoscaler/simulator/clustersnapshot/test_utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ func InitializeClusterSnapshotOrDie(
pods []*apiv1.Pod) {
var err error

snapshot.Clear()
assert.NoError(t, snapshot.SetClusterState(nil, nil))

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

0 comments on commit 00e304b

Please sign in to comment.