From bd2890c0dbcf0ba2b032dfb2ea3d623905fa77c6 Mon Sep 17 00:00:00 2001 From: John Gardiner Myers Date: Wed, 24 Jun 2020 21:01:33 -0700 Subject: [PATCH] Refactor more cluster creation code into NewCluster() --- cmd/kops/create_cluster.go | 57 +++++++----------------------- upup/pkg/fi/cloudup/new_cluster.go | 47 ++++++++++++++++++++---- 2 files changed, 53 insertions(+), 51 deletions(-) diff --git a/cmd/kops/create_cluster.go b/cmd/kops/create_cluster.go index 44379bc088..bfc0644ec3 100644 --- a/cmd/kops/create_cluster.go +++ b/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) diff --git a/upup/pkg/fi/cloudup/new_cluster.go b/upup/pkg/fi/cloudup/new_cluster.go index b24ddb1270..0ce075b6ad 100644 --- a/upup/pkg/fi/cloudup/new_cluster.go +++ b/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,