Refactoring and cleanup

- move name generator functions into one file make usage consistent
- review stack manager code
- make examples in docs more consistent
- update output message formatting, improve consistency
- update some usage messages, move more flags into cmdutils
- refactor `ctl.GetCredentials`
- do not reset tags when scaling nodegroup

README.md

@@ -172,28 +172,28 @@ eksctl delete cluster --name=<name> [--region=<region>]
 
 You can add one or more nodegroups in addition to the initial nodegroup created along with the cluster.
 
-To create an additional nodegroup, run:
+To create an additional nodegroup, use:
 
 ```
-eksctl create nodegroup --cluster=<cluser name>
+eksctl create nodegroup --cluster=<clusterName> [--name=<nodegroupName>]
 ```
 
 To list the details about a nodegroup or all of the nodegroups, use:
 
 ```
-eksctl get nodegroup --cluster=<cluster name> [<nodegroup name>]
+eksctl get nodegroup --cluster=<clusterName> [--name=<nodegroupName>]
 ```
 
 A nodegroup can be scaled by using the `eksctl scale nodegroup` command:
 
 ```
-eksctl delete nodegroup --cluster=<cluster name> --nodes=<desired count> <nodegroup name>
+eksctl delete nodegroup --cluster=<clusterName> --nodes=<desiredCount> --name=<nodegroupName>
 ```
 
-For example, to scale the nodegroup `ng-abcd1234` to 5 nodes:
+For example, to scale nodegroup `ng-a345f4e1` in `cluster-1` to 5 nodes, run:
 
 ```
-eksctl scale nodegroup --cluster=<cluster name> --nodes=5 ng-abcd1234
+eksctl scale nodegroup --cluster=cluster-1 --nodes=5 ng-a345f4e1
 ```
 
 If the desired number of nodes is greater than the current maximum set on the ASG then the maximum value will be increased to match the number of requested nodes. And likewise for the minimum.
@@ -205,7 +205,7 @@ Scaling a nodegroup works by modifying the nodegroup CloudFormation stack via a
 To delete a nodegroup, run:
 
 ```
-eksctl delete nodegroup --cluster=<cluster name> <nodegroup name>
+eksctl delete nodegroup --cluster=<clusterName> --name=<nodegroupName>
 ```
 
 ### VPC Networking

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
@@ -287,7 +290,6 @@ func (c *StackCollection) doCreateChangeSetRequest(i *Stack, action string, desc
 
 	input.SetChangeSetType(cloudformation.ChangeSetTypeUpdate)
 
-	input.SetTags(c.tags)
 	input.SetTemplateBody(string(templateBody))
 
 	if withIAM {

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,25 +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 := data.(string)
-	stack := c.MakeNodeGroupStackName(name)
-	return c.WaitDeleteStackTask(stack, errs)
+	name := c.MakeNodeGroupStackName(data.(string))
+	return c.WaitDeleteStack(name, errs)
 }
 
-// ScaleInitialNodeGroup will scale the first nodegroup (ID: 0)
-func (c *StackCollection) ScaleInitialNodeGroup() error {
-	return c.ScaleNodeGroup(c.spec.NodeGroups[0])
+// BlockingWaitDeleteNodeGroup waits until the nodegroup is deleted
+func (c *StackCollection) BlockingWaitDeleteNodeGroup(name string) error {
+	name = c.MakeNodeGroupStackName(name)
+	return c.BlockingWaitDeleteStack(name)
 }
 
 // ScaleNodeGroup will scale an existing nodegroup
@@ -154,23 +151,29 @@ 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)
+		if *stack.StackStatus == cfn.StackStatusDeleteComplete {
+			continue
+		}
 		logger.Debug("stack = %#v", stack)
 
 		summary, err := c.mapStackToNodeGroupSummary(stack)
 		if err != nil {
-			return nil, errors.New("error mapping stack to node gorup summary")
+			return nil, errors.Wrap(err, "mapping stack to nodegorup summary")
 		}
 
-		summaries = append(summaries, summary)
+		if name == "" {
+			summaries = append(summaries, summary)
+		} else if summary.Name == name {
+			summaries = append(summaries, summary)
+		}
 	}
 
 	return summaries, nil
@@ -210,6 +213,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/nodegroup_test.go

@@ -1,13 +1,14 @@
 package manager
 
 import (
+	"errors"
+
 	"github.com/aws/aws-sdk-go/aws"
 	cfn "github.com/aws/aws-sdk-go/service/cloudformation"
 	. "github.com/onsi/ginkgo"
 	. "github.com/onsi/gomega"
 	"github.com/stretchr/testify/mock"
 	"github.com/weaveworks/eksctl/pkg/eks/api"
-	"errors"
 	"github.com/weaveworks/eksctl/pkg/testutils/mockprovider"
 )
 
@@ -79,8 +80,9 @@ var _ = Describe("StackCollection NodeGroup", func() {
 				})).Return(&cfn.DescribeStacksOutput{
 					Stacks: []*cfn.Stack{
 						{
-							StackName: aws.String("eksctl-test-cluster-nodegroup-12345"),
-							StackId: aws.String("eksctl-test-cluster-nodegroup-12345-id"),
+							StackName:   aws.String("eksctl-test-cluster-nodegroup-12345"),
+							StackId:     aws.String("eksctl-test-cluster-nodegroup-12345-id"),
+							StackStatus: aws.String("CREATE_COMPLETE"),
 						},
 					},
 				}, nil)
@@ -94,7 +96,7 @@ var _ = Describe("StackCollection NodeGroup", func() {
 				})
 
 				JustBeforeEach(func() {
-					out, err = sc.GetNodeGroupSummaries()
+					out, err = sc.GetNodeGroupSummaries("")
 				})
 
 				It("should not error", func() {
@@ -116,7 +118,7 @@ var _ = Describe("StackCollection NodeGroup", func() {
 				})
 
 				JustBeforeEach(func() {
-					out, err = sc.GetNodeGroupSummaries()
+					out, err = sc.GetNodeGroupSummaries("")
 				})
 
 				It("should not error", func() {