fix(cluster-autoscaler): prevent panic in SimulateNodeRemoval by handling missing node info

Author: kincoy

cluster-autoscaler/simulator/cluster.go

@@ -17,10 +17,12 @@ limitations under the License.
 package simulator
 
 import (
+	"errors"
 	"fmt"
 	"time"
 
 	apiv1 "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/autoscaler/cluster-autoscaler/core/scaledown/pdb"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/clustersnapshot"
 	"k8s.io/autoscaler/cluster-autoscaler/simulator/drainability/rules"
@@ -91,6 +93,8 @@ const (
 	BlockedByPod
 	// UnexpectedError - node can't be removed because of an unexpected error.
 	UnexpectedError
+	// NoNodeInfo - node can't be removed because it doesn't have any node info in the cluster snapshot.
+	NoNodeInfo
 )
 
 // RemovalSimulator is a helper object for simulating node removal scenarios.
@@ -151,6 +155,12 @@ func (r *RemovalSimulator) SimulateNodeRemoval(
 	nodeInfo, err := r.clusterSnapshot.GetNodeInfo(nodeName)
 	if err != nil {
 		klog.Errorf("Can't retrieve node %s from snapshot, err: %v", nodeName, err)
+		unremovableReason := UnexpectedError
+		if errors.Is(err, clustersnapshot.ErrNodeNotFound) {
+			unremovableReason = NoNodeInfo
+		}
+		unremovableNode := &UnremovableNode{Node: &apiv1.Node{ObjectMeta: metav1.ObjectMeta{Name: nodeName}}, Reason: unremovableReason}
+		return nil, unremovableNode
 	}
 	klog.V(2).Infof("Simulating node %s removal", nodeName)
 

cluster-autoscaler/simulator/cluster_test.go

@@ -88,6 +88,9 @@ func TestFindNodesToRemove(t *testing.T) {
 	fullNode := BuildTestNode("n4", 1000, 2000000)
 	fullNodeInfo := framework.NewTestNodeInfo(fullNode)
 
+	// node with no info in cluster snapshot
+	nodeWithoutInfo := &apiv1.Node{ObjectMeta: metav1.ObjectMeta{Name: "n5"}}
+
 	SetNodeReadyState(emptyNode, true, time.Time{})
 	SetNodeReadyState(drainableNode, true, time.Time{})
 	SetNodeReadyState(nonDrainableNode, true, time.Time{})
@@ -137,6 +140,10 @@ func TestFindNodesToRemove(t *testing.T) {
 		Node:             drainableNode,
 		PodsToReschedule: []*apiv1.Pod{pod1, pod2},
 	}
+	ghostNodeToUnremovable := UnremovableNode{
+		Node:   nodeWithoutInfo,
+		Reason: NoNodeInfo,
+	}
 
 	clusterSnapshot := testsnapshot.NewTestSnapshotOrDie(t)
 
@@ -240,6 +247,14 @@ func TestFindNodesToRemove(t *testing.T) {
 				{Node: topoNode3, Reason: BlockedByPod, BlockingPod: &drain.BlockingPod{Pod: blocker2, Reason: drain.NotReplicated}},
 			},
 		},
+		{
+			name:        "candidate not in clusterSnapshot should be marked unremovable",
+			candidates:  []string{nodeWithoutInfo.Name},
+			allNodes:    []*apiv1.Node{},
+			pods:        []*apiv1.Pod{},
+			toRemove:    nil,
+			unremovable: []*UnremovableNode{&ghostNodeToUnremovable},
+		},
 	}
 
 	for _, test := range tests {