review feedback: ->computeScaleDownData and revised godoc

gardener · Feb 4, 2025 · 36b59f1 · 36b59f1
1 parent f31884d
commit 36b59f1
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 12 deletions.
diff --git a/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go b/cluster-autoscaler/cloudprovider/mcm/mcm_manager.go
@@ -507,7 +507,7 @@ func (m *McmManager) scaleDownMachineDeployment(ctx context.Context, mdName stri
 		return true, err
 	}
 
-	data := computeScaledownData(md, toBeDeletedMachineNames)
+	data := computeScaleDownData(md, toBeDeletedMachineNames)
 	if data.RevisedScaledownAmount == 0 {
 		klog.V(3).Infof("Skipping scaledown since MachineDeployment %q has already marked %v for deletion by MCM, skipping the scale-down", md.Name, toBeDeletedMachineNames)
 		return false, nil
@@ -1076,8 +1076,9 @@ func filterExtendedResources(allResources v1.ResourceList) (extendedResources v1
 	return
 }
 
-// computeScaledownData computes fresh scaleDownData for the given MachineDeployment given the machineNamesForDeletion
-func computeScaledownData(md *v1alpha1.MachineDeployment, machineNamesForDeletion []string) (data scaleDownData) {
+// computeScaleDownData computes fresh scaleDownData for the given input MachineDeployment and the machineNamesForDeletion.
+// The output scaleDownData encapsulates the scale-down amount and an updated, non-nil MachineDeployment.
+func computeScaleDownData(md *v1alpha1.MachineDeployment, machineNamesForDeletion []string) (data scaleDownData) {
 	forDeletionSet := sets.New(machineNamesForDeletion...)
 	alreadyMarkedSet := sets.New(getMachineNamesTriggeredForDeletion(md)...)
 

diff --git a/cluster-autoscaler/cloudprovider/mcm/mcm_manager_test.go b/cluster-autoscaler/cloudprovider/mcm/mcm_manager_test.go
@@ -231,7 +231,7 @@ func TestComputeScaledownData(t *testing.T) {
 		md.Annotations = map[string]string{}
 
 		machineNamesForDeletion := []string{"n1"}
-		data := computeScaledownData(md, machineNamesForDeletion)
+		data := computeScaleDownData(md, machineNamesForDeletion)
 		assert.Equal(t, createMachinesTriggeredForDeletionAnnotValue(machineNamesForDeletion), data.RevisedMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM])
 		assert.Equal(t, len(machineNamesForDeletion), data.RevisedScaledownAmount)
 		assert.Equal(t, int32(2-len(machineNamesForDeletion)), data.RevisedMachineDeployment.Spec.Replicas)
@@ -243,16 +243,16 @@ func TestComputeScaledownData(t *testing.T) {
 		md.Annotations = map[string]string{}
 
 		machineNamesForDeletion := []string{"n1"}
-		data := computeScaledownData(md, machineNamesForDeletion)
+		data := computeScaleDownData(md, machineNamesForDeletion)
 		assert.Equal(t, createMachinesTriggeredForDeletionAnnotValue(machineNamesForDeletion), data.RevisedMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM])
 		assert.Equal(t, len(machineNamesForDeletion), data.RevisedScaledownAmount)
 
 		expectedReplicas := int32(initialReplicas - len(machineNamesForDeletion))
 		assert.Equal(t, expectedReplicas, data.RevisedMachineDeployment.Spec.Replicas)
 
 		md = data.RevisedMachineDeployment
-		// repeating computeScaledownData for same machineNamesForDeletion should have 0 RevisedScaledownAmount, empty RevisedToBeDeletedNames, and nil RevisedMachineDeployment
-		data = computeScaledownData(md, machineNamesForDeletion)
+		// repeating computeScaleDownData for same machineNamesForDeletion should have 0 RevisedScaledownAmount, empty RevisedToBeDeletedNames, and nil RevisedMachineDeployment
+		data = computeScaleDownData(md, machineNamesForDeletion)
 		assert.Equal(t, 0, data.RevisedScaledownAmount)
 		assert.Empty(t, data.RevisedToBeDeletedNames)
 		assert.Nil(t, data.RevisedMachineDeployment)
@@ -265,15 +265,15 @@ func TestComputeScaledownData(t *testing.T) {
 		md.Annotations = map[string]string{}
 
 		machineNamesForDeletion := []string{"n1", "n2"}
-		data := computeScaledownData(md, machineNamesForDeletion)
+		data := computeScaleDownData(md, machineNamesForDeletion)
 		assert.Equal(t, createMachinesTriggeredForDeletionAnnotValue(machineNamesForDeletion), data.RevisedMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM])
 		assert.Equal(t, len(machineNamesForDeletion), data.RevisedScaledownAmount)
 		expectedReplicas := int32(initialReplicas - len(machineNamesForDeletion))
 		assert.Equal(t, expectedReplicas, data.RevisedMachineDeployment.Spec.Replicas)
 
 		md = data.RevisedMachineDeployment
-		// repeating computeScaledownData for same machineNamesForDeletion should have 0 RevisedScaledownAmount, empty RevisedToBeDeletedNames, and nil RevisedMachineDeployment
-		data = computeScaledownData(md, machineNamesForDeletion)
+		// repeating computeScaleDownData for same machineNamesForDeletion should have 0 RevisedScaledownAmount, empty RevisedToBeDeletedNames, and nil RevisedMachineDeployment
+		data = computeScaleDownData(md, machineNamesForDeletion)
 		assert.Equal(t, 0, data.RevisedScaledownAmount)
 		assert.Empty(t, data.RevisedToBeDeletedNames)
 		assert.Nil(t, data.RevisedMachineDeployment)
@@ -286,15 +286,15 @@ func TestComputeScaledownData(t *testing.T) {
 		md.Annotations = map[string]string{}
 
 		machineNamesForDeletion := sets.New("n1", "n2")
-		data := computeScaledownData(md, machineNamesForDeletion.UnsortedList())
+		data := computeScaleDownData(md, machineNamesForDeletion.UnsortedList())
 		assert.Equal(t, createMachinesTriggeredForDeletionAnnotValue(machineNamesForDeletion.UnsortedList()), data.RevisedMachineDeployment.Annotations[machineutils.TriggerDeletionByMCM])
 		assert.Equal(t, len(machineNamesForDeletion), data.RevisedScaledownAmount)
 		expectedReplicas := int32(initialReplicas - len(machineNamesForDeletion))
 		assert.Equal(t, expectedReplicas, data.RevisedMachineDeployment.Spec.Replicas)
 
 		newMachineNamesForDeletion := sets.New("n2", "n3", "n4")
 		md = data.RevisedMachineDeployment
-		data = computeScaledownData(md, newMachineNamesForDeletion.UnsortedList())
+		data = computeScaleDownData(md, newMachineNamesForDeletion.UnsortedList())
 		assert.NotNil(t, data.RevisedMachineDeployment)
 		uniqueMachinesNamesForDeletion := newMachineNamesForDeletion.Difference(machineNamesForDeletion)
 		assert.Equal(t, uniqueMachinesNamesForDeletion.Len(), data.RevisedScaledownAmount)