No backoff when scaled-up nodes never register #150

Which component are you using?:

/area cluster-autoscaler
/kind regression

What version of the component are you using?:

Component version: v0.0.0-k8s.v1.37.0. It also affects 1.36.0 and later, and main at 873d75e. The 1.35 release branch doesn't have the change below.

Component version:

What k8s version are you using (kubectl version)?:

$ kubectl version
Server Version: v1.37.0

What environment is this in?:

Azure, on a self-managed kubeadm cluster with VMSS node groups. The code involved is provider-agnostic.

What did you expect to happen?:

When a node group's new instances never register, the scale-up times out after --max-node-provision-time, CA emits ScaleUpTimedOut, and it backs off the node group.

What happened instead?:

CA records the scale-up as successful. There's no ScaleUpTimedOut and no backoff. removeOldUnregisteredNodes deletes the instance, the Pod is still pending, and CA scales up again right away, in a loop.

This run used --max-node-provision-time=3m, --initial-node-group-backoff-duration=5m and --scan-interval=10s. The log is trimmed and redacted:

21:44:45 "Final scale-up plan" scaleUpInfos="[{zero-pool 0->1 (max: 1)}]"
21:44:47 "unregistered nodes present" unregisteredNodesCount=1
21:47:54 "Removing unregistered nodes for node group" nodesCount=1 nodeGroupId="zero-pool"
21:48:54 "Final scale-up plan" scaleUpInfos="[{zero-pool 0->1 (max: 1)}]"
21:51:57 "Removing unregistered nodes for node group" nodesCount=1 nodeGroupId="zero-pool"
21:52:57 "Final scale-up plan" scaleUpInfos="[{zero-pool 0->1 (max: 1)}]"

How to reproduce it (as minimally and precisely as possible):

Add this test to pkg/clusterstate/clusterstate_test.go and run go test ./pkg/clusterstate -run TestLongUnregisteredScaleUpBacksOff. It fails on main. If the instance is first listed 15 seconds after the scale-up instead, it passes.

func TestLongUnregisteredScaleUpBacksOff(t *testing.T) {
	ctx := context.Background()
	start := time.Now()
	ready := BuildTestNode("ng1-1", 1000, 1000)
	ready.Spec.ProviderID = "ng1-1"
	SetNodeReadyState(ready, true, start.Add(-time.Hour))
	neverRegisters := BuildTestNode("ng1-2", 1000, 1000)
	neverRegisters.Spec.ProviderID = "ng1-2"
	provider := testprovider.NewTestCloudProviderBuilder().WithOnScaleUp(func(string, int) error { return nil }).Build()
	provider.AddNodeGroup("ng1", 1, 10, 2)
	provider.AddNode("ng1", ready)
	provider.AddNode("ng1", neverRegisters)
	ng := provider.GetNodeGroup("ng1")
	logRecorder, _ := utils.NewStatusMapRecorder(&fake.Clientset{}, "kube-system", kube_record.NewFakeRecorder(10), false, "status")
	csr := NewNotifiedClusterStateRegistry(provider, logRecorder, newBackoff(),
		nodegroupconfig.NewDefaultNodeGroupConfigProcessor(config.NodeGroupAutoscalingOptions{MaxNodeProvisionTime: 15 * time.Minute}),
		&mockTemplateNodeInfoRegistry{nodeInfos: map[string]*framework.NodeInfo{"ng1": framework.NewTestNodeInfo(ready)}})

	csr.RegisterScaleUp(ctx, ng, 1, start)
	// The loop right after the scale-up lists the new instance, which never registers.
	firstSeen := start.Add(time.Second)
	assert.NoError(t, csr.UpdateNodes(ctx, []*apiv1.Node{ready}, firstSeen))
	// With a 10s scan interval, the previous loop ran before the request expired.
	// This is the first loop after it expired, with a small overrun.
	now := firstSeen.Add(15*time.Minute + 100*time.Millisecond)
	assert.NoError(t, csr.UpdateNodes(ctx, []*apiv1.Node{ready}, now))
	assert.True(t, csr.BackoffStatusForNodeGroup(ctx, ng, now).IsBackedOff)
}

Anything else we need to know?:

updateScaleRequests treats "no upcoming nodes" as success before it checks the timeout, and long-unregistered nodes don't count as upcoming (calculateUpcomingNodesInNodeGroup). So when the new instance becomes long-unregistered in the same loop that the request expires, the scale-up is closed as successful. This started with kubernetes/autoscaler#9860, which was backported to 1.36 in kubernetes/autoscaler#9868. Before it, only registered nodes fulfilled a scale-up request. The test passes on its parent, 4fb69f31, and fails on it, 45e74c68. I'd be happy to fix this issue!