Merge pull request #941 from justinsb/switch_to_sets_string

Switch to use sets.String

cmd/kops/create_cluster.go

@@ -204,7 +204,7 @@ func RunCreateCluster(f *util.Factory, cmd *cobra.Command, args []string, out io
 		return fmt.Errorf("unknown networking mode %q", c.Networking)
 	}
 
-	glog.Infof("networking selected %s, %s", c.Networking, cluster.Spec.Networking)
+	glog.V(4).Infof("networking mode=%s => %s", c.Networking, fi.DebugAsJsonString(cluster.Spec.Networking))
 
 	if c.Zones != "" {
 		existingZones := make(map[string]*api.ClusterZoneSpec)

upup/pkg/fi/cloudup/apply_cluster.go

@@ -388,7 +388,7 @@ func (c *ApplyClusterCmd) Run() error {
 		}
 
 		config := &nodeup.NodeUpConfig{}
-		for _, tag := range nodeUpTags {
+		for _, tag := range nodeUpTags.List() {
 			config.Tags = append(config.Tags, tag)
 		}
 

upup/pkg/fi/cloudup/loader.go

@@ -27,6 +27,7 @@ import (
 	"k8s.io/kops/upup/pkg/fi/loader"
 	"k8s.io/kops/upup/pkg/fi/utils"
 	"k8s.io/kops/util/pkg/vfs"
+	"k8s.io/kubernetes/pkg/util/sets"
 	"os"
 	"reflect"
 	"strings"
@@ -45,7 +46,7 @@ type Loader struct {
 
 	ModelStore vfs.Path
 
-	Tags              map[string]struct{}
+	Tags              sets.String
 	TemplateFunctions template.FuncMap
 
 	typeMap map[string]reflect.Type

upup/pkg/fi/cloudup/spec_builder.go

@@ -23,12 +23,13 @@ import (
 	"k8s.io/kops/upup/pkg/fi/loader"
 	"k8s.io/kops/upup/pkg/fi/utils"
 	"k8s.io/kops/util/pkg/vfs"
+	"k8s.io/kubernetes/pkg/util/sets"
 )
 
 type SpecBuilder struct {
 	OptionsLoader *loader.OptionsLoader
 
-	Tags map[string]struct{}
+	Tags sets.String
 }
 
 func (l *SpecBuilder) BuildCompleteSpec(clusterSpec *api.ClusterSpec, modelStore vfs.Path, models []string) (*api.ClusterSpec, error) {

upup/pkg/fi/cloudup/tagbuilder.go

@@ -28,77 +28,74 @@ import (
 	"github.com/golang/glog"
 	api "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
+	"k8s.io/kubernetes/pkg/util/sets"
 )
 
-//
-//
-func buildCloudupTags(cluster *api.Cluster) (map[string]struct{}, error) {
+func buildCloudupTags(cluster *api.Cluster) (sets.String, error) {
 	// TODO: Make these configurable?
 	useMasterASG := true
 	useMasterLB := false
 
-	tags := make(map[string]struct{})
+	tags := sets.NewString()
 
 	networking := cluster.Spec.Networking
-	glog.Infof("networking: %s", networking)
 
 	if networking == nil || networking.Classic != nil {
-		tags["_networking_classic"] = struct{}{}
+		tags.Insert("_networking_classic")
 	} else if networking.Kubenet != nil {
-		tags["_networking_kubenet"] = struct{}{}
+		tags.Insert("_networking_kubenet")
 	} else if networking.External != nil {
 		// external is based on kubenet
-		tags["_networking_kubenet"] = struct{}{}
-		tags["_networking_external"] = struct{}{}
+		tags.Insert("_networking_kubenet", "_networking_external")
 	} else if networking.CNI != nil || networking.Weave != nil {
-		tags["_networking_cni"] = struct{}{}
-		// TODO combine with the External
+		tags.Insert("_networking_cni")
 	} else if networking.Kopeio != nil {
+		// TODO combine with the External
 		// Kopeio is based on kubenet / external
-		tags["_networking_kubenet"] = struct{}{}
-		tags["_networking_external"] = struct{}{}
+		// TODO combine with External
+		tags.Insert("_networking_kubenet", "_networking_external")
 	} else {
 		return nil, fmt.Errorf("No networking mode set")
 	}
 
 	if useMasterASG {
-		tags["_master_asg"] = struct{}{}
+		tags.Insert("_master_asg")
 	} else {
-		tags["_master_single"] = struct{}{}
+		tags.Insert("_master_single")
 	}
 
 	if useMasterLB {
-		tags["_master_lb"] = struct{}{}
+		tags.Insert("_master_lb")
 	} else if cluster.Spec.Topology.Masters == api.TopologyPublic {
-		tags["_not_master_lb"] = struct{}{}
+		tags.Insert("_not_master_lb")
 	}
 
 	// Network Topologies
 	if cluster.Spec.Topology == nil {
 		return nil, fmt.Errorf("missing topology spec")
 	}
 	if cluster.Spec.Topology.Masters == api.TopologyPublic && cluster.Spec.Topology.Nodes == api.TopologyPublic {
-		tags["_topology_public"] = struct{}{}
+		tags.Insert("_topology_public")
 	} else if cluster.Spec.Topology.Masters == api.TopologyPrivate && cluster.Spec.Topology.Nodes == api.TopologyPrivate {
-		tags["_topology_private"] = struct{}{}
+		tags.Insert("_topology_private")
 	} else {
 		return nil, fmt.Errorf("Unable to parse topology. Unsupported topology configuration. Masters and nodes must match!")
 	}
 
 	if fi.BoolValue(cluster.Spec.IsolateMasters) {
-		tags["_isolate_masters"] = struct{}{}
+		tags.Insert("_isolate_masters")
 	}
 
 	switch cluster.Spec.CloudProvider {
 	case "gce":
 		{
 			glog.Fatalf("GCE is (probably) not working currently - please ping @justinsb for cleanup")
-			tags["_gce"] = struct{}{}
+			tags.Insert("_gce")
 		}
 
 	case "aws":
 		{
-			tags["_aws"] = struct{}{}
+			tags.Insert("_aws")
 		}
 
 	default:
@@ -124,16 +121,16 @@ func buildCloudupTags(cluster *api.Cluster) (map[string]struct{}, error) {
 	if versionTag == "" {
 		return nil, fmt.Errorf("unable to determine kubernetes version from %q", cluster.Spec.KubernetesVersion)
 	} else {
-		tags[versionTag] = struct{}{}
+		tags.Insert(versionTag)
 	}
 
-	glog.Infof("tags: %s", tags)
+	glog.V(4).Infof("tags: %s", tags.List())
 
 	return tags, nil
 }
 
-func buildNodeupTags(role api.InstanceGroupRole, cluster *api.Cluster, clusterTags map[string]struct{}) ([]string, error) {
-	var tags []string
+func buildNodeupTags(role api.InstanceGroupRole, cluster *api.Cluster, clusterTags sets.String) (sets.String, error) {
+	tags := sets.NewString()
 
 	networking := cluster.Spec.Networking
 
@@ -143,52 +140,49 @@ func buildNodeupTags(role api.InstanceGroupRole, cluster *api.Cluster, clusterTa
 
 	if networking.CNI != nil || networking.Weave != nil {
 		// external is based on cni, weave, flannel, etc
-		tags = append(tags, "_networking_cni")
+		tags.Insert("_networking_cni")
 	}
 
 	switch role {
 	case api.InstanceGroupRoleNode:
-		tags = append(tags, "_kubernetes_pool")
+		tags.Insert("_kubernetes_pool")
 
 		// TODO: Should we run _protokube on the nodes?
-		tags = append(tags, "_protokube")
+		tags.Insert("_protokube")
 
 	case api.InstanceGroupRoleMaster:
-		tags = append(tags, "_kubernetes_master")
+		tags.Insert("_kubernetes_master")
 
 		if !fi.BoolValue(cluster.Spec.IsolateMasters) {
 			// Run this master as a pool node also (start kube-proxy etc)
-			tags = append(tags, "_kubernetes_pool")
+			tags.Insert("_kubernetes_pool")
 		}
 
-		tags = append(tags, "_protokube")
+		tags.Insert("_protokube")
 	default:
 		return nil, fmt.Errorf("Unrecognized role: %v", role)
 	}
 
 	// TODO: Replace with list of CNI plugins ?
 	if usesCNI(cluster) {
-		tags = append(tags, "_cni_bridge")
-		tags = append(tags, "_cni_host_local")
-		tags = append(tags, "_cni_loopback")
-		tags = append(tags, "_cni_ptp")
-		//tags = append(tags, "_cni_tuning")
+		tags.Insert("_cni_bridge", "_cni_host_local", "_cni_loopback", "_cni_ptp")
+		//tags.Insert("_cni_tuning")
 	}
 
 	switch fi.StringValue(cluster.Spec.UpdatePolicy) {
 	case "": // default
-		tags = append(tags, "_automatic_upgrades")
+		tags.Insert("_automatic_upgrades")
 	case api.UpdatePolicyExternal:
 	// Skip applying the tag
 	default:
 		glog.Warningf("Unrecognized value for UpdatePolicy: %v", fi.StringValue(cluster.Spec.UpdatePolicy))
 	}
 
-	if _, found := clusterTags["_gce"]; found {
-		tags = append(tags, "_gce")
+	if clusterTags.Has("_gce") {
+		tags.Insert("_gce")
 	}
-	if _, found := clusterTags["_aws"]; found {
-		tags = append(tags, "_aws")
+	if clusterTags.Has("_aws") {
+		tags.Insert("_aws")
 	}
 
 	return tags, nil

upup/pkg/fi/cloudup/tagbuilder_test.go

@@ -74,15 +74,15 @@ func TestBuildTags_CloudProvider_AWS_Weave(t *testing.T) {
 		t.Fatalf("buildCloudupTags error: %v", err)
 	}
 
-	if _, found := tags["_aws"]; !found {
+	if !tags.Has("_aws") {
 		t.Fatal("tag _aws not found")
 	}
 
-	if _, found := tags["_networking_cni"]; !found {
+	if !tags.Has("_networking_cni") {
 		t.Fatal("tag _networking_cni not found")
 	}
 
-	if _, found := tags["_networking_kubenet"]; found {
+	if tags.Has("_networking_kubenet") {
 		t.Fatal("tag _networking_kubenet found")
 	}
 
@@ -91,7 +91,7 @@ func TestBuildTags_CloudProvider_AWS_Weave(t *testing.T) {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
 
-	if !stringSliceContains(nodeUpTags, "_aws") {
+	if !nodeUpTags.Has("_aws") {
 		t.Fatal("nodeUpTag _aws not found")
 	}
 }
@@ -105,11 +105,11 @@ func TestBuildTags_CloudProvider_AWS(t *testing.T) {
 		t.Fatalf("buildCloudupTags error: %v", err)
 	}
 
-	if _, found := tags["_aws"]; !found {
+	if !tags.Has("_aws") {
 		t.Fatal("tag _aws not found")
 	}
 
-	if _, found := tags["_networking_cni"]; !found {
+	if !tags.Has("_networking_cni") {
 		t.Fatal("tag _networking_cni not found")
 	}
 
@@ -118,7 +118,7 @@ func TestBuildTags_CloudProvider_AWS(t *testing.T) {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
 
-	if !stringSliceContains(nodeUpTags, "_aws") {
+	if !nodeUpTags.Has("_aws") {
 		t.Fatal("nodeUpTag _aws not found")
 	}
 }
@@ -138,7 +138,7 @@ func TestBuildTags_KubernetesVersions(t *testing.T) {
 			t.Fatalf("buildCloudupTags error: %v", err)
 		}
 
-		if _, found := tags[tag]; !found {
+		if !tags.Has(tag) {
 			t.Fatalf("tag %q not found for %q: %v", tag, version, tags)
 		}
 	}
@@ -157,7 +157,7 @@ func TestBuildTags_UpdatePolicy_Nil(t *testing.T) {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
 
-	if !stringSliceContains(nodeUpTags, "_automatic_upgrades") {
+	if !nodeUpTags.Has("_automatic_upgrades") {
 		t.Fatal("nodeUpTag _automatic_upgrades not found")
 	}
 }
@@ -175,16 +175,7 @@ func TestBuildTags_UpdatePolicy_None(t *testing.T) {
 		t.Fatalf("buildNodeupTags error: %v", err)
 	}
 
-	if stringSliceContains(nodeUpTags, "_automatic_upgrades") {
+	if nodeUpTags.Has("_automatic_upgrades") {
 		t.Fatal("nodeUpTag _automatic_upgrades found unexpectedly")
 	}
 }
-
-func stringSliceContains(haystack []string, needle string) bool {
-	for _, s := range haystack {
-		if needle == s {
-			return true
-		}
-	}
-	return false
-}

upup/pkg/fi/cloudup/template_functions.go

@@ -35,6 +35,7 @@ import (
 	api "k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/util/pkg/vfs"
+	"k8s.io/kubernetes/pkg/util/sets"
 	"math/big"
 	"net"
 	"sort"
@@ -46,7 +47,7 @@ type TemplateFunctions struct {
 	cluster        *api.Cluster
 	instanceGroups []*api.InstanceGroup
 
-	tags   map[string]struct{}
+	tags   sets.String
 	region string
 }
 

upup/pkg/fi/loader/tree_walker.go

@@ -20,6 +20,7 @@ import (
 	"fmt"
 	"github.com/golang/glog"
 	"k8s.io/kops/util/pkg/vfs"
+	"k8s.io/kubernetes/pkg/util/sets"
 	"os"
 	"path"
 	"strings"
@@ -29,7 +30,7 @@ type TreeWalker struct {
 	Contexts       map[string]Handler
 	Extensions     map[string]Handler
 	DefaultHandler Handler
-	Tags           map[string]struct{}
+	Tags           sets.String
 }
 
 type TreeWalkItem struct {

upup/pkg/fi/nodeup/cloudinit/cloud_init_target.go

@@ -23,14 +23,15 @@ import (
 	"io"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/utils"
+	"k8s.io/kubernetes/pkg/util/sets"
 	"os"
 	"path"
 )
 
 type CloudInitTarget struct {
 	Config *CloudConfig
 	out    io.Writer
-	Tags   map[string]struct{}
+	Tags   sets.String
 }
 
 type AddBehaviour int
@@ -40,7 +41,7 @@ const (
 	Once
 )
 
-func NewCloudInitTarget(out io.Writer, tags map[string]struct{}) *CloudInitTarget {
+func NewCloudInitTarget(out io.Writer, tags sets.String) *CloudInitTarget {
 	t := &CloudInitTarget{
 		Config: &CloudConfig{},
 		out:    out,