capi: node and provider ID accounting funcs

Signed-off-by: Jack Francis <jackfrancis@gmail.com>

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller.go

@@ -356,6 +356,11 @@ func machineKeyFromDeletingMachineProviderID(providerID normalizedProviderID) st
 	return strings.Replace(namespaceName, "_", "/", 1)
 }
 
+// createPendingMachineProviderID creates a providerID for a machine that is pending
+func createPendingMachineProviderID(namespace, name string) string {
+	return fmt.Sprintf("%s%s_%s", pendingMachinePrefix, namespace, name)
+}
+
 func isPendingMachineProviderID(providerID normalizedProviderID) bool {
 	return strings.HasPrefix(string(providerID), pendingMachinePrefix)
 }
@@ -378,6 +383,15 @@ func machineKeyFromFailedProviderID(providerID normalizedProviderID) string {
 	return strings.Replace(namespaceName, "_", "/", 1)
 }
 
+// nodeHasValidProviderID determines whether a node's providerID is the standard
+// providerID assigned by the cloud provider, or if it has
+// been modified by the CAS CAPI provider to indicate deleting, pending, or failed
+func nodeHasValidProviderID(providerID normalizedProviderID) bool {
+	return !isDeletingMachineProviderID(providerID) &&
+		!isPendingMachineProviderID(providerID) &&
+		!isFailedMachineProviderID(providerID)
+}
+
 // findNodeByNodeName finds the Node object keyed by name.. Returns
 // nil if it cannot be found. A DeepCopy() of the object is returned
 // on success.
@@ -677,7 +691,7 @@ func (c *machineController) findScalableResourceProviderIDs(scalableResource *un
 
 		if !found {
 			klog.V(4).Infof("Status.NodeRef of machine %q is currently nil", machine.GetName())
-			providerIDs = append(providerIDs, fmt.Sprintf("%s%s_%s", pendingMachinePrefix, machine.GetNamespace(), machine.GetName()))
+			providerIDs = append(providerIDs, createPendingMachineProviderID(machine.GetNamespace(), machine.GetName()))
 			continue
 		}
 

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_controller_test.go

@@ -2183,6 +2183,49 @@ func Test_isDeletingMachineProviderID(t *testing.T) {
 
 }
 
+// TestNodeHasValidProviderID tests all permutations of provider IDs
+// to determine whether the providerID is the standard cloud provider ID
+// or has been modified by CAS CAPI provider
+func TestNodeHasValidProviderID(t *testing.T) {
+	type testCase struct {
+		description    string
+		providerID     normalizedProviderID
+		expectedReturn bool
+	}
+
+	testCases := []testCase{
+		{
+			description:    "real looking provider ID should return true",
+			providerID:     normalizedProviderID("fake-provider://a.provider.id-0001"),
+			expectedReturn: true,
+		},
+		{
+			description:    "provider ID created with createDeletingMachineNormalizedProviderID should return false",
+			providerID:     normalizedProviderID(createDeletingMachineNormalizedProviderID("cluster-api", "id-0001")),
+			expectedReturn: false,
+		},
+		{
+			description:    "provider ID created with createPendingDeletionMachineNormalizedProviderID should return false",
+			providerID:     normalizedProviderID(createPendingMachineProviderID("cluster-api", "id-0001")),
+			expectedReturn: false,
+		},
+		{
+			description:    "provider ID created with createFailedMachineNormalizedProviderID should return false",
+			providerID:     normalizedProviderID(createFailedMachineNormalizedProviderID("cluster-api", "id-0001")),
+			expectedReturn: false,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			observed := nodeHasValidProviderID(tc.providerID)
+			if observed != tc.expectedReturn {
+				t.Fatalf("unexpected return for provider ID %q, expected %t, observed %t", tc.providerID, tc.expectedReturn, observed)
+			}
+		})
+	}
+}
+
 func Test_machineKeyFromDeletingMachineProviderID(t *testing.T) {
 	type testCase struct {
 		description    string
@@ -2254,3 +2297,31 @@ func Test_createDeletingMachineNormalizedProviderID(t *testing.T) {
 		})
 	}
 }
+
+// Test_createPendingMachineProviderID tests the creation of a pending machine provider ID
+func Test_createPendingMachineProviderID(t *testing.T) {
+	type testCase struct {
+		description    string
+		namespace      string
+		name           string
+		expectedReturn string
+	}
+
+	testCases := []testCase{
+		{
+			description:    "namespace and name return proper normalized ID",
+			namespace:      "cluster-api",
+			name:           "id-0001",
+			expectedReturn: fmt.Sprintf("%s%s_%s", pendingMachinePrefix, "cluster-api", "id-0001"),
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.description, func(t *testing.T) {
+			observed := createPendingMachineProviderID(tc.namespace, tc.name)
+			if observed != tc.expectedReturn {
+				t.Fatalf("unexpected return for (namespace %q, name %q), expected %q, observed %q", tc.namespace, tc.name, tc.expectedReturn, observed)
+			}
+		})
+	}
+}

cluster-autoscaler/cloudprovider/clusterapi/clusterapi_nodegroup.go

@@ -212,9 +212,7 @@ func (ng *nodegroup) DecreaseTargetSize(delta int) error {
 	// provider.
 	actualNodes := 0
 	for _, node := range nodes {
-		if !isPendingMachineProviderID(normalizedProviderID(node.Id)) &&
-			!isFailedMachineProviderID(normalizedProviderID(node.Id)) &&
-			!isDeletingMachineProviderID(normalizedProviderID(node.Id)) {
+		if nodeHasValidProviderID(normalizedProviderID(node.Id)) {
 			actualNodes += 1
 		}
 	}