Fetch providerID from DigitalOcean API as a fallback if not specified on the node object (#842)

Author: llDrLove

CHANGELOG.md

@@ -2,6 +2,9 @@
 
 * Add optional `DEFAULT_LB_TYPE` environment variable to configure the default load balancer type between `REGIONAL` and `REGIONAL_NETWORK` (@olove)
 * Restrict the kube-proxy health port to be accessible by the load balancer only instead of allowing all IPv4 and IPv6 addresses (@olove)
+* Fixes an issue with nodes that don't have their providerID set from the kubelet configuration. From now on, 
+  if the node object `node.Spec.ProviderID` property is empty, we will retrieve the information using the DO API by 
+  listing the droplets with that name.
 
 ## v0.1.61 (beta) - May 29, 2025
 * Ensure Network LoadBalancer (NLB) is the default loadbalancer type (@olove)

cloud-controller-manager/do/common.go

@@ -29,12 +29,13 @@ import (
 // are very large and the response gets too big with 200 objects
 const apiResultsPerPage = 50
 
-func allDropletList(ctx context.Context, client *godo.Client) ([]godo.Droplet, error) {
-	list := []godo.Droplet{}
+func allDropletList(ctx context.Context, listFunc func(ctx context.Context, opt *godo.ListOptions) ([]godo.Droplet, *godo.Response, error)) ([]godo.Droplet, error) {
+	var list []godo.Droplet
 
 	opt := &godo.ListOptions{Page: 1, PerPage: apiResultsPerPage}
 	for {
-		droplets, resp, err := client.Droplets.List(ctx, opt)
+		droplets, resp, err := listFunc(ctx, opt)
+
 		if err != nil {
 			return nil, err
 		}

cloud-controller-manager/do/common_test.go

@@ -122,7 +122,9 @@ func TestAllDropletList(t *testing.T) {
 		},
 	)
 
-	droplets, err := allDropletList(context.Background(), client)
+	droplets, err := allDropletList(context.Background(), func(ctx context.Context, opt *godo.ListOptions) ([]godo.Droplet, *godo.Response, error) {
+		return client.Droplets.List(ctx, opt)
+	})
 	if err != nil {
 		t.Fatalf("unexpected err: %s", err)
 	}

cloud-controller-manager/do/droplets.go

@@ -26,11 +26,13 @@ import (
 
 	"github.com/digitalocean/godo"
 	v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/types"
 	cloudprovider "k8s.io/cloud-provider"
 )
 
 const (
 	dropletShutdownStatus = "off"
+	providerIDPrefix      = "digitalocean://"
 )
 
 // instances implements the InstancesV2() interface
@@ -92,25 +94,66 @@ func (i *instances) InstanceShutdown(ctx context.Context, node *v1.Node) (bool,
 	return droplet.Status == dropletShutdownStatus, nil
 }
 
-func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
-	dropletID, err := dropletIDFromProviderID(node.Spec.ProviderID)
+// dropletByName returns a *godo.Droplet for the droplet identified by nodeName.
+//
+// When nodeName identifies more than one droplet, only the first will be
+// considered.
+func dropletByName(ctx context.Context, client *godo.Client, nodeName types.NodeName) (*godo.Droplet, error) {
+	droplets, err := allDropletList(ctx, func(ctx context.Context, opt *godo.ListOptions) ([]godo.Droplet, *godo.Response, error) {
+		return client.Droplets.ListByName(ctx, string(nodeName), opt)
+	})
 	if err != nil {
-		return nil, fmt.Errorf("determining droplet ID from providerID: %s", err.Error())
+		return nil, err
 	}
 
-	droplet, err := dropletByID(ctx, i.resources.gclient, dropletID)
-	if err != nil {
-		return nil, fmt.Errorf("getting droplet by ID: %s: ", err.Error())
+	for _, droplet := range droplets {
+		if droplet.Name == string(nodeName) {
+			return &droplet, nil
+		}
+		addresses, _ := nodeAddresses(&droplet)
+		for _, address := range addresses {
+			if address.Address == string(nodeName) {
+				return &droplet, nil
+			}
+		}
 	}
-	if droplet == nil {
-		return nil, fmt.Errorf("droplet %d for node %s does not exist", dropletID, node.Name)
+
+	return nil, cloudprovider.InstanceNotFound
+}
+
+func (i *instances) InstanceMetadata(ctx context.Context, node *v1.Node) (*cloudprovider.InstanceMetadata, error) {
+	var (
+		dropletID int
+		err       error
+		droplet   *godo.Droplet
+	)
+
+	if node.Spec.ProviderID == "" {
+		droplet, err = dropletByName(ctx, i.resources.gclient, types.NodeName(node.GetName()))
+		if err != nil {
+			return nil, fmt.Errorf("getting droplet by name: %s", err.Error())
+		}
+		dropletID = droplet.ID
+	} else {
+		dropletID, err = dropletIDFromProviderID(node.Spec.ProviderID)
+		if err != nil {
+			return nil, fmt.Errorf("determining droplet ID from providerID: %s", err.Error())
+		}
+		droplet, err = dropletByID(ctx, i.resources.gclient, dropletID)
+		if err != nil {
+			return nil, fmt.Errorf("getting droplet by ID: %s: ", err.Error())
+		}
+		if droplet == nil {
+			return nil, fmt.Errorf("droplet %d for node %s does not exist", dropletID, node.Name)
+		}
 	}
+
 	nodeAddrs, err := nodeAddresses(droplet)
 	if err != nil {
 		return nil, fmt.Errorf("getting node addresses of droplet %d for node %s: %s", dropletID, node.Name, err.Error())
 	}
 	return &cloudprovider.InstanceMetadata{
-		ProviderID:    node.Spec.ProviderID, // the providerID may or may not be present according to the interface doc. However, we set this from kubelet.
+		ProviderID:    fmt.Sprintf("%s%d", providerIDPrefix, dropletID),
 		InstanceType:  droplet.SizeSlug,
 		Region:        droplet.Region.Slug,
 		NodeAddresses: nodeAddrs,
@@ -132,13 +175,11 @@ func dropletIDFromProviderID(providerID string) (int, error) {
 		return 0, errors.New("provider ID cannot be empty")
 	}
 
-	const prefix = "digitalocean://"
-
-	if !strings.HasPrefix(providerID, prefix) {
-		return 0, fmt.Errorf("provider ID %q is missing prefix %q", providerID, prefix)
+	if !strings.HasPrefix(providerID, providerIDPrefix) {
+		return 0, fmt.Errorf("provider ID %q is missing prefix %q", providerID, providerIDPrefix)
 	}
 
-	provIDNum := strings.TrimPrefix(providerID, prefix)
+	provIDNum := strings.TrimPrefix(providerID, providerIDPrefix)
 	if provIDNum == "" {
 		return 0, errors.New("provider ID number cannot be empty")
 	}

cloud-controller-manager/do/droplets_test.go

@@ -339,6 +339,57 @@ func TestInstanceMetadata(t *testing.T) {
 	}
 }
 
+func TestInstanceMetadataWithoutProviderID(t *testing.T) {
+	fake := &fakeDropletService{}
+	fake.listByNameFunc = func(ctx context.Context, name string, opt *godo.ListOptions) ([]godo.Droplet, *godo.Response, error) {
+		droplet := newFakeDroplet()
+		resp := newFakeOKResponse()
+		return []godo.Droplet{*droplet}, resp, nil
+	}
+	res := &resources{gclient: newFakeDropletClient(fake)}
+	instances := newInstances(res, "nyc1")
+
+	expectedAddresses := []v1.NodeAddress{
+		{
+			Type:    v1.NodeHostName,
+			Address: "test-droplet",
+		},
+		{
+			Type:    v1.NodeInternalIP,
+			Address: "10.0.0.0",
+		},
+		{
+			Type:    v1.NodeExternalIP,
+			Address: "99.99.99.99",
+		},
+	}
+
+	metadata, err := instances.InstanceMetadata(context.TODO(), &v1.Node{
+		ObjectMeta: metav1.ObjectMeta{
+			Name: "test-droplet",
+		},
+		Spec: v1.NodeSpec{},
+	})
+	if err != nil {
+		t.Errorf("unexpected err, expected nil. got: %v", err)
+	}
+
+	if !reflect.DeepEqual(metadata.NodeAddresses, expectedAddresses) {
+		t.Errorf("unexpected node addresses. got: %v want: %v", metadata.NodeAddresses, expectedAddresses)
+	}
+
+	if metadata.ProviderID != "digitalocean://123" {
+		t.Errorf("unexpected node providerID. got: %v, want: %v", metadata.ProviderID, "digitalocean://123")
+	}
+
+	if metadata.InstanceType != "2gb" {
+		t.Errorf("unexpected node instance type. got: %v want: %v", metadata.InstanceType, newFakeDroplet().SizeSlug)
+	}
+	if metadata.Region != "test1" {
+		t.Errorf("unexpected node region. got: %v want: %v", metadata.Region, newFakeDroplet().Region)
+	}
+}
+
 func Test_dropletIDFromProviderID(t *testing.T) {
 	testcases := []struct {
 		name       string

cloud-controller-manager/do/loadbalancers.go

@@ -533,7 +533,9 @@ func (l *loadBalancers) nodesToDropletIDs(ctx context.Context, nodes []*v1.Node)
 
 	if len(missingDroplets) > 0 {
 		// Discover missing droplets by matching names.
-		droplets, err := allDropletList(ctx, l.resources.gclient)
+		droplets, err := allDropletList(ctx, func(ctx context.Context, opt *godo.ListOptions) ([]godo.Droplet, *godo.Response, error) {
+			return l.resources.gclient.Droplets.List(ctx, opt)
+		})
 		if err != nil {
 			return nil, fmt.Errorf("failed to list all droplets: %s", err)
 		}