Use list VMs with filtering when fetching VMs

Using GET VMs call can result in 403 if the VM is not found or if the credentials are incorrect. To avoid ambiguity use list VMs call with filtering based on name or IDs which will result in empty list if the VM is not found.

Also handles mismatch in providerID in case the VM is deleted and recreated with same name again. In such cases, we query crusoe APIs to fetch the VM by name.

Updates the deployment to run CCM in host network to remove dependency on a functional CNI

Author: Pushkar Acharya

.gitignore

@@ -9,3 +9,4 @@ tests.xml
 coverage.xml
 .vscode/*
 .idea/*
+vendor/*

internal/client/client.go

@@ -83,14 +83,20 @@ func (a *APIClientImpl) GetInstanceByID(ctx context.Context,
 	}
 
 	klog.Infof("getInstanceByID: %s", instanceID)
-	instance, response, err := a.CrusoeAPIClient.VMsApi.GetInstance(ctx, projectID, instanceID)
+	listVMOpts := &crusoeapi.VMsApiListInstancesOpts{
+		Ids: optional.NewString(instanceID),
+	}
+	instances, response, err := a.CrusoeAPIClient.VMsApi.ListInstances(ctx, projectID, listVMOpts)
 	if err != nil {
 		return nil, response, fmt.Errorf("failed to list instances: %w", err)
 	}
 	if response != nil {
 		defer response.Body.Close()
 	}
-	klog.Infof("getInstanceByID: %v", instance)
+	klog.Infof("getInstanceByID: %v", instances)
+	if len(instances.Items) == 0 {
+		return nil, nil, ErrInstanceNotFound
+	}
 
-	return &instance, response, nil
+	return &instances.Items[0], response, nil
 }

internal/instances/instances.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"fmt"
+	"strings"
 	"sync"
 	"time"
 
@@ -168,12 +169,19 @@ func (i *Instances) InstanceExists(ctx context.Context, node *v1.Node) (bool, er
 
 func (i *Instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
 	klog.Infof("Get Instance Metadata for (%v)", node.Name)
-	currInstance, responseBody, err := i.apiClient.GetInstanceByID(ctx, getInstanceIDFromProviderID(node.Spec.ProviderID))
+	prefixedProviderID, err := getProviderID(ctx, node, i)
+	if err != nil {
+		klog.Errorf("could not get provider ID from Crusoe Cloud %v", err)
+
+		return nil, err
+	}
+	providerID := getInstanceIDFromProviderID(prefixedProviderID)
+	currInstance, responseBody, err := i.apiClient.GetInstanceByID(ctx, providerID)
 	if responseBody != nil {
 		defer responseBody.Body.Close()
 	}
 	if err != nil {
-		return nil, fmt.Errorf("failed to get instance by ID %s: %w", node.Spec.ProviderID, err)
+		return nil, fmt.Errorf("failed to get instance by ID %s: %w", providerID, err)
 	}
 	klog.Infof("InstanceMetadata for (%v:%v)", node.Name, currInstance)
 	nodeAddress, err := getNodeAddress(currInstance)
@@ -218,6 +226,17 @@ func NewCrusoeInstances(c client.APIClient) *Instances {
 
 func getProviderID(ctx context.Context, node *v1.Node, i *Instances) (string, error) {
 	providerID := node.Spec.ProviderID
+	// While kubelet does not update the node.spec or the node.status.addresses or metadata fields when
+	// node information changes it is still able to update the nodeInfo field in node.status. Kubelet updates
+	// node.Status.NodeInfo.SystemUUID to /sys/class/dmi/id/product_uuid which is the correct VM ID in Crusoe Cloud
+	if node.Status.NodeInfo.SystemUUID != "" &&
+		getInstanceIDFromProviderID(node.Spec.ProviderID) != node.Status.NodeInfo.SystemUUID {
+
+		klog.Warningf("ProviderID and SystemUUID do not match; providerID: "+
+			"%s; systemUUID: %s. Fetching UUID from Crusoe Cloud directly.",
+			providerID, node.Status.NodeInfo.SystemUUID)
+		providerID = ""
+	}
 	if providerID == "" {
 		currInstance, err := i.apiClient.GetInstanceByName(ctx, node.Name)
 		if err != nil {
@@ -230,7 +249,7 @@ func getProviderID(ctx context.Context, node *v1.Node, i *Instances) (string, er
 }
 
 func getInstanceIDFromProviderID(providerID string) string {
-	return providerID[len(ProviderPrefix):]
+	return strings.TrimPrefix(providerID, ProviderPrefix)
 }
 
 func getNodeAddress(currInstance *crusoeapi.InstanceV1Alpha5) ([]v1.NodeAddress, error) {
@@ -243,7 +262,7 @@ func getNodeAddress(currInstance *crusoeapi.InstanceV1Alpha5) ([]v1.NodeAddress,
 		Address: currInstance.NetworkInterfaces[0].Ips[0].PublicIpv4.Address,
 	}, v1.NodeAddress{
 		Type:    v1.NodeHostName,
-		Address: currInstance.Name,
+		Address: fmt.Sprintf("%s.%s.compute.internal", currInstance.Name, currInstance.Location),
 	},
 	)
 

internal/instances/instances_test.go

@@ -2,6 +2,7 @@ package instances_test
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"testing"
 
@@ -23,6 +24,7 @@ const (
 	ProviderIDPrefix = "crusoe://"
 	CrusoeProjectID  = "CRUSOE_PROJECT_ID"
 	TestProjectID    = "1841af90-a4f6-4412-8b23-b7035a6c72ae"
+	TestLocation     = "us-easttesting1-a"
 )
 
 func TestNodeAddresses(t *testing.T) {
@@ -47,15 +49,16 @@ func TestNodeAddresses(t *testing.T) {
 				},
 			},
 		},
-		Name: "node1",
+		Name:     "node1",
+		Location: TestLocation,
 	}, nil)
 
 	addresses, err := instanceService.NodeAddresses(context.Background(), types.NodeName(TESTNodeName))
 	require.NoError(t, err)
 	require.Len(t, addresses, 3)
 	require.Equal(t, "10.0.0.1", addresses[0].Address)
 	require.Equal(t, "192.168.0.1", addresses[1].Address)
-	require.Equal(t, "node1", addresses[2].Address)
+	require.Equal(t, "node1.us-easttesting1-a.compute.internal", addresses[2].Address)
 }
 
 func TestInstanceID(t *testing.T) {
@@ -126,7 +129,8 @@ func TestInstanceMetadata(t *testing.T) {
 				},
 			},
 		},
-		Name: TESTNodeName,
+		Name:     TESTNodeName,
+		Location: TestLocation,
 	}, nil, nil)
 
 	node := &v1.Node{
@@ -139,7 +143,7 @@ func TestInstanceMetadata(t *testing.T) {
 	require.NoError(t, err)
 	require.NotNil(t, metadata)
 	require.Equal(t, ProviderIDPrefix+TESTInstanceID, metadata.ProviderID)
-	require.Equal(t, TESTNodeName, metadata.NodeAddresses[2].Address)
+	require.Equal(t, fmt.Sprintf("%s.%s.compute.internal", TESTNodeName, TestLocation), metadata.NodeAddresses[2].Address)
 }
 
 func TestNodeAddressesByProviderID(t *testing.T) {
@@ -163,15 +167,16 @@ func TestNodeAddressesByProviderID(t *testing.T) {
 				},
 			},
 		},
-		Name: TESTNodeName,
+		Name:     TESTNodeName,
+		Location: TestLocation,
 	}, nil, nil)
 
 	addresses, err := instanceService.NodeAddressesByProviderID(context.Background(), ProviderIDPrefix+TESTInstanceID)
 	require.NoError(t, err)
 	require.Len(t, addresses, 3)
 	require.Equal(t, "10.0.0.1", addresses[0].Address)
 	require.Equal(t, "192.168.0.1", addresses[1].Address)
-	require.Equal(t, TESTNodeName, addresses[2].Address)
+	require.Equal(t, fmt.Sprintf("%s.%s.compute.internal", TESTNodeName, TestLocation), addresses[2].Address)
 }
 
 func TestGetInstanceType(t *testing.T) {

releases/crusoe-cloud-controller-manager/v0.1.2.yaml

@@ -56,6 +56,8 @@ spec:
                     values:
                       - crusoe-cloud-controller-manager
               topologyKey: "kubernetes.io/hostname"
+      # needed to remove dependency on CNI
+      hostNetwork: true
       containers:
         - name: crusoe-cloud-controller-manager
           image:  ghcr.io/crusoecloud/crusoe-cloud-controller-manager:v0.1.0