Refactor more cluster creation code into NewCluster()

cmd/kops/create_cluster.go

@@ -59,13 +59,8 @@ import (
 	"k8s.io/kubectl/pkg/util/templates"
 )
 
-const (
-	AuthorizationFlagAlwaysAllow = "AlwaysAllow"
-	AuthorizationFlagRBAC        = "RBAC"
-)
-
 type CreateClusterOptions struct {
-	ClusterName          string
+	cloudup.NewClusterOptions
 	Yes                  bool
 	Target               string
 	Cloud                string
@@ -105,15 +100,9 @@ type CreateClusterOptions struct {
 	// Overrides allows settings values direct in the spec
 	Overrides []string
 
-	// Channel is the location of the api.Channel to use for our defaults
-	Channel string
-
 	// The network topology to use
 	Topology string
 
-	// The authorization approach to use (RBAC, AlwaysAllow)
-	Authorization string
-
 	// The DNS type to use (public/private)
 	DNSType string
 
@@ -156,9 +145,6 @@ type CreateClusterOptions struct {
 	// GCEServiceAccount specifies the service account with which the GCE VM runs
 	GCEServiceAccount string
 
-	// ConfigBase is the location where we will store the configuration, it defaults to the state store
-	ConfigBase string
-
 	// DryRun mode output a cluster manifest of Output type.
 	DryRun bool
 	// Output type during a DryRun
@@ -166,10 +152,11 @@ type CreateClusterOptions struct {
 }
 
 func (o *CreateClusterOptions) InitDefaults() {
+	o.NewClusterOptions.InitDefaults()
+
 	o.Yes = false
 	o.Target = cloudup.TargetDirect
 	o.Networking = "kubenet"
-	o.Channel = api.DefaultChannel
 	o.Topology = api.TopologyPublic
 	o.DNSType = string(api.DNSTypePublic)
 	o.Bastion = false
@@ -177,8 +164,6 @@ func (o *CreateClusterOptions) InitDefaults() {
 	// Default to open API & SSH access
 	o.AdminAccess = []string{"0.0.0.0/0"}
 
-	o.Authorization = AuthorizationFlagRBAC
-
 	o.ContainerRuntime = "docker"
 }
 
@@ -340,7 +325,7 @@ func NewCmdCreateCluster(f *util.Factory, out io.Writer) *cobra.Command {
 	cmd.Flags().StringVarP(&options.Topology, "topology", "t", options.Topology, "Controls network topology for the cluster: public|private.")
 
 	// Authorization
-	cmd.Flags().StringVar(&options.Authorization, "authorization", options.Authorization, "Authorization mode to use: "+AuthorizationFlagAlwaysAllow+" or "+AuthorizationFlagRBAC)
+	cmd.Flags().StringVar(&options.Authorization, "authorization", options.Authorization, "Authorization mode to use: "+cloudup.AuthorizationFlagAlwaysAllow+" or "+cloudup.AuthorizationFlagRBAC)
 
 	// DNS
 	cmd.Flags().StringVar(&options.DNSType, "dns", options.DNSType, "DNS hosted zone to use: public|private.")
@@ -426,16 +411,11 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 		return err
 	}
 
-	options := &cloudup.NewClusterOptions{
-		Name:    c.ClusterName,
-		Channel: c.Channel,
-	}
-
-	if options.Name == "" {
+	if c.ClusterName == "" {
 		return fmt.Errorf("--name is required")
 	}
 
-	cluster, err := clientset.GetCluster(ctx, options.Name)
+	cluster, err := clientset.GetCluster(ctx, c.ClusterName)
 	if err != nil {
 		if apierrors.IsNotFound(err) {
 			cluster = nil
@@ -445,10 +425,10 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 	}
 
 	if cluster != nil {
-		return fmt.Errorf("cluster %q already exists; use 'kops update cluster' to apply changes", options.Name)
+		return fmt.Errorf("cluster %q already exists; use 'kops update cluster' to apply changes", c.ClusterName)
 	}
 
-	clusterResult, err := cloudup.NewCluster(options)
+	clusterResult, err := cloudup.NewCluster(&c.NewClusterOptions, clientset)
 	if err != nil {
 		return err
 	}
@@ -456,23 +436,6 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 	// TODO: push more of the following logic into cloudup.NewCluster()
 	cluster = clusterResult.Cluster
 
-	cluster.Spec.ConfigBase = c.ConfigBase
-	configBase, err := clientset.ConfigBaseFor(cluster)
-	if err != nil {
-		return fmt.Errorf("error building ConfigBase for cluster: %v", err)
-	}
-	cluster.Spec.ConfigBase = configBase.Path()
-
-	// In future we could change the default if the flag is not specified, e.g. in 1.7 maybe the default is RBAC?
-	cluster.Spec.Authorization = &api.AuthorizationSpec{}
-	if strings.EqualFold(c.Authorization, AuthorizationFlagAlwaysAllow) {
-		cluster.Spec.Authorization.AlwaysAllow = &api.AlwaysAllowAuthorizationSpec{}
-	} else if strings.EqualFold(c.Authorization, AuthorizationFlagRBAC) {
-		cluster.Spec.Authorization.RBAC = &api.RBACAuthorizationSpec{}
-	} else {
-		return fmt.Errorf("unknown authorization mode %q", c.Authorization)
-	}
-
 	if c.Cloud != "" {
 		cluster.Spec.CloudProvider = c.Cloud
 	}
@@ -1323,6 +1286,10 @@ func RunCreateCluster(ctx context.Context, f *util.Factory, out io.Writer, c *Cr
 		return fmt.Errorf("error writing updated configuration: %v", err)
 	}
 
+	configBase, err := clientset.ConfigBaseFor(cluster)
+	if err != nil {
+		return fmt.Errorf("error building ConfigBase for cluster: %v", err)
+	}
 	err = registry.WriteConfigDeprecated(cluster, configBase.Join(registry.PathClusterCompleted), fullCluster)
 	if err != nil {
 		return fmt.Errorf("error writing completed cluster spec: %v", err)

upup/pkg/fi/cloudup/new_cluster.go

@@ -18,18 +18,34 @@ package cloudup
 
 import (
 	"fmt"
+	"strings"
 
 	"k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/kops"
 	api "k8s.io/kops/pkg/apis/kops"
+	"k8s.io/kops/pkg/client/simple"
+)
+
+const (
+	AuthorizationFlagAlwaysAllow = "AlwaysAllow"
+	AuthorizationFlagRBAC        = "RBAC"
 )
 
 type NewClusterOptions struct {
-	// Name is the name of the cluster to initialize.
-	Name string
+	// ClusterName is the name of the cluster to initialize.
+	ClusterName string
 
-	// Channel is a channel location for initializing the cluster.
+	// Authorization is the authorization mode to use. The options are "RBAC" (default) and "AlwaysAllow".
+	Authorization string
+	// Channel is a channel location for initializing the cluster. It defaults to "stable".
 	Channel string
+	// ConfigBase is the location where we will store the configuration. It defaults to the state store.
+	ConfigBase string
+}
+
+func (o *NewClusterOptions) InitDefaults() {
+	o.Channel = api.DefaultChannel
+	o.Authorization = AuthorizationFlagRBAC
 }
 
 type NewClusterResult struct {
@@ -42,11 +58,14 @@ type NewClusterResult struct {
 
 // NewCluster initializes cluster and instance groups specifications as
 // intended for newly created clusters.
-func NewCluster(opt *NewClusterOptions) (*NewClusterResult, error) {
-	if opt.Name == "" {
+func NewCluster(opt *NewClusterOptions, clientset simple.Clientset) (*NewClusterResult, error) {
+	if opt.ClusterName == "" {
 		return nil, fmt.Errorf("name is required")
 	}
 
+	if opt.Channel == "" {
+		opt.Channel = api.DefaultChannel
+	}
 	channel, err := api.LoadChannel(opt.Channel)
 	if err != nil {
 		return nil, err
@@ -54,7 +73,7 @@ func NewCluster(opt *NewClusterOptions) (*NewClusterResult, error) {
 
 	cluster := api.Cluster{
 		ObjectMeta: v1.ObjectMeta{
-			Name: opt.Name,
+			Name: opt.ClusterName,
 		},
 	}
 
@@ -68,6 +87,22 @@ func NewCluster(opt *NewClusterOptions) (*NewClusterResult, error) {
 	}
 	cluster.Spec.Channel = opt.Channel
 
+	cluster.Spec.ConfigBase = opt.ConfigBase
+	configBase, err := clientset.ConfigBaseFor(&cluster)
+	if err != nil {
+		return nil, fmt.Errorf("error building ConfigBase for cluster: %v", err)
+	}
+	cluster.Spec.ConfigBase = configBase.Path()
+
+	cluster.Spec.Authorization = &api.AuthorizationSpec{}
+	if strings.EqualFold(opt.Authorization, AuthorizationFlagAlwaysAllow) {
+		cluster.Spec.Authorization.AlwaysAllow = &api.AlwaysAllowAuthorizationSpec{}
+	} else if opt.Authorization == "" || strings.EqualFold(opt.Authorization, AuthorizationFlagRBAC) {
+		cluster.Spec.Authorization.RBAC = &api.RBACAuthorizationSpec{}
+	} else {
+		return nil, fmt.Errorf("unknown authorization mode %q", opt.Authorization)
+	}
+
 	result := NewClusterResult{
 		Cluster: &cluster,
 		Channel: channel,