Move topology setup to pkg

cmd/kops/BUILD.bazel

@@ -84,7 +84,6 @@ go_library(
         "//upup/pkg/fi:go_default_library",
         "//upup/pkg/fi/cloudup:go_default_library",
         "//upup/pkg/fi/cloudup/awsup:go_default_library",
-        "//upup/pkg/fi/cloudup/openstack:go_default_library",
         "//upup/pkg/fi/utils:go_default_library",
         "//upup/pkg/kutil:go_default_library",
         "//util/pkg/tables:go_default_library",

cmd/kops/create_cluster.go

@@ -40,8 +40,6 @@ import (
 	"k8s.io/kops/pkg/featureflag"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/cloudup"
-	"k8s.io/kops/upup/pkg/fi/cloudup/awsup"
-	"k8s.io/kops/upup/pkg/fi/cloudup/openstack"
 	"k8s.io/kops/upup/pkg/fi/utils"
 	"k8s.io/kubectl/pkg/util/i18n"
 	"k8s.io/kubectl/pkg/util/templates"
@@ -60,7 +58,6 @@ type CreateClusterOptions struct {
 	Image                string
 	NodeImage            string
 	MasterImage          string
-	UtilitySubnetIDs     []string
 	DisableSubnetTags    bool
 	NetworkCIDR          string
 	DNSZone              string
@@ -76,15 +73,6 @@ type CreateClusterOptions struct {
 	// Overrides allows settings values direct in the spec
 	Overrides []string
 
-	// The network topology to use
-	Topology string
-
-	// The DNS type to use (public/private)
-	DNSType string
-
-	// Enable/Disable Bastion Host complete setup
-	Bastion bool
-
 	// Specify tags for AWS instance groups
 	CloudLabels string
 
@@ -114,9 +102,6 @@ func (o *CreateClusterOptions) InitDefaults() {
 
 	o.Yes = false
 	o.Target = cloudup.TargetDirect
-	o.Topology = api.TopologyPublic
-	o.DNSType = string(api.DNSTypePublic)
-	o.Bastion = false
 
 	// Default to open API & SSH access
 	o.AdminAccess = []string{"0.0.0.0/0"}
@@ -492,10 +477,6 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 		cluster.Spec.DNSZone = c.DNSZone
 	}
 
-	// TODO: push more of the following logic into cloudup.NewCluster()
-	channel := clusterResult.Channel
-	allZones := clusterResult.AllZones
-
 	if c.ContainerRuntime != "" {
 		cluster.Spec.ContainerRuntime = c.ContainerRuntime
 	}
@@ -504,123 +485,15 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 		cluster.Spec.NetworkCIDR = c.NetworkCIDR
 	}
 
-	// Network Topology
-	if c.Topology == "" {
-		// The flag default should have set this, but we might be being called as a library
-		klog.Infof("Empty topology. Defaulting to public topology")
-		c.Topology = api.TopologyPublic
-	}
-
 	cluster.Spec.DisableSubnetTags = c.DisableSubnetTags
 
-	switch c.Topology {
-	case api.TopologyPublic:
-		cluster.Spec.Topology = &api.TopologySpec{
-			Masters: api.TopologyPublic,
-			Nodes:   api.TopologyPublic,
-			//Bastion: &api.BastionSpec{Enable: c.Bastion},
-		}
-
-		if c.Bastion {
-			return fmt.Errorf("Bastion supports --topology='private' only.")
-		}
-
-		for i := range cluster.Spec.Subnets {
-			cluster.Spec.Subnets[i].Type = api.SubnetTypePublic
-		}
-
-	case api.TopologyPrivate:
-		if cluster.Spec.Networking.Kubenet != nil {
-			return fmt.Errorf("invalid networking option %s. Kubenet does not support private topology", c.Networking)
-		}
-		cluster.Spec.Topology = &api.TopologySpec{
-			Masters: api.TopologyPrivate,
-			Nodes:   api.TopologyPrivate,
-		}
-
-		for i := range cluster.Spec.Subnets {
-			cluster.Spec.Subnets[i].Type = api.SubnetTypePrivate
-		}
-
-		var utilitySubnets []api.ClusterSubnetSpec
-
-		var zoneToSubnetProviderID map[string]string
-		if len(c.Zones) > 0 && len(c.UtilitySubnetIDs) > 0 {
-			if api.CloudProviderID(cluster.Spec.CloudProvider) == api.CloudProviderAWS {
-				zoneToSubnetProviderID, err = getZoneToSubnetProviderID(cluster.Spec.NetworkID, c.Zones[0][:len(c.Zones[0])-1], c.UtilitySubnetIDs)
-				if err != nil {
-					return err
-				}
-			} else if api.CloudProviderID(cluster.Spec.CloudProvider) == api.CloudProviderOpenstack {
-				tags := make(map[string]string)
-				tags[openstack.TagClusterName] = c.ClusterName
-				zoneToSubnetProviderID, err = getSubnetProviderID(&cluster.Spec, allZones.List(), c.UtilitySubnetIDs, tags)
-				if err != nil {
-					return err
-				}
-			}
-		}
-
-		for _, s := range cluster.Spec.Subnets {
-			if s.Type == api.SubnetTypeUtility {
-				continue
-			}
-			subnet := api.ClusterSubnetSpec{
-				Name: "utility-" + s.Name,
-				Zone: s.Zone,
-				Type: api.SubnetTypeUtility,
-			}
-			if subnetID, ok := zoneToSubnetProviderID[s.Zone]; ok {
-				subnet.ProviderID = subnetID
-			}
-			utilitySubnets = append(utilitySubnets, subnet)
-		}
-		cluster.Spec.Subnets = append(cluster.Spec.Subnets, utilitySubnets...)
-
-		if c.Bastion {
-			bastionGroup := &api.InstanceGroup{}
-			bastionGroup.Spec.Role = api.InstanceGroupRoleBastion
-			bastionGroup.ObjectMeta.Name = "bastions"
-			bastionGroup.Spec.Image = c.Image
-			instanceGroups = append(instanceGroups, bastionGroup)
-
-			if !dns.IsGossipHostname(cluster.Name) {
-				cluster.Spec.Topology.Bastion = &api.BastionSpec{
-					BastionPublicName: "bastion." + cluster.Name,
-				}
-			}
-		}
-
-	default:
-		return fmt.Errorf("Invalid topology %s.", c.Topology)
-	}
-
-	// DNS
-	if c.DNSType == "" {
-		// The flag default should have set this, but we might be being called as a library
-		klog.Infof("Empty DNS. Defaulting to public DNS")
-		c.DNSType = string(api.DNSTypePublic)
-	}
-
-	if cluster.Spec.Topology == nil {
-		cluster.Spec.Topology = &api.TopologySpec{}
-	}
-	if cluster.Spec.Topology.DNS == nil {
-		cluster.Spec.Topology.DNS = &api.DNSSpec{}
-	}
-	switch strings.ToLower(c.DNSType) {
-	case "public":
-		cluster.Spec.Topology.DNS.Type = api.DNSTypePublic
-	case "private":
-		cluster.Spec.Topology.DNS.Type = api.DNSTypePrivate
-	default:
-		return fmt.Errorf("unknown DNSType: %q", c.DNSType)
-	}
-
 	if c.MasterPublicName != "" {
 		cluster.Spec.MasterPublicName = c.MasterPublicName
 	}
 
+	// TODO: push more of the following logic into cloudup.NewCluster()
+	channel := clusterResult.Channel
+
 	// check if we should set anonymousAuth to false
 	{
 		if cluster.Spec.Kubelet == nil {
@@ -919,74 +792,6 @@ func initializeOpenstackAPI(c *CreateClusterOptions, cluster *api.Cluster) {
 	}
 }
 
-// TODO dedup or remove
-func getZoneToSubnetProviderID(VPCID string, region string, subnetIDs []string) (map[string]string, error) {
-	res := make(map[string]string)
-	cloudTags := map[string]string{}
-	awsCloud, err := awsup.NewAWSCloud(region, cloudTags)
-	if err != nil {
-		return res, fmt.Errorf("error loading cloud: %v", err)
-	}
-	vpcInfo, err := awsCloud.FindVPCInfo(VPCID)
-	if err != nil {
-		return res, fmt.Errorf("error describing VPC: %v", err)
-	}
-	if vpcInfo == nil {
-		return res, fmt.Errorf("VPC %q not found", VPCID)
-	}
-	subnetByID := make(map[string]*fi.SubnetInfo)
-	for _, subnetInfo := range vpcInfo.Subnets {
-		subnetByID[subnetInfo.ID] = subnetInfo
-	}
-	for _, subnetID := range subnetIDs {
-		subnet, ok := subnetByID[subnetID]
-		if !ok {
-			return res, fmt.Errorf("subnet %s not found in VPC %s", subnetID, VPCID)
-		}
-		if res[subnet.Zone] != "" {
-			return res, fmt.Errorf("subnet %s and %s have the same zone", subnetID, res[subnet.Zone])
-		}
-		res[subnet.Zone] = subnetID
-	}
-	return res, nil
-}
-
-// TODO dedup or remove
-func getSubnetProviderID(spec *api.ClusterSpec, zones []string, subnetIDs []string, tags map[string]string) (map[string]string, error) {
-	res := make(map[string]string)
-	osCloud, err := openstack.NewOpenstackCloud(tags, spec)
-	if err != nil {
-		return res, fmt.Errorf("error loading cloud: %v", err)
-	}
-	osCloud.UseZones(zones)
-
-	networkInfo, err := osCloud.FindVPCInfo(spec.NetworkID)
-	if err != nil {
-		return res, fmt.Errorf("error describing Network: %v", err)
-	}
-	if networkInfo == nil {
-		return res, fmt.Errorf("network %q not found", spec.NetworkID)
-	}
-
-	subnetByID := make(map[string]*fi.SubnetInfo)
-	for _, subnetInfo := range networkInfo.Subnets {
-		subnetByID[subnetInfo.ID] = subnetInfo
-	}
-
-	for _, subnetID := range subnetIDs {
-		subnet, ok := subnetByID[subnetID]
-		if !ok {
-			return res, fmt.Errorf("subnet %s not found in network %s", subnetID, spec.NetworkID)
-		}
-
-		if res[subnet.Zone] != "" {
-			return res, fmt.Errorf("subnet %s and %s have the same zone", subnetID, res[subnet.Zone])
-		}
-		res[subnet.Zone] = subnetID
-	}
-	return res, nil
-}
-
 func loadSSHPublicKeys(sshPublicKey string) (map[string][]byte, error) {
 	sshPublicKeys := make(map[string][]byte)
 	if sshPublicKey != "" {

upup/pkg/fi/cloudup/new_cluster.go

@@ -35,6 +35,7 @@ import (
 	version "k8s.io/kops/pkg/apis/kops/util"
 	"k8s.io/kops/pkg/apis/kops/validation"
 	"k8s.io/kops/pkg/client/simple"
+	"k8s.io/kops/pkg/dns"
 	"k8s.io/kops/pkg/featureflag"
 	"k8s.io/kops/pkg/model/components"
 	"k8s.io/kops/upup/pkg/fi"
@@ -85,6 +86,8 @@ type NewClusterOptions struct {
 	// SubnetIDs are the IDs of the shared subnets.
 	// If empty, creates new subnets to be owned by the cluster.
 	SubnetIDs []string
+	// UtilitySubnetIDs are the IDs of the shared utility subnets. If empty and the topology is "private", creates new subnets to be owned by the cluster.
+	UtilitySubnetIDs []string
 	// Egress defines the method of traffic egress for subnets.
 	Egress string
 
@@ -108,15 +111,23 @@ type NewClusterOptions struct {
 	// NodeCount is the number of nodes to create. Defaults to leaving the count unspecified
 	// on the InstanceGroup, which results in a count of 2.
 	NodeCount int32
+	// Bastion enables the creation of a Bastion instance.
+	Bastion bool
 
 	// Networking is the networking provider/node to use.
 	Networking string
+	// Topology is the network topology to use. Defaults to "public".
+	Topology string
+	// DNSType is the DNS type to use; "public" or "private". Defaults to "public".
+	DNSType string
 }
 
 func (o *NewClusterOptions) InitDefaults() {
 	o.Channel = api.DefaultChannel
 	o.Authorization = AuthorizationFlagRBAC
 	o.Networking = "kubenet"
+	o.Topology = api.TopologyPublic
+	o.DNSType = string(api.DNSTypePublic)
 }
 
 type NewClusterResult struct {
@@ -126,8 +137,7 @@ type NewClusterResult struct {
 	InstanceGroups []*api.InstanceGroup
 
 	// TODO remove after more create_cluster logic refactored in
-	Channel  *api.Channel
+	Channel *api.Channel
-	AllZones sets.String
 }
 
 // NewCluster initializes cluster and instance groups specifications as
@@ -222,19 +232,24 @@ func NewCluster(opt *NewClusterOptions, clientset simple.Clientset) (*NewCluster
 		return nil, err
 	}
 
-	instanceGroups := append([]*api.InstanceGroup(nil), masters...)
-	instanceGroups = append(instanceGroups, nodes...)
-
 	err = setupNetworking(opt, &cluster)
 	if err != nil {
 		return nil, err
 	}
 
+	bastions, err := setupTopology(opt, &cluster, allZones)
+	if err != nil {
+		return nil, err
+	}
+
+	instanceGroups := append([]*api.InstanceGroup(nil), masters...)
+	instanceGroups = append(instanceGroups, nodes...)
+	instanceGroups = append(instanceGroups, bastions...)
+
 	result := NewClusterResult{
 		Cluster:        &cluster,
 		InstanceGroups: instanceGroups,
 		Channel:        channel,
-		AllZones:       allZones,
 	}
 	return &result, nil
 }
@@ -309,7 +324,7 @@ func setupVPC(opt *NewClusterOptions, cluster *api.Cluster) error {
 
 		if cluster.Spec.NetworkID == "" && len(opt.SubnetIDs) > 0 {
 			tags := make(map[string]string)
-			tags[openstack.TagClusterName] = opt.ClusterName
+			tags[openstack.TagClusterName] = cluster.Name
 			osCloud, err := openstack.NewOpenstackCloud(tags, &cluster.Spec)
 			if err != nil {
 				return fmt.Errorf("error loading cloud: %v", err)
@@ -413,7 +428,7 @@ func setupZones(opt *NewClusterOptions, cluster *api.Cluster, allZones sets.Stri
 
 	case api.CloudProviderAWS:
 		if len(opt.Zones) > 0 && len(opt.SubnetIDs) > 0 {
-			zoneToSubnetProviderID, err = getZoneToSubnetProviderID(cluster.Spec.NetworkID, opt.Zones[0][:len(opt.Zones[0])-1], opt.SubnetIDs)
+			zoneToSubnetProviderID, err = getAWSZoneToSubnetProviderID(cluster.Spec.NetworkID, opt.Zones[0][:len(opt.Zones[0])-1], opt.SubnetIDs)
 			if err != nil {
 				return nil, err
 			}
@@ -422,8 +437,8 @@ func setupZones(opt *NewClusterOptions, cluster *api.Cluster, allZones sets.Stri
 	case api.CloudProviderOpenstack:
 		if len(opt.Zones) > 0 && len(opt.SubnetIDs) > 0 {
 			tags := make(map[string]string)
-			tags[openstack.TagClusterName] = opt.ClusterName
+			tags[openstack.TagClusterName] = cluster.Name
-			zoneToSubnetProviderID, err = getSubnetProviderID(&cluster.Spec, allZones.List(), opt.SubnetIDs, tags)
+			zoneToSubnetProviderID, err = getOpenstackZoneToSubnetProviderID(&cluster.Spec, allZones.List(), opt.SubnetIDs, tags)
 			if err != nil {
 				return nil, err
 			}
@@ -452,8 +467,7 @@ func setupZones(opt *NewClusterOptions, cluster *api.Cluster, allZones sets.Stri
 	return zoneToSubnetMap, nil
 }
 
-// TODO rename to getAWSZoneToSubnetProviderID
+func getAWSZoneToSubnetProviderID(VPCID string, region string, subnetIDs []string) (map[string]string, error) {
-func getZoneToSubnetProviderID(VPCID string, region string, subnetIDs []string) (map[string]string, error) {
 	res := make(map[string]string)
 	cloudTags := map[string]string{}
 	awsCloud, err := awsup.NewAWSCloud(region, cloudTags)
@@ -484,8 +498,7 @@ func getZoneToSubnetProviderID(VPCID string, region string, subnetIDs []string)
 	return res, nil
 }
 
-// TODO rename to getOpenstackZoneToSubnetProviderID
+func getOpenstackZoneToSubnetProviderID(spec *api.ClusterSpec, zones []string, subnetIDs []string, tags map[string]string) (map[string]string, error) {
-func getSubnetProviderID(spec *api.ClusterSpec, zones []string, subnetIDs []string, tags map[string]string) (map[string]string, error) {
 	res := make(map[string]string)
 	osCloud, err := openstack.NewOpenstackCloud(tags, spec)
 	if err != nil {
@@ -716,7 +729,7 @@ func setupNodes(opt *NewClusterOptions, cluster *api.Cluster, zoneToSubnetMap ma
 func setupNetworking(opt *NewClusterOptions, cluster *api.Cluster) error {
 	cluster.Spec.Networking = &api.NetworkingSpec{}
 	switch opt.Networking {
-	case "kubenet":
+	case "kubenet", "":
 		cluster.Spec.Networking.Kubenet = &api.KubenetNetworkingSpec{}
 	case "external":
 		cluster.Spec.Networking.External = &api.ExternalNetworkingSpec{}
@@ -799,3 +812,102 @@ func setupNetworking(opt *NewClusterOptions, cluster *api.Cluster) error {
 
 	return nil
 }
+
+func setupTopology(opt *NewClusterOptions, cluster *api.Cluster, allZones sets.String) ([]*api.InstanceGroup, error) {
+	var bastions []*api.InstanceGroup
+
+	switch opt.Topology {
+	case api.TopologyPublic, "":
+		cluster.Spec.Topology = &api.TopologySpec{
+			Masters: api.TopologyPublic,
+			Nodes:   api.TopologyPublic,
+			//Bastion: &api.BastionSpec{Enable: c.Bastion},
+		}
+
+		if opt.Bastion {
+			return nil, fmt.Errorf("bastion supports --topology='private' only")
+		}
+
+		for i := range cluster.Spec.Subnets {
+			cluster.Spec.Subnets[i].Type = api.SubnetTypePublic
+		}
+
+	case api.TopologyPrivate:
+		if cluster.Spec.Networking.Kubenet != nil {
+			return nil, fmt.Errorf("invalid networking option %s. Kubenet does not support private topology", opt.Networking)
+		}
+		cluster.Spec.Topology = &api.TopologySpec{
+			Masters: api.TopologyPrivate,
+			Nodes:   api.TopologyPrivate,
+		}
+
+		for i := range cluster.Spec.Subnets {
+			cluster.Spec.Subnets[i].Type = api.SubnetTypePrivate
+		}
+
+		var utilitySubnets []api.ClusterSubnetSpec
+
+		var zoneToSubnetProviderID map[string]string
+		var err error
+		if len(opt.Zones) > 0 && len(opt.UtilitySubnetIDs) > 0 {
+			switch api.CloudProviderID(cluster.Spec.CloudProvider) {
+			case api.CloudProviderAWS:
+				zoneToSubnetProviderID, err = getAWSZoneToSubnetProviderID(cluster.Spec.NetworkID, opt.Zones[0][:len(opt.Zones[0])-1], opt.UtilitySubnetIDs)
+				if err != nil {
+					return nil, err
+				}
+			case api.CloudProviderOpenstack:
+				tags := make(map[string]string)
+				tags[openstack.TagClusterName] = cluster.Name
+				zoneToSubnetProviderID, err = getOpenstackZoneToSubnetProviderID(&cluster.Spec, allZones.List(), opt.UtilitySubnetIDs, tags)
+				if err != nil {
+					return nil, err
+				}
+			}
+		}
+
+		for _, s := range cluster.Spec.Subnets {
+			if s.Type == api.SubnetTypeUtility {
+				continue
+			}
+			subnet := api.ClusterSubnetSpec{
+				Name: "utility-" + s.Name,
+				Zone: s.Zone,
+				Type: api.SubnetTypeUtility,
+			}
+			if subnetID, ok := zoneToSubnetProviderID[s.Zone]; ok {
+				subnet.ProviderID = subnetID
+			}
+			utilitySubnets = append(utilitySubnets, subnet)
+		}
+		cluster.Spec.Subnets = append(cluster.Spec.Subnets, utilitySubnets...)
+
+		if opt.Bastion {
+			bastionGroup := &api.InstanceGroup{}
+			bastionGroup.Spec.Role = api.InstanceGroupRoleBastion
+			bastionGroup.ObjectMeta.Name = "bastions"
+			bastions = append(bastions, bastionGroup)
+
+			if !dns.IsGossipHostname(cluster.Name) {
+				cluster.Spec.Topology.Bastion = &api.BastionSpec{
+					BastionPublicName: "bastion." + cluster.Name,
+				}
+			}
+		}
+
+	default:
+		return nil, fmt.Errorf("invalid topology %s", opt.Topology)
+	}
+
+	cluster.Spec.Topology.DNS = &api.DNSSpec{}
+	switch strings.ToLower(opt.DNSType) {
+	case "public", "":
+		cluster.Spec.Topology.DNS.Type = api.DNSTypePublic
+	case "private":
+		cluster.Spec.Topology.DNS.Type = api.DNSTypePrivate
+	default:
+		return nil, fmt.Errorf("unknown DNSType: %q", opt.DNSType)
+	}
+
+	return bastions, nil
+}