Address review comments

pkg/model/components/etcdmanager/model.go

@@ -394,7 +394,7 @@ func (b *EtcdManagerBuilder) buildPod(etcdCluster *kops.EtcdClusterSpec) (*v1.Po
 			config.VolumeProvider = "do"
 
 			// DO does not support . in tags / names
-			safeClusterName := strings.Replace(b.Cluster.Name, ".", "-", -1)
+			safeClusterName := do.SafeDOClusterName(b.Cluster.Name)
 
 			config.VolumeTag = []string{
 				fmt.Sprintf("%s=%s", do.TagKubernetesClusterNamePrefix, safeClusterName),

pkg/model/domodel/droplets.go

@@ -47,7 +47,7 @@ func (d *DropletBuilder) Build(c *fi.ModelBuilderContext) error {
 	// replace "." with "-" since DO API does not accept "."
 	clusterTag := do.TagKubernetesClusterNamePrefix + ":" + strings.Replace(d.ClusterName(), ".", "-", -1)
 
-	indexCount := 0
+	masterIndexCount := 0
 	// In the future, DigitalOcean will use Machine API to manage groups,
 	// for now create d.InstanceGroups.Spec.MinSize amount of droplets
 	for _, ig := range d.InstanceGroups {
@@ -64,12 +64,12 @@ func (d *DropletBuilder) Build(c *fi.ModelBuilderContext) error {
 		droplet.Image = fi.String(ig.Spec.Image)
 		droplet.SSHKey = fi.String(sshKeyFingerPrint)
 
+		droplet.Tags = []string{clusterTag}
+
 		if ig.IsMaster() {
-			indexCount++
-			clusterTagIndex := do.TagKubernetesClusterIndex + ":" + strconv.Itoa(indexCount)
-			droplet.Tags = []string{clusterTag, clusterTagIndex}
-		} else {
-			droplet.Tags = []string{clusterTag}
+			masterIndexCount++
+			clusterTagIndex := do.TagKubernetesClusterIndex + ":" + strconv.Itoa(masterIndexCount)
+			droplet.Tags = append(droplet.Tags, clusterTagIndex)
 		}
 
 		userData, err := d.BootstrapScript.ResourceNodeUp(ig, d.Cluster)

pkg/model/master_volumes.go

@@ -178,7 +178,7 @@ func (b *MasterVolumeBuilder) addAWSVolume(c *fi.ModelBuilderContext, name strin
 
 func (b *MasterVolumeBuilder) addDOVolume(c *fi.ModelBuilderContext, name string, volumeSize int32, zone string, etcd *kops.EtcdClusterSpec, m *kops.EtcdMemberSpec, allMembers []string) {
 	// required that names start with a lower case and only contains letters, numbers and hyphens
-	name = "kops-" + strings.Replace(name, ".", "-", -1)
+	name = "kops-" + do.SafeDOClusterName(name)
 
 	// DO has a 64 character limit for volume names
 	if len(name) >= 64 {
@@ -186,11 +186,11 @@ func (b *MasterVolumeBuilder) addDOVolume(c *fi.ModelBuilderContext, name string
 	}
 
 	tags := make(map[string]string)
-	tags[do.TagNameEtcdClusterPrefix+etcd.Name] = gce.SafeClusterName(m.Name)
-	tags[do.TagKubernetesClusterIndex] = gce.SafeClusterName(m.Name)
+	tags[do.TagNameEtcdClusterPrefix+etcd.Name] = do.SafeDOClusterName(m.Name)
+	tags[do.TagKubernetesClusterIndex] = do.SafeDOClusterName(m.Name)
 
 	// We always add an owned tags (these can't be shared)
-	tags[do.TagKubernetesClusterNamePrefix] = gce.SafeClusterName(b.Cluster.ObjectMeta.Name)
+	tags[do.TagKubernetesClusterNamePrefix] = do.SafeDOClusterName(b.Cluster.ObjectMeta.Name)
 
 	t := &dotasks.Volume{
 		Name:      s(name),

upup/pkg/fi/cloudup/do/cloud.go

@@ -19,6 +19,7 @@ package do
 import (
 	"k8s.io/kops/pkg/resources/digitalocean"
 	"k8s.io/kops/upup/pkg/fi"
+	"strings"
 )
 
 const TagKubernetesClusterIndex = "k8s-index"
@@ -26,6 +27,12 @@ const TagNameEtcdClusterPrefix = "etcdCluster-"
 const TagNameRolePrefix = "k8s.io/role/"
 const TagKubernetesClusterNamePrefix = "KubernetesCluster"
 
+func SafeDOClusterName(clusterName string) string {
+	// GCE does not support . in tags / names
+	safeClusterName := strings.ReplaceAll(clusterName, ".", "-")
+	return safeClusterName
+}
+
 func NewDOCloud(region string) (fi.Cloud, error) {
 	return digitalocean.NewCloud(region)
 }

upup/pkg/fi/cloudup/dotasks/droplet.go

@@ -19,7 +19,6 @@ package dotasks
 import (
 	"context"
 	"errors"
-	//"strconv"
 	"github.com/digitalocean/godo"
 
 	"k8s.io/klog"

upup/pkg/fi/cloudup/dotasks/volume.go

@@ -18,8 +18,8 @@ package dotasks
 
 import (
 	"context"
+	"fmt"
 	"github.com/digitalocean/godo"
-	"strings"
 
 	"k8s.io/klog"
 	"k8s.io/kops/pkg/resources/digitalocean"
@@ -112,14 +112,10 @@ func (_ *Volume) RenderDO(t *do.DOAPITarget, a, e, changes *Volume) error {
 
 	tagArray := []string{}
 
-	klog.V(2).Info("Looping DO tag arrays")
 	for k, v := range e.Tags {
-		s := []string{k, v}
-
 		// DO tags don't accept =. Separate the key and value with an ":"
-		strJoin := strings.Join(s, ":")
-		klog.V(2).Infof("DO - Join the volume tag - %s", strJoin)
-		tagArray = append(tagArray, strJoin)
+		klog.V(10).Infof("DO - Join the volume tag - %s", fmt.Sprintf("%s:%s", k, v))
+		tagArray = append(tagArray, fmt.Sprintf("%s:%s", k, v))
 	}
 
 	volService := t.Cloud.Volumes()