Merge pull request #7868 from norbertcyran/clean-taints-on-copy

Clean taints on node's copy in failed scale down cleanup

Author: k8s-ci-robot

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

@@ -1274,10 +1274,9 @@ func runStartDeletionTest(t *testing.T, tc startDeletionTestCase, force bool) {
 	ndb := NewNodeDeletionBatcher(&ctx, scaleStateNotifier, ndt, 0*time.Second)
 	legacyFlagDrainConfig := SingleRuleDrainConfig(ctx.MaxGracefulTerminationSec)
 	evictor := Evictor{EvictionRetryTime: 0, PodEvictionHeadroom: DefaultPodEvictionHeadroom, shutdownGracePeriodByPodPriority: legacyFlagDrainConfig, fullDsEviction: force}
-	scheduler := NewGroupDeletionScheduler(&ctx, ndt, ndb, evictor)
 	actuator := Actuator{
 		ctx: &ctx, nodeDeletionTracker: ndt,
-		nodeDeletionScheduler: scheduler,
+		nodeDeletionScheduler: NewGroupDeletionScheduler(&ctx, ndt, ndb, evictor),
 		budgetProcessor:       budgets.NewScaleDownBudgetProcessor(&ctx),
 		configGetter:          nodegroupconfig.NewDefaultNodeGroupConfigProcessor(ctx.NodeGroupDefaults),
 	}
@@ -1303,15 +1302,12 @@ func runStartDeletionTest(t *testing.T, tc startDeletionTestCase, force bool) {
 	// Verify ScaleDownNodes looks as expected.
 	ignoreSdNodeOrder := cmpopts.SortSlices(func(a, b *status.ScaleDownNode) bool { return a.Node.Name < b.Node.Name })
 	cmpNg := cmp.Comparer(func(a, b *testprovider.TestNodeGroup) bool { return a.Id() == b.Id() })
-	// Deletion taint may be lifted by goroutine, ignore taints to avoid race condition
+	// Nodes will have deletion taints, skipping them here since we check them later
 	ignoreTaints := cmpopts.IgnoreFields(apiv1.NodeSpec{}, "Taints")
 	statusCmpOpts := cmp.Options{ignoreSdNodeOrder, cmpNg, cmpopts.EquateEmpty(), ignoreTaints}
-	// lock deletion scheduler so race detector does not complain
-	scheduler.Lock()
 	if diff := cmp.Diff(wantScaleDownNodes, gotScaleDownNodes, statusCmpOpts); diff != "" {
 		t.Errorf("StartDeletion scaled down nodes diff (-want +got):\n%s", diff)
 	}
-	scheduler.Unlock()
 
 	// Verify that all expected nodes were deleted using the cloud provider hook.
 	var gotDeletedNodes []string

cluster-autoscaler/core/scaledown/actuation/delete_in_batch.go

@@ -206,7 +206,7 @@ func CleanUpAndRecordFailedScaleDownEvent(ctx *context.AutoscalingContext, node
 		klog.Errorf("Scale-down: "+logMsgFormat+", %v, status error: %v", node.Name, errMsg, status.Err)
 	}
 	ctx.Recorder.Eventf(node, apiv1.EventTypeWarning, "ScaleDownFailed", eventMsgFormat+": %v", status.Err)
-	taints.CleanToBeDeleted(node, ctx.ClientSet, ctx.CordonNodeBeforeTerminate)
+	taints.CleanToBeDeleted(node.DeepCopy(), ctx.ClientSet, ctx.CordonNodeBeforeTerminate)
 	nodeDeletionTracker.EndDeletion(nodeGroupId, node.Name, status)
 }