More refactoring and cleanup

pkg/cfn/manager/api.go

@@ -18,7 +18,8 @@ const (
 	ClusterNameTag = "eksctl.cluster.k8s.io/v1alpha1/cluster-name"
 
 	// NodeGroupNameTag defines the tag of the node group name
-	NodeGroupNameTag = "eksctl.cluster.k8s.io/v1alpha1/nodegroup-name"
+	NodeGroupNameTag  = "eksctl.cluster.k8s.io/v1alpha1/nodegroup-name"
+	oldNodeGroupIDTag = "eksctl.cluster.k8s.io/v1alpha1/nodegroup-id"
 )
 
 var (
@@ -95,7 +96,7 @@ func (c *StackCollection) doCreateStackRequest(i *Stack, templateBody []byte, pa
 // CreateStack with given name, stack builder instance and parameters;
 // any errors will be written to errs channel, when nil is written,
 // assume completion, do not expect more then one error value on the
-// channel, it's closed immediately after it is written two
+// channel, it's closed immediately after it is written to
 func (c *StackCollection) CreateStack(name string, stack builder.ResourceSet, parameters map[string]string, errs chan error) error {
 	i := &Stack{StackName: &name}
 	templateBody, err := stack.RenderJSON()
@@ -225,31 +226,33 @@ func (c *StackCollection) DeleteStack(name string) (*Stack, error) {
 		fmt.Sprintf("%s:%s", ClusterNameTag, c.spec.Metadata.Name))
 }
 
-// WaitDeleteStack kills a stack by name and waits for DELETED status
-func (c *StackCollection) WaitDeleteStack(name string) error {
+// WaitDeleteStack kills a stack by name and waits for DELETED status;
+// any errors will be written to errs channel, when nil is written,
+// assume completion, do not expect more then one error value on the
+// channel, it's closed immediately after it is written to
+func (c *StackCollection) WaitDeleteStack(name string, errs chan error) error {
 	i, err := c.DeleteStack(name)
 	if err != nil {
 		return err
 	}
 
 	logger.Info("waiting for stack %q to get deleted", *i.StackName)
 
-	return c.doWaitUntilStackIsDeleted(i)
+	go c.waitUntilStackIsDeleted(i, errs)
+
+	return nil
 }
 
-// WaitDeleteStackTask kills a stack by name and waits for DELETED status
-// When nil is returned, the `errs` channel must receive an `error` object or `nil`.
-func (c *StackCollection) WaitDeleteStackTask(name string, errs chan error) error {
+// BlockingWaitDeleteStack kills a stack by name and waits for DELETED status
+func (c *StackCollection) BlockingWaitDeleteStack(name string) error {
 	i, err := c.DeleteStack(name)
 	if err != nil {
 		return err
 	}
 
 	logger.Info("waiting for stack %q to get deleted", *i.StackName)
 
-	go c.waitUntilStackIsDeleted(i, errs)
-
-	return nil
+	return c.doWaitUntilStackIsDeleted(i)
 }
 
 // DescribeStacks describes the existing stacks

pkg/cfn/manager/cluster.go

@@ -29,5 +29,5 @@ func (c *StackCollection) DeleteCluster() error {
 
 // WaitDeleteCluster waits till the cluster is deleted
 func (c *StackCollection) WaitDeleteCluster() error {
-	return c.WaitDeleteStack(c.makeClusterStackName())
+	return c.BlockingWaitDeleteStack(c.makeClusterStackName())
 }

pkg/cfn/manager/deprecated.go

@@ -6,7 +6,7 @@ func (c *StackCollection) DeprecatedDeleteStackVPC(wait bool) error {
 	stackName := "EKS-" + c.spec.Metadata.Name + "-VPC"
 
 	if wait {
-		err = c.WaitDeleteStack(stackName)
+		err = c.BlockingWaitDeleteStack(stackName)
 	} else {
 		_, err = c.DeleteStack(stackName)
 	}
@@ -20,7 +20,7 @@ func (c *StackCollection) DeprecatedDeleteStackServiceRole(wait bool) error {
 	stackName := "EKS-" + c.spec.Metadata.Name + "-ServiceRole"
 
 	if wait {
-		err = c.WaitDeleteStack(stackName)
+		err = c.BlockingWaitDeleteStack(stackName)
 	} else {
 		_, err = c.DeleteStack(stackName)
 	}
@@ -34,7 +34,7 @@ func (c *StackCollection) DeprecatedDeleteStackDefaultNodeGroup(wait bool) error
 	stackName := "EKS-" + c.spec.Metadata.Name + "-DefaultNodeGroup"
 
 	if wait {
-		err = c.WaitDeleteStack(stackName)
+		err = c.BlockingWaitDeleteStack(stackName)
 	} else {
 		_, err = c.DeleteStack(stackName)
 	}
@@ -48,7 +48,7 @@ func (c *StackCollection) DeprecatedDeleteStackControlPlane(wait bool) error {
 	stackName := "EKS-" + c.spec.Metadata.Name + "-ControlPlane"
 
 	if wait {
-		err = c.WaitDeleteStack(stackName)
+		err = c.BlockingWaitDeleteStack(stackName)
 	} else {
 		_, err = c.DeleteStack(stackName)
 	}

pkg/cfn/manager/nodegroup.go

@@ -76,19 +76,22 @@ func (c *StackCollection) listAllNodeGroups() ([]string, error) {
 }
 
 // DeleteNodeGroup deletes a nodegroup stack
-func (c *StackCollection) DeleteNodeGroup(errs chan error, data interface{}) error {
-	defer close(errs)
-	name := data.(string)
-	stack := c.MakeNodeGroupStackName(name)
-	_, err := c.DeleteStack(stack)
-	errs <- err
-	return nil
+func (c *StackCollection) DeleteNodeGroup(name string) error {
+	name = c.MakeNodeGroupStackName(name)
+	_, err := c.DeleteStack(name)
+	return err
 }
 
 // WaitDeleteNodeGroup waits until the nodegroup is deleted
 func (c *StackCollection) WaitDeleteNodeGroup(errs chan error, data interface{}) error {
 	name := c.MakeNodeGroupStackName(data.(string))
-	return c.WaitDeleteStackTask(name, errs)
+	return c.WaitDeleteStack(name, errs)
+}
+
+// BlockingWaitDeleteNodeGroup waits until the nodegroup is deleted
+func (c *StackCollection) BlockingWaitDeleteNodeGroup(name string) error {
+	name = c.MakeNodeGroupStackName(name)
+	return c.BlockingWaitDeleteStack(name)
 }
 
 // ScaleInitialNodeGroup will scale the first nodegroup (ID: 0)
@@ -153,23 +156,26 @@ func (c *StackCollection) ScaleNodeGroup(ng *api.NodeGroup) error {
 }
 
 // GetNodeGroupSummaries returns a list of summaries for the nodegroups of a cluster
-func (c *StackCollection) GetNodeGroupSummaries() ([]*NodeGroupSummary, error) {
+func (c *StackCollection) GetNodeGroupSummaries(name string) ([]*NodeGroupSummary, error) {
 	stacks, err := c.ListStacks(fmt.Sprintf("^(eksctl|EKS)-%s-nodegroup-.+$", c.spec.Metadata.Name))
 	if err != nil {
 		return nil, errors.Wrap(err, "getting nodegroup stacks")
 	}
 
 	summaries := []*NodeGroupSummary{}
 	for _, stack := range stacks {
-		logger.Info("stack %s\n", *stack.StackName)
 		logger.Debug("stack = %#v", stack)
 
 		summary, err := c.mapStackToNodeGroupSummary(stack)
 		if err != nil {
 			return nil, errors.New("error mapping stack to node gorup summary")
 		}
 
-		summaries = append(summaries, summary)
+		if name == "" {
+			summaries = append(summaries, summary)
+		} else if summary.Name == name {
+			summaries = append(summaries, summary)
+		}
 	}
 
 	return summaries, nil
@@ -209,6 +215,9 @@ func getNodeGroupName(tags []*cfn.Tag) string {
 		if *tag.Key == NodeGroupNameTag {
 			return *tag.Value
 		}
+		if *tag.Key == oldNodeGroupIDTag {
+			return *tag.Value
+		}
 	}
 	return ""
 }

pkg/cfn/manager/tasks.go

@@ -100,61 +100,45 @@ func (s *StackCollection) CreateOneNodeGroup(ng *api.NodeGroup) []error {
 	})
 }
 
-// deleteAllNodeGroupsTasks returns a list of tasks for deleting all the
-// nodegroup stacks
-func (s *StackCollection) deleteAllNodeGroupsTasks(call taskFunc) ([]task, error) {
-	stacks, err := s.listAllNodeGroups()
+// DeleteAllNodeGroups deletes all nodegroups without waiting
+func (s *StackCollection) DeleteAllNodeGroups(call taskFunc) []error {
+	nodeGroupStackNames, err := s.listAllNodeGroups()
 	if err != nil {
-		return nil, err
+		return []error{err}
 	}
-	deleteAllNodeGroups := []task{}
-	for i := range stacks {
-		t := task{
-			call: call,
-			data: stacks[i],
-		}
-		deleteAllNodeGroups = append(deleteAllNodeGroups, t)
-	}
-	return deleteAllNodeGroups, nil
-}
 
-// DeleteAllNodeGroups runs all tasks required to delete all the nodegroup
-// stacks; any errors will be returned as a slice as soon as the group
-// of tasks is completed
-func (s *StackCollection) DeleteAllNodeGroups() []error {
 	errs := []error{}
-	appendErr := func(err error) {
-		errs = append(errs, err)
-	}
-
-	deleteAllNodeGroups, err := s.deleteAllNodeGroupsTasks(s.DeleteNodeGroup)
-	if err != nil {
-		appendErr(err)
-		return errs
-	}
-
-	if Run(appendErr, deleteAllNodeGroups...); len(errs) > 0 {
-		return errs
+	for _, stackName := range nodeGroupStackNames {
+		if err := s.DeleteNodeGroup(stackName); err != nil {
+			errs = append(errs, err)
+		}
 	}
 
-	return nil
+	return errs
 }
 
 // WaitDeleteAllNodeGroups runs all tasks required to delete all the nodegroup
-// stacks, it waits for each nodegroup to get deleted; any errors will be
-// returned as a slice as soon as the group of tasks is completed
+// stacks and wait for all nodegroups to be deleted; any errors will be returned
+// as a slice as soon as the group of tasks is completed
 func (s *StackCollection) WaitDeleteAllNodeGroups() []error {
+	nodeGroupStackNames, err := s.listAllNodeGroups()
+	if err != nil {
+		return []error{err}
+	}
+
 	errs := []error{}
 	appendErr := func(err error) {
 		errs = append(errs, err)
 	}
 
-	deleteAllNodeGroups, err := s.deleteAllNodeGroupsTasks(s.WaitDeleteNodeGroup)
-	if err != nil {
-		appendErr(err)
-		return errs
+	deleteAllNodeGroups := []task{}
+	for i := range nodeGroupStackNames {
+		t := task{
+			call: s.WaitDeleteNodeGroup,
+			data: nodeGroupStackNames[i],
+		}
+		deleteAllNodeGroups = append(deleteAllNodeGroups, t)
 	}
-
 	if Run(appendErr, deleteAllNodeGroups...); len(errs) > 0 {
 		return errs
 	}

pkg/ctl/cmdutils/cmdutils.go

@@ -27,7 +27,7 @@ func GetNameArg(args []string) string {
 }
 
 // AddCommonFlagsForAWS adds common flags for api.ProviderConfig
-func AddCommonFlagsForAWS(group *NamedFlagSetGroup, p *api.ProviderConfig) {
+func AddCommonFlagsForAWS(group *NamedFlagSetGroup, p *api.ProviderConfig, cfnRole bool) {
 	group.InFlagSet("AWS client", func(fs *pflag.FlagSet) {
 		fs.StringVarP(&p.Profile, "profile", "p", "", "AWS credentials profile to use (overrides the AWS_PROFILE environment variable)")
 
@@ -37,6 +37,9 @@ func AddCommonFlagsForAWS(group *NamedFlagSetGroup, p *api.ProviderConfig) {
 			logger.Debug("ignoring error %q", err.Error())
 		}
 		fs.DurationVar(&p.WaitTimeout, "timeout", api.DefaultWaitTimeout, "max wait time in any polling operations")
+		if cfnRole {
+			fs.StringVar(&p.CloudFormationRoleARN, "cfn-role-arn", "", "IAM role used by CloudFormation to call AWS API on your behalf")
+		}
 	})
 }
 
@@ -45,9 +48,9 @@ func AddRegionFlag(fs *pflag.FlagSet, p *api.ProviderConfig) {
 	fs.StringVarP(&p.Region, "region", "r", "", "AWS region")
 }
 
-// AddCFNRoleARNFlag adds common --cfn-role-arn flag
-func AddCFNRoleARNFlag(fs *pflag.FlagSet, p *api.ProviderConfig) {
-	fs.StringVar(&p.CloudFormationRoleARN, "cfn-role-arn", "", "IAM role used by CloudFormation to call AWS API on your behalf")
+// AddWaitFlag adds common --wait flag
+func AddWaitFlag(wait *bool, fs *pflag.FlagSet) {
+	fs.BoolVarP(wait, "wait", "w", false, "Wait for deletion of all resources before exiting")
 }
 
 // AddCommonFlagsForKubeconfig adds common flags for controlling how output kubeconfig is written

pkg/ctl/completion/completion.go

@@ -3,8 +3,7 @@ package completion
 import (
 	"os"
 
-	"github.com/kubicorn/kubicorn/pkg/logger"
-
+	"github.com/kris-nova/logger"
 	"github.com/spf13/cobra"
 )
 

pkg/ctl/create/cluster.go

@@ -54,7 +54,6 @@ func createClusterCmd(g *cmdutils.Grouping) *cobra.Command {
 		fs.StringVarP(&cfg.Metadata.Name, "name", "n", "", fmt.Sprintf("EKS cluster name (generated if unspecified, e.g. %q)", exampleClusterName))
 		fs.StringToStringVarP(&cfg.Metadata.Tags, "tags", "", map[string]string{}, `A list of KV pairs used to tag the AWS resources (e.g. "Owner=John Doe,Team=Some Team")`)
 		cmdutils.AddRegionFlag(fs, p)
-		cmdutils.AddCFNRoleARNFlag(fs, p)
 		fs.StringSliceVar(&availabilityZones, "zones", nil, "(auto-select if unspecified)")
 		fs.StringVar(&cfg.Metadata.Version, "version", api.LatestVersion, fmt.Sprintf("Kubernetes version (valid options: %s)", strings.Join(api.SupportedVersions(), ",")))
 	})
@@ -80,7 +79,7 @@ func createClusterCmd(g *cmdutils.Grouping) *cobra.Command {
 		fs.StringVar(&kopsClusterNameForVPC, "vpc-from-kops-cluster", "", "re-use VPC from a given kops cluster")
 	})
 
-	cmdutils.AddCommonFlagsForAWS(group, p)
+	cmdutils.AddCommonFlagsForAWS(group, p, true)
 
 	group.InFlagSet("Output kubeconfig", func(fs *pflag.FlagSet) {
 		cmdutils.AddCommonFlagsForKubeconfig(fs, &kubeconfigPath, &setContext, &autoKubeconfigPath, exampleClusterName)
@@ -110,6 +109,7 @@ func doCreateCluster(p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.Node
 	}
 	meta.Name = utils.ClusterName(meta.Name, nameArg)
 
+	// Use given name or generate one, no argument mode here
 	ng.Name = utils.NodeGroupName(ng.Name, "")
 
 	if autoKubeconfigPath {

pkg/ctl/create/nodegroup.go

@@ -3,13 +3,13 @@ package create
 import (
 	"fmt"
 	"os"
-
 	"strings"
 
-	"github.com/kubicorn/kubicorn/pkg/logger"
+	"github.com/kris-nova/logger"
 	"github.com/pkg/errors"
 	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
+
 	"github.com/weaveworks/eksctl/pkg/ctl/cmdutils"
 	"github.com/weaveworks/eksctl/pkg/eks"
 	"github.com/weaveworks/eksctl/pkg/eks/api"
@@ -39,7 +39,6 @@ func createNodeGroupCmd(g *cmdutils.Grouping) *cobra.Command {
 	group.InFlagSet("General", func(fs *pflag.FlagSet) {
 		fs.StringVar(&cfg.Metadata.Name, "cluster", "", "name of the EKS cluster to add the nodegroup to")
 		cmdutils.AddRegionFlag(fs, p)
-		cmdutils.AddCFNRoleARNFlag(fs, p)
 		fs.StringVar(&cfg.Metadata.Version, "version", api.LatestVersion, fmt.Sprintf("Kubernetes version (valid options: %s)", strings.Join(api.SupportedVersions(), ",")))
 	})
 
@@ -48,7 +47,7 @@ func createNodeGroupCmd(g *cmdutils.Grouping) *cobra.Command {
 		cmdutils.AddCommonCreateNodeGroupFlags(fs, p, cfg, ng)
 	})
 
-	cmdutils.AddCommonFlagsForAWS(group, p)
+	cmdutils.AddCommonFlagsForAWS(group, p, true)
 
 	group.AddTo(cmd)
 
@@ -134,7 +133,7 @@ func doCreateNodeGroup(p *api.ProviderConfig, cfg *api.ClusterConfig, ng *api.No
 			return err
 		}
 	}
-	logger.Success("EKS cluster %q in %q region has a new nodegroup with name %d", cfg.Metadata.Name, cfg.Metadata.Region, ng.Name)
+	logger.Success("created nodegroup %q in cluster %q", cfg.Metadata.Name, ng.Name)
 
 	return nil