From d721decc8c0ae1f801646a9ebd2ebf40b834fe09 Mon Sep 17 00:00:00 2001 From: gotjosh Date: Sat, 3 Nov 2018 12:35:12 +0000 Subject: [PATCH 1/3] Add `--all-regions` flag for `get cluster` Allows listing of clusters across all supported regions. --- pkg/ctl/get/cluster.go | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) diff --git a/pkg/ctl/get/cluster.go b/pkg/ctl/get/cluster.go index 02217c0d62..11943ffa17 100644 --- a/pkg/ctl/get/cluster.go +++ b/pkg/ctl/get/cluster.go @@ -11,6 +11,8 @@ import ( "github.com/weaveworks/eksctl/pkg/eks/api" ) +var listAllRegions bool + func getClusterCmd() *cobra.Command { cfg := api.NewClusterConfig() @@ -19,16 +21,25 @@ func getClusterCmd() *cobra.Command { Short: "Get cluster(s)", Aliases: []string{"clusters"}, Run: func(_ *cobra.Command, args []string) { - if err := doGetCluster(cfg, ctl.GetNameArg(args)); err != nil { - logger.Critical("%s\n", err.Error()) - os.Exit(1) + if listAllRegions { + if err := doGetClusterForAllRegions(cfg); err != nil { + logger.Critical("%s\n", err.Error()) + os.Exit(1) + } + } else { + if err := doGetCluster(cfg, ctl.GetNameArg(args)); err != nil { + logger.Critical("%s\n", err.Error()) + os.Exit(1) + } } + }, } fs := cmd.Flags() fs.StringVarP(&cfg.ClusterName, "name", "n", "", "EKS cluster name") + fs.BoolVarP(&listAllRegions, "all-regions", "a", false, "List clusters across all supported regions") fs.IntVar(&chunkSize, "chunk-size", defaultChunkSize, "Return large lists in chunks rather than all at once. Pass 0 to disable.") fs.StringVarP(&cfg.Region, "region", "r", "", "AWS region") @@ -59,3 +70,21 @@ func doGetCluster(cfg *api.ClusterConfig, name string) error { return nil } + +func doGetClusterForAllRegions(cfg *api.ClusterConfig) error { + for _, region := range api.SupportedRegions() { + logger.Info("using region %s", region) + cfg.Region = region + ctl := eks.New(cfg) + + if err := ctl.CheckAuth(); err != nil { + return err + } + + if err := ctl.ListClusters(chunkSize, output); err != nil { + return err + } + } + + return nil +} From 6a680250c52dae9f1012fb0db9dc374da7f5a6ad Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Mon, 5 Nov 2018 10:27:49 +0000 Subject: [PATCH 2/3] Refactor `--all-regions` logic To print clusters in all regions nicely we need to make list of all the clusters with regions and call printer once. For this it is better/easier to have `listAllRegions` flags in the underlying API package. --- pkg/ctl/get/cluster.go | 38 ++++--------------------------- pkg/eks/eks.go | 51 ++++++++++++++++++++++++++---------------- pkg/eks/eks_test.go | 10 ++++----- 3 files changed, 41 insertions(+), 58 deletions(-) diff --git a/pkg/ctl/get/cluster.go b/pkg/ctl/get/cluster.go index 11943ffa17..486f7ed553 100644 --- a/pkg/ctl/get/cluster.go +++ b/pkg/ctl/get/cluster.go @@ -21,18 +21,10 @@ func getClusterCmd() *cobra.Command { Short: "Get cluster(s)", Aliases: []string{"clusters"}, Run: func(_ *cobra.Command, args []string) { - if listAllRegions { - if err := doGetClusterForAllRegions(cfg); err != nil { - logger.Critical("%s\n", err.Error()) - os.Exit(1) - } - } else { - if err := doGetCluster(cfg, ctl.GetNameArg(args)); err != nil { - logger.Critical("%s\n", err.Error()) - os.Exit(1) - } + if err := doGetCluster(cfg, ctl.GetNameArg(args)); err != nil { + logger.Critical("%s\n", err.Error()) + os.Exit(1) } - }, } @@ -64,27 +56,5 @@ func doGetCluster(cfg *api.ClusterConfig, name string) error { cfg.ClusterName = name } - if err := ctl.ListClusters(chunkSize, output); err != nil { - return err - } - - return nil -} - -func doGetClusterForAllRegions(cfg *api.ClusterConfig) error { - for _, region := range api.SupportedRegions() { - logger.Info("using region %s", region) - cfg.Region = region - ctl := eks.New(cfg) - - if err := ctl.CheckAuth(); err != nil { - return err - } - - if err := ctl.ListClusters(chunkSize, output); err != nil { - return err - } - } - - return nil + return ctl.ListClusters(chunkSize, output, listAllRegions) } diff --git a/pkg/eks/eks.go b/pkg/eks/eks.go index 1a67dc6037..ae6c5d5349 100644 --- a/pkg/eks/eks.go +++ b/pkg/eks/eks.go @@ -7,6 +7,7 @@ import ( "strings" "time" + "github.com/weaveworks/eksctl/pkg/eks/api" "github.com/weaveworks/eksctl/pkg/printers" "github.com/pkg/errors" @@ -66,7 +67,7 @@ func (c *ClusterProvider) GetCredentials(cluster awseks.Cluster) error { } // ListClusters display details of all the EKS cluster in your account -func (c *ClusterProvider) ListClusters(chunkSize int, output string) error { +func (c *ClusterProvider) ListClusters(chunkSize int, output string, eachRegion bool) error { // NOTE: this needs to be reworked in the future so that the functionality // is combined. This require the ability to return details of all clusters // in a single call. @@ -85,33 +86,49 @@ func (c *ClusterProvider) ListClusters(chunkSize int, output string) error { if output == "table" { addListTableColumns(printer.(*printers.TablePrinter)) } - return c.doListClusters(int64(chunkSize), printer) + allClusters := []*clusterWithRegion{} + if err := c.doListClusters(int64(chunkSize), printer, &allClusters, eachRegion); err != nil { + return err + } + return printer.PrintObj("clusters", allClusters, os.Stdout) } -func (c *ClusterProvider) doListClusters(chunkSize int64, printer printers.OutputPrinter) error { - allClusters := []*clusterWithRegion{} +func (c *ClusterProvider) getClustersRequest(chunkSize int64, nextToken string) ([]*string, *string, error) { + input := &awseks.ListClustersInput{MaxResults: &chunkSize} + if nextToken != "" { + input = input.SetNextToken(nextToken) + } + output, err := c.Provider.EKS().ListClusters(input) + if err != nil { + return nil, nil, errors.Wrap(err, "listing control planes") + } + return output.Clusters, output.NextToken, nil +} - getFunc := func(chunkSize int64, nextToken string) ([]*string, *string, error) { - input := &awseks.ListClustersInput{MaxResults: &chunkSize} - if nextToken != "" { - input = input.SetNextToken(nextToken) - } - output, err := c.Provider.EKS().ListClusters(input) - if err != nil { - return nil, nil, errors.Wrap(err, "listing control planes") +func (c *ClusterProvider) doListClusters(chunkSize int64, printer printers.OutputPrinter, allClusters *[]*clusterWithRegion, eachRegion bool) error { + if eachRegion { + // reset region and re-create the client, then make a recursive call + for _, region := range api.SupportedRegions() { + c.Spec.Region = region + if err := New(c.Spec).doListClusters(chunkSize, printer, allClusters, false); err != nil { + return err + } } - return output.Clusters, output.NextToken, nil + return nil } token := "" for { - clusters, nextToken, err := getFunc(chunkSize, token) + clusters, nextToken, err := c.getClustersRequest(chunkSize, token) if err != nil { return err } for _, clusterName := range clusters { - allClusters = append(allClusters, &clusterWithRegion{Name: *clusterName, Region: c.Spec.Region}) + *allClusters = append(*allClusters, &clusterWithRegion{ + Name: *clusterName, + Region: c.Spec.Region, + }) } if nextToken != nil && *nextToken != "" { @@ -121,10 +138,6 @@ func (c *ClusterProvider) doListClusters(chunkSize int64, printer printers.Outpu } } - if err := printer.PrintObj("clusters", allClusters, os.Stdout); err != nil { - return err - } - return nil } diff --git a/pkg/eks/eks_test.go b/pkg/eks/eks_test.go index 85ffef6f56..2b5ff5ef5a 100644 --- a/pkg/eks/eks_test.go +++ b/pkg/eks/eks_test.go @@ -61,7 +61,7 @@ var _ = Describe("Eks", func() { }) JustBeforeEach(func() { - err = c.ListClusters(100, output) + err = c.ListClusters(100, output, false) }) It("should not error", func() { @@ -92,7 +92,7 @@ var _ = Describe("Eks", func() { }) JustBeforeEach(func() { - err = c.ListClusters(100, output) + err = c.ListClusters(100, output, false) }) It("should not error", func() { @@ -143,7 +143,7 @@ var _ = Describe("Eks", func() { }) JustBeforeEach(func() { - err = c.ListClusters(100, output) + err = c.ListClusters(100, output, false) }) AfterEach(func() { @@ -222,7 +222,7 @@ var _ = Describe("Eks", func() { }) JustBeforeEach(func() { - err = c.ListClusters(chunkSize, output) + err = c.ListClusters(chunkSize, output, false) }) It("should not error", func() { @@ -257,7 +257,7 @@ var _ = Describe("Eks", func() { }) JustBeforeEach(func() { - err = c.ListClusters(chunkSize, output) + err = c.ListClusters(chunkSize, output, false) }) It("should not error", func() { From ae6c894ed8be854e555d5cb01196fdad266ba15a Mon Sep 17 00:00:00 2001 From: Ilya Dmitrichenko Date: Mon, 5 Nov 2018 13:42:09 +0000 Subject: [PATCH 3/3] Add flag validation, use `-A` as alias for `--all-regions` As `-a` will be more appropritate for `--all`, that would be for including clusters with non-active status (i.e. creating & deleting). --- pkg/ctl/get/cluster.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/pkg/ctl/get/cluster.go b/pkg/ctl/get/cluster.go index 486f7ed553..bc7ba86591 100644 --- a/pkg/ctl/get/cluster.go +++ b/pkg/ctl/get/cluster.go @@ -3,6 +3,7 @@ package get import ( "fmt" "os" + "strings" "github.com/kubicorn/kubicorn/pkg/logger" "github.com/spf13/cobra" @@ -31,7 +32,7 @@ func getClusterCmd() *cobra.Command { fs := cmd.Flags() fs.StringVarP(&cfg.ClusterName, "name", "n", "", "EKS cluster name") - fs.BoolVarP(&listAllRegions, "all-regions", "a", false, "List clusters across all supported regions") + fs.BoolVarP(&listAllRegions, "all-regions", "A", false, "List clusters across all supported regions") fs.IntVar(&chunkSize, "chunk-size", defaultChunkSize, "Return large lists in chunks rather than all at once. Pass 0 to disable.") fs.StringVarP(&cfg.Region, "region", "r", "", "AWS region") @@ -42,10 +43,15 @@ func getClusterCmd() *cobra.Command { } func doGetCluster(cfg *api.ClusterConfig, name string) error { + regionGiven := cfg.Region != "" // eks.New resets this field, so we need to check if it was set in the fist place ctl := eks.New(cfg) - if err := ctl.CheckAuth(); err != nil { - return err + if !cfg.IsSupportedRegion() { + return fmt.Errorf("--region=%s is not supported - use one of: %s", cfg.Region, strings.Join(api.SupportedRegions(), ", ")) + } + + if regionGiven && listAllRegions { + logger.Warning("--region=%s is ignored, as --all-regions is given", cfg.Region) } if cfg.ClusterName != "" && name != "" { @@ -56,5 +62,13 @@ func doGetCluster(cfg *api.ClusterConfig, name string) error { cfg.ClusterName = name } + if cfg.ClusterName != "" && listAllRegions { + return fmt.Errorf("--all-regions is for listing all clusters, it must be used without cluster name flag/argument") + } + + if err := ctl.CheckAuth(); err != nil { + return err + } + return ctl.ListClusters(chunkSize, output, listAllRegions) }