scale-down-delay-after-delete parameter doesn't work properly

[marwanad]

I believe that started around the time the switch to the new scale down processor happened.

The parameter goes into effect and puts scaledown in cooldown mode based on the lastScaleDownDeleteTime.

autoscaler/cluster-autoscaler/core/static_autoscaler.go

Line 499 in 774390e

a.lastScaleDownDeleteTime.Add(a.ScaleDownDelayAfterDelete).After(currentTime)

However, this is only set when the scale down status result is ScaleDownNodeDeleted

autoscaler/cluster-autoscaler/core/static_autoscaler.go

Lines 532 to 535 in 774390e

	if scaleDownStatus.Result == status.ScaleDownNodeDeleted {
	a.lastScaleDownDeleteTime = currentTime
	a.clusterStateRegistry.Recalculate()
	}

And it seems with the switch to the async deletions, this is no longer set:

autoscaler/cluster-autoscaler/core/scale_down.go

Lines 943 to 964 in 3071905

	go func() {
	// Finishing the delete process once this goroutine is over.
	var result status.NodeDeleteResult
	defer func() { sd.nodeDeletionTracker.AddNodeDeleteResult(toRemove.Node.Name, result) }()
	defer sd.nodeDeletionTracker.SetNonEmptyNodeDeleteInProgress(false)
	nodeGroup, found := candidateNodeGroups[toRemove.Node.Name]
	if !found {
	result = status.NodeDeleteResult{ResultType: status.NodeDeleteErrorFailedToDelete, Err: errors.NewAutoscalerError(
	errors.InternalError, "failed to find node group for %s", toRemove.Node.Name)}
	return
	}
	result = sd.deleteNode(toRemove.Node, toRemove.PodsToReschedule, nodeGroup)
	if result.ResultType != status.NodeDeleteOk {
	klog.Errorf("Failed to delete %s: %v", toRemove.Node.Name, result.Err)
	return
	}
	if readinessMap[toRemove.Node.Name] {
	metrics.RegisterScaleDown(1, gpu.GetGpuTypeForMetrics(gpuLabel, availableGPUTypes, toRemove.Node, nodeGroup), metrics.Underutilized)
	} else {
	metrics.RegisterScaleDown(1, gpu.GetGpuTypeForMetrics(gpuLabel, availableGPUTypes, toRemove.Node, nodeGroup), metrics.Unready)
	}
	}()

/kind bug

[marwanad]

@MaciekPytel @towca

[marwanad]

Should we be returning ScaleDownNodeDeleted instead of ScaleDownNodeDeleteStarted - it seems that the later isn't used around so it seems like the most straightforward fix

[towca]

Thanks for pointing this out Marwan! Although I'd stick to using ScaleDownNodeDeleteStarted since it better conveys what actually happens. I'll send out a fix shortly.

[towca]

The bug should be fixed by #3570.

[ryaneorth]

thanks @towca @marwanad for finding/fixing this issue! any idea when/if this will make it into a patch release for versions 1.16.x/1.17.x?

[ryaneorth]

@MaciekPytel - maybe you know the answer to the above question?

[marwanad]

@ryaneorth we can prepare the cherry-pick PRs. I'm guessing we'll have one set of patch releases before K8s 1.20 and another around 1.20.

[ryaneorth]

Sounds great, thanks @marwanad . I'm happy to perform the cherry-picks if you'd like - let me know!

[marwanad]

@ryaneorth that would be great, thanks!

[ryaneorth]

Done!

#3597
#3598
#3599
#3600

[ryaneorth]

All of the above cherry-picks are complete. @MaciekPytel - do you have any information as to when the next patches will be released?

[marwanad]

@ryaneorth keep an 👀 on #3611