From e77f7c432ac9e5f92fc9ff93d2ec51e8ddb8de9a Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Wed, 6 Jun 2018 11:44:26 +0100 Subject: [PATCH] Expand to us-east-1, add proper config validation --- cmd/eksctl/create.go | 37 +++++------- cmd/eksctl/delete.go | 11 ++-- cmd/eksctl/get.go | 8 ++- cmd/eksctl/utils.go | 10 ++-- pkg/eks/api.go | 133 ++++++++++++++++++++++++++++++++++++++++--- pkg/eks/auth.go | 7 +++ pkg/eks/cfn.go | 13 ----- 7 files changed, 162 insertions(+), 57 deletions(-) diff --git a/cmd/eksctl/create.go b/cmd/eksctl/create.go index 1433522bbc1..6731e646f8c 100644 --- a/cmd/eksctl/create.go +++ b/cmd/eksctl/create.go @@ -29,13 +29,6 @@ func createCmd() *cobra.Command { return cmd } -const ( - DEFAULT_EKS_REGION = "us-west-2" - DEFAULT_NODE_COUNT = 2 - DEFAULT_NODE_TYPE = "m5.large" - DEFAULT_SSH_PUBLIC_KEY = "~/.ssh/id_rsa.pub" -) - var ( writeKubeconfig bool kubeconfigPath string @@ -46,11 +39,11 @@ func getClusterName() string { } func createClusterCmd() *cobra.Command { - cfg := &eks.ClusterConfig{} + cfg := &eks.ClusterConfig{Interactive: true} cmd := &cobra.Command{ Use: "cluster", - Short: "Create a custer", + Short: "Create a custer (all flags are optional)", Run: func(_ *cobra.Command, _ []string) { if err := doCreateCluster(cfg); err != nil { logger.Critical(err.Error()) @@ -62,16 +55,16 @@ func createClusterCmd() *cobra.Command { fs := cmd.Flags() fs.StringVarP(&cfg.ClusterName, "cluster-name", "n", "", fmt.Sprintf("EKS cluster name (generated if unspecified, e.g. %q)", getClusterName())) - fs.StringVarP(&cfg.Region, "region", "r", DEFAULT_EKS_REGION, "AWS region") + fs.StringVarP(&cfg.Region, "region", "r", eks.DEFAULT_REGION, "AWS region") - fs.StringVarP(&cfg.NodeType, "node-type", "t", DEFAULT_NODE_TYPE, "node instance type") - fs.IntVarP(&cfg.Nodes, "nodes", "N", DEFAULT_NODE_COUNT, "total number of nodes (for a static ASG)") + fs.StringVarP(&cfg.NodeType, "node-type", "t", eks.DEFAULT_NODE_TYPE, "node instance type") + fs.IntVarP(&cfg.Nodes, "nodes", "N", eks.DEFAULT_NODE_COUNT, "total number of nodes (for a static ASG)") + fs.StringVarP(&cfg.NodeAMI, "node-ami", "", "", "custom node AMI") - // TODO: https://github.com/weaveworks/eksctl/issues/28 fs.IntVarP(&cfg.MinNodes, "nodes-min", "m", 0, "minimum nodes in ASG") fs.IntVarP(&cfg.MaxNodes, "nodes-max", "M", 0, "maximum nodes in ASG") - fs.StringVar(&cfg.SSHPublicKeyPath, "ssh-public-key", DEFAULT_SSH_PUBLIC_KEY, "SSH public key to use for nodes (import from local path, or use existing EC2 key pair)") + fs.StringVar(&cfg.SSHPublicKeyPath, "ssh-public-key", eks.DEFAULT_SSH_PUBLIC_KEY, "SSH public key to use for nodes (import from local path, or use existing EC2 key pair)") fs.BoolVar(&writeKubeconfig, "write-kubeconfig", true, "toggle writing of kubeconfig") fs.StringVar(&kubeconfigPath, "kubeconfig", "kubeconfig", "path to write kubeconfig") @@ -82,20 +75,20 @@ func createClusterCmd() *cobra.Command { func doCreateCluster(cfg *eks.ClusterConfig) error { ctl := eks.New(cfg) - if err := ctl.CheckAuth(); err != nil { - return err - } - if cfg.ClusterName == "" { cfg.ClusterName = getClusterName() } - if cfg.SSHPublicKeyPath == "" { - return fmt.Errorf("--ssh-public-key must be non-empty string") + if err := ctl.CheckConfig(); err != nil { + return err + } + + if err := ctl.CheckNodeCountConfig(); err != nil { + return err } - if cfg.Region != DEFAULT_EKS_REGION { - return fmt.Errorf("only --region=%s is supported in this version") + if err := ctl.CheckAuth(); err != nil { + return err } if err := ctl.LoadSSHPublicKey(); err != nil { diff --git a/cmd/eksctl/delete.go b/cmd/eksctl/delete.go index 3996acf877a..68cdb3dd049 100644 --- a/cmd/eksctl/delete.go +++ b/cmd/eksctl/delete.go @@ -1,7 +1,6 @@ package main import ( - "fmt" "os" "github.com/spf13/cobra" @@ -26,7 +25,7 @@ func deleteCmd() *cobra.Command { } func deleteClusterCmd() *cobra.Command { - cfg := &eks.ClusterConfig{} + cfg := &eks.ClusterConfig{Interactive: true} cmd := &cobra.Command{ Use: "cluster", @@ -42,7 +41,7 @@ func deleteClusterCmd() *cobra.Command { fs := cmd.Flags() fs.StringVarP(&cfg.ClusterName, "cluster-name", "n", "", "EKS cluster name (required)") - fs.StringVarP(&cfg.Region, "region", "r", DEFAULT_EKS_REGION, "AWS region") + fs.StringVarP(&cfg.Region, "region", "r", eks.DEFAULT_REGION, "AWS region") return cmd } @@ -50,12 +49,12 @@ func deleteClusterCmd() *cobra.Command { func doDeleteCluster(cfg *eks.ClusterConfig) error { ctl := eks.New(cfg) - if err := ctl.CheckAuth(); err != nil { + if err := ctl.CheckConfig(); err != nil { return err } - if cfg.ClusterName == "" { - return fmt.Errorf("--cluster-name must be set") + if err := ctl.CheckAuth(); err != nil { + return err } logger.Info("deleting EKS cluster %q", cfg.ClusterName) diff --git a/cmd/eksctl/get.go b/cmd/eksctl/get.go index 2c83026df73..16f6eb18061 100644 --- a/cmd/eksctl/get.go +++ b/cmd/eksctl/get.go @@ -25,7 +25,7 @@ func getCmd() *cobra.Command { } func getClusterCmd() *cobra.Command { - cfg := &eks.ClusterConfig{} + cfg := &eks.ClusterConfig{Interactive: true} cmd := &cobra.Command{ Use: "cluster", @@ -42,7 +42,7 @@ func getClusterCmd() *cobra.Command { fs := cmd.Flags() fs.StringVarP(&cfg.ClusterName, "cluster-name", "n", "", "EKS cluster name") - fs.StringVarP(&cfg.Region, "region", "r", DEFAULT_EKS_REGION, "AWS region") + fs.StringVarP(&cfg.Region, "region", "r", eks.DEFAULT_REGION, "AWS region") return cmd } @@ -50,6 +50,10 @@ func getClusterCmd() *cobra.Command { func doGetCluster(cfg *eks.ClusterConfig) error { ctl := eks.New(cfg) + if err := ctl.CheckConfig(); err != nil { + return err + } + if err := ctl.CheckAuth(); err != nil { return err } diff --git a/cmd/eksctl/utils.go b/cmd/eksctl/utils.go index b5a758efbf3..f96f6f31ef9 100644 --- a/cmd/eksctl/utils.go +++ b/cmd/eksctl/utils.go @@ -51,7 +51,7 @@ func waitNodesCmd() *cobra.Command { fs := cmd.Flags() fs.StringVar(&utilsKubeconfigInputPath, "kubeconfig", "kubeconfig", "path to read kubeconfig") - fs.IntVarP(&cfg.MinNodes, "nodes-min", "m", DEFAULT_NODE_COUNT, "minimum nodes to wait for") + fs.IntVarP(&cfg.MinNodes, "nodes-min", "m", eks.DEFAULT_NODE_COUNT, "minimum nodes to wait for") return cmd } @@ -79,7 +79,7 @@ func doWaitNodes(cfg *eks.ClusterConfig) error { } func writeKubeconfigCmd() *cobra.Command { - cfg := &eks.ClusterConfig{} + cfg := &eks.ClusterConfig{Interactive: true} cmd := &cobra.Command{ Use: "write-kubeconfig", @@ -95,7 +95,7 @@ func writeKubeconfigCmd() *cobra.Command { fs := cmd.Flags() fs.StringVarP(&cfg.ClusterName, "cluster-name", "n", "", fmt.Sprintf("EKS cluster name (generated if unspecified, e.g. %q)", getClusterName())) - fs.StringVarP(&cfg.Region, "region", "r", DEFAULT_EKS_REGION, "AWS region") + fs.StringVarP(&cfg.Region, "region", "r", eks.DEFAULT_REGION, "AWS region") fs.StringVar(&utilsKubeconfigOutputPath, "kubeconfig", "", "path to write kubeconfig") @@ -105,8 +105,8 @@ func writeKubeconfigCmd() *cobra.Command { func doWriteKubeconfigCmd(cfg *eks.ClusterConfig) error { ctl := eks.New(cfg) - if cfg.ClusterName == "" { - return fmt.Errorf("--cluster-name must be set") + if err := ctl.CheckConfig(); err != nil { + return err } if utilsKubeconfigOutputPath == "" { diff --git a/pkg/eks/api.go b/pkg/eks/api.go index 85e71502a47..3ca01e187e9 100644 --- a/pkg/eks/api.go +++ b/pkg/eks/api.go @@ -1,7 +1,10 @@ package eks import ( + "fmt" "os" + "reflect" + "strings" "sync" "github.com/pkg/errors" @@ -33,15 +36,18 @@ type providerServices struct { // simple config, to be replaced with Cluster API type ClusterConfig struct { - Region string - ClusterName string - NodeAMI string - NodeType string - Nodes int - MinNodes int - MaxNodes int - - SSHPublicKeyPath string + Interactive bool // for interactive use, i.e. eksctl + + Region string `flag:"--regions"` + ClusterName string `flag:"--cluster-name"` + NodeOS string `flag:"--node-os"` + NodeAMI string `flag:"--node-ami"` + NodeType string `flag:"--node-type"` + Nodes int `flag:"--nodes"` + MinNodes int `flag:"--nodes-min"` + MaxNodes int `flag:"--nodes-max"` + + SSHPublicKeyPath string `flag:"--ssh-public-key"` SSHPublicKey []byte keyName string @@ -56,6 +62,29 @@ type ClusterConfig struct { CertificateAuthorityData []byte } +const ( + DEFAULT_NODE_COUNT = 2 + DEFAULT_NODE_TYPE = "m5.large" + + REGION_US_WEST_2 = "us-west-2" + REGION_US_EAST_2 = "us-east-2" + + DEFAULT_REGION = REGION_US_WEST_2 + + NODE_OS_AMAZON_LINUX_2 = "Amazon Linux 2" + + DEFAULT_NODE_OS = NODE_OS_AMAZON_LINUX_2 +) + +var regionalAMIs = map[string]map[string]string{ + REGION_US_WEST_2: { + NODE_OS_AMAZON_LINUX_2: "ami-73a6e20b", + }, + REGION_US_EAST_2: { + NODE_OS_AMAZON_LINUX_2: "ami-dea4d5a1", + }, +} + func New(clusterConfig *ClusterConfig) *ClusterProvider { // we might want to use bits from kops, although right now it seems like too many thing we // don't want yet @@ -97,6 +126,92 @@ func New(clusterConfig *ClusterConfig) *ClusterProvider { return cfn } +func (c *ClusterConfig) getFlagOrField(fieldName string) string { + if !c.Interactive { + return fieldName + } + field, ok := reflect.TypeOf(c).Elem().FieldByName(fieldName) + if !ok { + return fieldName + } + if flag := field.Tag.Get("flag"); flag != "" { + return flag + } + return fieldName +} + +func (c *ClusterProvider) CheckConfig() error { + + regionalAMIs, ok := regionalAMIs[c.cfg.Region] + if !ok { + supportedRegions := []string{} + for r := range regionalAMIs { + supportedRegions = append(supportedRegions, r) + } + return fmt.Errorf("unsupported %s %q, EKS is only availabe in the following regions: %s", + c.cfg.getFlagOrField("Region"), + c.cfg.Region, + strings.Join(supportedRegions, ", "), + ) + } + + c.cfg.NodeOS = DEFAULT_NODE_OS // will expose when more OSs are available + + if c.cfg.NodeAMI == "" { + c.cfg.NodeAMI, ok = regionalAMIs[c.cfg.NodeOS] + if !ok { + return fmt.Errorf("unsuported %s %q, use %s to set custom AMI", + c.cfg.getFlagOrField("NodeOS"), + c.cfg.NodeOS, + c.cfg.getFlagOrField("NodeAMI"), + ) + } + } + + if c.cfg.ClusterName == "" { + return fmt.Errorf("%s must be set", c.cfg.getFlagOrField("ClusterName")) + } + return nil +} + +func (c *ClusterProvider) CheckNodeCountConfig() error { + // this is separate from CheckConfig as we would only call it on create + if c.cfg.MinNodes < 0 || c.cfg.MaxNodes < 0 || c.cfg.Nodes < 0 { + return fmt.Errorf("%s, %s or %s cannot be less than zero", + c.cfg.getFlagOrField("MinNodes"), + c.cfg.getFlagOrField("MaxNodes"), + c.cfg.getFlagOrField("Nodes"), + ) + } + + if c.cfg.MinNodes != 0 && c.cfg.MaxNodes != 0 && c.cfg.Nodes != 0 { + return fmt.Errorf("%s, %s and %s cannot be specified all at the same time", + c.cfg.getFlagOrField("MinNodes"), + c.cfg.getFlagOrField("MaxNodes"), + c.cfg.getFlagOrField("Nodes"), + ) + } + + if c.cfg.MinNodes == 0 && c.cfg.MaxNodes == 0 { + // defaults + c.cfg.MinNodes = c.cfg.Nodes + c.cfg.MaxNodes = c.cfg.Nodes + } else { + // ambiguities + if c.cfg.MinNodes > c.cfg.MaxNodes { + return fmt.Errorf("%s cannot be greater than %s", + c.cfg.getFlagOrField("MinNodes"), + c.cfg.getFlagOrField("MaxNodes"), + ) + } + if c.cfg.MinNodes > 0 && c.cfg.MaxNodes == 0 && c.cfg.Nodes > 0 { + c.cfg.MaxNodes = c.cfg.Nodes + } + } + + return nil +} + func (c *ClusterProvider) CheckAuth() error { { input := &sts.GetCallerIdentityInput{} diff --git a/pkg/eks/auth.go b/pkg/eks/auth.go index ef1d835a8b4..29b48786ff5 100644 --- a/pkg/eks/auth.go +++ b/pkg/eks/auth.go @@ -22,7 +22,14 @@ import ( "k8s.io/kops/upup/pkg/fi/utils" ) +const DEFAULT_SSH_PUBLIC_KEY = "~/.ssh/id_rsa.pub" + func (c *ClusterProvider) LoadSSHPublicKey() error { + // this is validate here, as it's only used explictily on creation + if c.cfg.SSHPublicKeyPath == "" { + return fmt.Errorf("%s must be set", c.cfg.getFlagOrField("SSHPublicKeyPath")) + } + c.cfg.SSHPublicKeyPath = utils.ExpandPath(c.cfg.SSHPublicKeyPath) sshPublicKey, err := ioutil.ReadFile(c.cfg.SSHPublicKeyPath) if err != nil { diff --git a/pkg/eks/cfn.go b/pkg/eks/cfn.go index c9135d0fde7..94fc5d42bf5 100644 --- a/pkg/eks/cfn.go +++ b/pkg/eks/cfn.go @@ -272,19 +272,6 @@ func (c *ClusterProvider) stackNameDefaultNodeGroup() string { } func (c *ClusterProvider) stackParamsDefaultNodeGroup() map[string]string { - regionalAMIs := map[string]string{ - "us-west-2": "ami-993141e1", - } - - if c.cfg.NodeAMI == "" { - c.cfg.NodeAMI = regionalAMIs[c.cfg.Region] - } - - if c.cfg.MinNodes == 0 && c.cfg.MaxNodes == 0 { - c.cfg.MinNodes = c.cfg.Nodes - c.cfg.MaxNodes = c.cfg.Nodes - } - return map[string]string{ "ClusterName": c.cfg.ClusterName, "NodeGroupName": "default",