doks: use providerID for node.ID

kubernetes · Oct 21, 2019 · 7e69503 · 7e69503
1 parent 1d39163
commit 7e69503
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 56 deletions.
diff --git a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider.go
@@ -17,8 +17,10 @@ limitations under the License.
 package digitalocean
 
 import (
+	"fmt"
 	"io"
 	"os"
+	"strings"
 
 	apiv1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/api/resource"
@@ -33,6 +35,8 @@ var _ cloudprovider.CloudProvider = (*digitaloceanCloudProvider)(nil)
 const (
 	// GPULabel is the label added to nodes with GPU resource.
 	GPULabel = "cloud.digitalocean.com/gpu-node"
+
+	doProviderIDPrefix = "digitalocean://"
 )
 
 // digitaloceanCloudProvider implements CloudProvider interface.
@@ -70,13 +74,8 @@ func (d *digitaloceanCloudProvider) NodeGroups() []cloudprovider.NodeGroup {
 // should not be processed by cluster autoscaler, or non-nil error if such
 // occurred. Must be implemented.
 func (d *digitaloceanCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudprovider.NodeGroup, error) {
-	nodeID, ok := node.Labels[nodeIDLabel]
-	if !ok {
-		// CA creates fake node objects to represent upcoming VMs that haven't
-		// registered as nodes yet. They have node.Spec.ProviderID set. Use
-		// that as nodeID.
-		nodeID = node.Spec.ProviderID
-	}
+	providerID := node.Spec.ProviderID
+	nodeID := toNodeID(providerID)
 
 	klog.V(5).Infof("checking nodegroup for node ID: %q", nodeID)
 
@@ -91,8 +90,10 @@ func (d *digitaloceanCloudProvider) NodeGroupForNode(node *apiv1.Node) (cloudpro
 		}
 
 		for _, node := range nodes {
-			klog.V(6).Infof("checking node has: %q want: %q", node.Id, nodeID)
-			if node.Id != nodeID {
+			klog.V(6).Infof("checking node has: %q want: %q", node.Id, providerID)
+			// CA uses node.Spec.ProviderID when looking for (un)registered nodes,
+			// so we need to use it here too.
+			if node.Id != providerID {
 				continue
 			}
 
@@ -191,3 +192,13 @@ func BuildDigitalOcean(
 
 	return provider
 }
+
+// toProviderID returns a provider ID from the given node ID.
+func toProviderID(nodeID string) string {
+	return fmt.Sprintf("%s%s", doProviderIDPrefix, nodeID)
+}
+
+// toNodeID returns a node or droplet ID from the given provider ID.
+func toNodeID(providerID string) string {
+	return strings.TrimPrefix(providerID, doProviderIDPrefix)
+}
diff --git a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider_test.go b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_cloud_provider_test.go
@@ -22,9 +22,9 @@ import (
 	"testing"
 
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 
 	apiv1 "k8s.io/api/core/v1"
-	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider"
 	"k8s.io/autoscaler/cluster-autoscaler/cloudprovider/digitalocean/godo"
 )
@@ -131,16 +131,16 @@ func TestDigitalOceanCloudProvider_NodeGroupForNode(t *testing.T) {
 				{
 					ID: "1",
 					Nodes: []*godo.KubernetesNode{
-						{ID: "2", Status: &godo.KubernetesNodeStatus{State: "deleting"}},
-						{ID: "3", Status: &godo.KubernetesNodeStatus{State: "running"}},
+						{ID: "2", Status: &godo.KubernetesNodeStatus{State: "deleting"}, DropletID: "droplet-2"},
+						{ID: "3", Status: &godo.KubernetesNodeStatus{State: "running"}, DropletID: "droplet-3"},
 					},
 					AutoScale: true,
 				},
 				{
 					ID: "2",
 					Nodes: []*godo.KubernetesNode{
-						{ID: "4", Status: &godo.KubernetesNodeStatus{State: "provisioning"}},
-						{ID: "5", Status: &godo.KubernetesNodeStatus{State: "draining"}},
+						{ID: "4", Status: &godo.KubernetesNodeStatus{State: "provisioning"}, DropletID: "droplet-4"},
+						{ID: "5", Status: &godo.KubernetesNodeStatus{State: "draining"}, DropletID: "droplet-5"},
 					},
 					AutoScale: true,
 				},
@@ -153,17 +153,15 @@ func TestDigitalOceanCloudProvider_NodeGroupForNode(t *testing.T) {
 
 		// let's get the nodeGroup for the node with ID 4
 		node := &apiv1.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Labels: map[string]string{
-					nodeIDLabel: "4",
-				},
+			Spec: apiv1.NodeSpec{
+				ProviderID: toProviderID("droplet-4"),
 			},
 		}
 
 		nodeGroup, err := provider.NodeGroupForNode(node)
-		assert.NoError(t, err)
-		assert.NotNil(t, nodeGroup)
-		assert.Equal(t, nodeGroup.Id(), "2", "node group ID does not match")
+		require.NoError(t, err)
+		require.NotNil(t, nodeGroup)
+		require.Equal(t, nodeGroup.Id(), "2", "node group ID does not match")
 	})
 
 	t.Run("node does not exist", func(t *testing.T) {
@@ -175,15 +173,15 @@ func TestDigitalOceanCloudProvider_NodeGroupForNode(t *testing.T) {
 				{
 					ID: "1",
 					Nodes: []*godo.KubernetesNode{
-						{ID: "2", Status: &godo.KubernetesNodeStatus{State: "deleting"}},
-						{ID: "3", Status: &godo.KubernetesNodeStatus{State: "running"}},
+						{ID: "2", Status: &godo.KubernetesNodeStatus{State: "deleting"}, DropletID: "droplet-2"},
+						{ID: "3", Status: &godo.KubernetesNodeStatus{State: "running"}, DropletID: "droplet-3"},
 					},
 				},
 				{
 					ID: "2",
 					Nodes: []*godo.KubernetesNode{
-						{ID: "4", Status: &godo.KubernetesNodeStatus{State: "provisioning"}},
-						{ID: "5", Status: &godo.KubernetesNodeStatus{State: "draining"}},
+						{ID: "4", Status: &godo.KubernetesNodeStatus{State: "provisioning"}, DropletID: "droplet-4"},
+						{ID: "5", Status: &godo.KubernetesNodeStatus{State: "draining"}, DropletID: "droplet-5"},
 					},
 				},
 			},
@@ -194,10 +192,8 @@ func TestDigitalOceanCloudProvider_NodeGroupForNode(t *testing.T) {
 		provider := testCloudProvider(t, client)
 
 		node := &apiv1.Node{
-			ObjectMeta: metav1.ObjectMeta{
-				Labels: map[string]string{
-					nodeIDLabel: "7",
-				},
+			Spec: apiv1.NodeSpec{
+				ProviderID: toProviderID("droplet-7"),
 			},
 		}
 

diff --git a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_node_group.go b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_node_group.go
@@ -233,9 +233,9 @@ func (n *NodeGroup) Autoprovisioned() bool {
 // toInstances converts a slice of *godo.KubernetesNode to
 // cloudprovider.Instance
 func toInstances(nodes []*godo.KubernetesNode) []cloudprovider.Instance {
-	instances := make([]cloudprovider.Instance, len(nodes))
-	for i, nd := range nodes {
-		instances[i] = toInstance(nd)
+	instances := make([]cloudprovider.Instance, 0, len(nodes))
+	for _, nd := range nodes {
+		instances = append(instances, toInstance(nd))
 	}
 	return instances
 }
@@ -244,7 +244,7 @@ func toInstances(nodes []*godo.KubernetesNode) []cloudprovider.Instance {
 // cloudprovider.Instance
 func toInstance(node *godo.KubernetesNode) cloudprovider.Instance {
 	return cloudprovider.Instance{
-		Id:     node.ID,
+		Id:     toProviderID(node.DropletID),
 		Status: toInstanceStatus(node.Status),
 	}
 }

diff --git a/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_node_group_test.go b/cluster-autoscaler/cloudprovider/digitalocean/digitalocean_node_group_test.go
@@ -283,55 +283,60 @@ func TestNodeGroup_Nodes(t *testing.T) {
 					Status: &godo.KubernetesNodeStatus{
 						State: "provisioning",
 					},
+					DropletID: "droplet-1",
 				},
 				{
 					ID: "2",
 					Status: &godo.KubernetesNodeStatus{
 						State: "running",
 					},
+					DropletID: "droplet-2",
 				},
 				{
 					ID: "3",
 					Status: &godo.KubernetesNodeStatus{
 						State: "deleting",
 					},
+					DropletID: "droplet-3",
 				},
 				{
 					ID: "4",
 					Status: &godo.KubernetesNodeStatus{
 						State:   "unknown",
 						Message: "some-message",
 					},
+					DropletID: "droplet-4",
 				},
 				{
 					// no status
-					ID: "5",
+					ID:        "5",
+					DropletID: "droplet-5",
 				},
 			},
 			Count: 5,
 		})
 
 		exp := []cloudprovider.Instance{
 			{
-				Id: "1",
+				Id: "digitalocean://droplet-1",
 				Status: &cloudprovider.InstanceStatus{
 					State: cloudprovider.InstanceCreating,
 				},
 			},
 			{
-				Id: "2",
+				Id: "digitalocean://droplet-2",
 				Status: &cloudprovider.InstanceStatus{
 					State: cloudprovider.InstanceRunning,
 				},
 			},
 			{
-				Id: "3",
+				Id: "digitalocean://droplet-3",
 				Status: &cloudprovider.InstanceStatus{
 					State: cloudprovider.InstanceDeleting,
 				},
 			},
 			{
-				Id: "4",
+				Id: "digitalocean://droplet-4",
 				Status: &cloudprovider.InstanceStatus{
 					ErrorInfo: &cloudprovider.InstanceErrorInfo{
 						ErrorClass:   cloudprovider.OtherErrorClass,
@@ -341,7 +346,7 @@ func TestNodeGroup_Nodes(t *testing.T) {
 				},
 			},
 			{
-				Id: "5",
+				Id: "digitalocean://droplet-5",
 			},
 		}
 

diff --git a/cluster-autoscaler/cloudprovider/digitalocean/godo/kubernetes.go b/cluster-autoscaler/cloudprovider/digitalocean/godo/kubernetes.go
@@ -311,9 +311,10 @@ type KubernetesNodePool struct {
 
 // KubernetesNode represents a Node in a node pool in a Kubernetes cluster.
 type KubernetesNode struct {
-	ID     string                `json:"id,omitempty"`
-	Name   string                `json:"name,omitempty"`
-	Status *KubernetesNodeStatus `json:"status,omitempty"`
+	ID        string                `json:"id,omitempty"`
+	Name      string                `json:"name,omitempty"`
+	Status    *KubernetesNodeStatus `json:"status,omitempty"`
+	DropletID string                `json:"droplet_id,omitempty"`
 
 	CreatedAt time.Time `json:"created_at,omitempty"`
 	UpdatedAt time.Time `json:"updated_at,omitempty"`