From 70ac43c442ef528ce71f1ed27ca80d1d80592f4f Mon Sep 17 00:00:00 2001
From: Joel Speed <joel.speed@hotmail.co.uk>
Date: Thu, 26 Mar 2020 18:33:29 +0000
Subject: [PATCH] Add testing for fake provider IDs

---
 .../clusterapi/clusterapi_controller_test.go  | 54 +++++++++++++
 .../clusterapi/clusterapi_nodegroup_test.go   | 79 +++++++++++++++++++
 2 files changed, 133 insertions(+)

diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go
index 776cc8e4122c..04bce9ae2146 100644
--- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go
+++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go
@@ -1222,3 +1222,57 @@ func TestGroupVersionHasResource(t *testing.T) {
 		})
 	}
 }
+
+func TestIsFailedMachineProviderID(t *testing.T) {
+	testCases := []struct{
+		name string
+		providerID normalizedProviderID
+		expected bool
+	}{
+		{
+			name: "with the failed machine prefix",
+			providerID: normalizedProviderID(fmt.Sprintf("%sfoo", failedMachinePrefix)),
+			expected: true,
+		},
+		{
+			name: "without the failed machine prefix",
+			providerID: normalizedProviderID("foo"),
+			expected: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := isFailedMachineProviderID(tc.providerID); got != tc.expected {
+				t.Errorf("test case: %s, expected: %v, got: %v", tc.name, tc.expected, got)
+			}
+		})
+	}
+}
+
+func TestMachineKeyFromFailedProviderID(t *testing.T) {
+	testCases := []struct{
+		name string
+		providerID normalizedProviderID
+		expected string
+	}{
+		{
+			name: "with a valid failed machine prefix",
+			providerID: normalizedProviderID(fmt.Sprintf("%stest-namespace_foo", failedMachinePrefix)),
+			expected: "test-namespace/foo",
+		},
+		{
+			name: "with a machine with an underscore in the name",
+			providerID: normalizedProviderID(fmt.Sprintf("%stest-namespace_foo_bar", failedMachinePrefix)),
+			expected: "test-namespace/foo_bar",
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := machineKeyFromFailedProviderID(tc.providerID); got != tc.expected {
+				t.Errorf("test case: %s, expected: %q, got: %q", tc.name, tc.expected, got)
+			}
+		})
+	}
+}
diff --git a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go
index 51045309bc10..d880134756cf 100644
--- a/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go
+++ b/cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup_test.go
@@ -943,3 +943,82 @@ func TestNodeGroupDeleteNodesTwice(t *testing.T) {
 		}))
 	})
 }
+
+func TestNodeGroupWithFailedMachine(t *testing.T) {
+	test := func(t *testing.T, testConfig *testConfig) {
+		controller, stop := mustCreateTestController(t, testConfig)
+		defer stop()
+
+		// Simulate a failed machine
+		machine := testConfig.machines[3].DeepCopy()
+		machine.Spec.ProviderID = nil
+		machine.Status.Phase = "Failed"
+		if err := controller.machineInformer.Informer().GetStore().Update(newUnstructuredFromMachine(machine)); err != nil {
+			t.Fatalf("unexpected error updating machine, got %v", err)
+		}
+
+		nodegroups, err := controller.nodeGroups()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		if l := len(nodegroups); l != 1 {
+			t.Fatalf("expected 1 nodegroup, got %d", l)
+		}
+
+		ng := nodegroups[0]
+		nodeNames, err := ng.Nodes()
+		if err != nil {
+			t.Fatalf("unexpected error: %v", err)
+		}
+
+		if len(nodeNames) != len(testConfig.nodes) {
+			t.Fatalf("expected len=%v, got len=%v", len(testConfig.nodes), len(nodeNames))
+		}
+
+		sort.SliceStable(nodeNames, func(i, j int) bool {
+			return nodeNames[i].Id < nodeNames[j].Id
+		})
+
+		// The failed machine key is sorted to the first index
+		failedMachineID := fmt.Sprintf("%s%s_%s", failedMachinePrefix, machine.Namespace, machine.Name)
+		if nodeNames[0].Id != failedMachineID {
+			t.Fatalf("expected %q, got %q", failedMachineID, nodeNames[0].Id)
+		}
+
+		for i := 1; i < len(nodeNames); i++ {
+			// Fix the indexing due the failed machine being removed from the list
+			var nodeIndex int
+			if i < 4 {
+				// for nodes 0, 1, 2
+				nodeIndex = i - 1
+			} else {
+				// for nodes 4 onwards
+				nodeIndex = i
+			}
+
+			if nodeNames[i].Id != string(normalizedProviderString(testConfig.nodes[nodeIndex].Spec.ProviderID)) {
+				t.Fatalf("expected %q, got %q", testConfig.nodes[nodeIndex].Spec.ProviderID, nodeNames[i].Id)
+			}
+		}
+	}
+
+	// Note: 10 is an upper bound for the number of nodes/replicas
+	// Going beyond 10 will break the sorting that happens in the
+	// test() function because sort.Strings() will not do natural
+	// sorting and the expected semantics in test() will fail.
+
+	t.Run("MachineSet", func(t *testing.T) {
+		test(t, createMachineSetTestConfig(testNamespace, 10, map[string]string{
+			nodeGroupMinSizeAnnotationKey: "1",
+			nodeGroupMaxSizeAnnotationKey: "10",
+		}))
+	})
+
+	t.Run("MachineDeployment", func(t *testing.T) {
+		test(t, createMachineDeploymentTestConfig(testNamespace, 10, map[string]string{
+			nodeGroupMinSizeAnnotationKey: "1",
+			nodeGroupMaxSizeAnnotationKey: "10",
+		}))
+	})
+}