Refactor and increase test coverage for creating nodegroups

go.mod

@@ -54,7 +54,7 @@ require (
 	github.com/tomnomnom/linkheader v0.0.0-20180905144013-02ca5825eb80 // indirect
 	github.com/vektra/mockery v1.1.2
 	github.com/voxelbrain/goptions v0.0.0-20180630082107-58cddc247ea2 // indirect
-	github.com/weaveworks/goformation/v4 v4.10.2-0.20210512074147-4fd9b878d08c
+	github.com/weaveworks/goformation/v4 v4.10.2-0.20210512152024-6bd11e940025
 	github.com/weaveworks/launcher v0.0.2-0.20200715141516-1ca323f1de15
 	github.com/whilp/git-urls v0.0.0-20191001220047-6db9661140c0
 	golang.org/x/tools v0.1.0

go.sum

@@ -1412,8 +1412,8 @@ github.com/voxelbrain/goptions v0.0.0-20180630082107-58cddc247ea2 h1:txplJASvd6b
 github.com/voxelbrain/goptions v0.0.0-20180630082107-58cddc247ea2/go.mod h1:DGCIhurYgnLz8J9ga1fMV/fbLDyUvTyrWXVWUIyJon4=
 github.com/weaveworks/goformation/v4 v4.10.2-0.20210510110359-567e61bbcf4c h1:vFyc7sCxT0Pcj5wPH8uR3dudRe0MSCQXBTMolLmquCU=
 github.com/weaveworks/goformation/v4 v4.10.2-0.20210510110359-567e61bbcf4c/go.mod h1:x92o12+Azh6DQ4yoXT5oEuE7dhQHR5V2vy/fmZ6pO7k=
-github.com/weaveworks/goformation/v4 v4.10.2-0.20210512074147-4fd9b878d08c h1:Qh6SvtJbxURTa9nvljYGDxp5j8iVEF6za6LbkamD7Z8=
-github.com/weaveworks/goformation/v4 v4.10.2-0.20210512074147-4fd9b878d08c/go.mod h1:x92o12+Azh6DQ4yoXT5oEuE7dhQHR5V2vy/fmZ6pO7k=
+github.com/weaveworks/goformation/v4 v4.10.2-0.20210512152024-6bd11e940025 h1:XX7Ch1ZuYef1D+M7EOr2tKc/8Y7quheNQJK36SMFWqw=
+github.com/weaveworks/goformation/v4 v4.10.2-0.20210512152024-6bd11e940025/go.mod h1:x92o12+Azh6DQ4yoXT5oEuE7dhQHR5V2vy/fmZ6pO7k=
 github.com/weaveworks/launcher v0.0.2-0.20200715141516-1ca323f1de15 h1:i/RhLevywqC6cuUWtGdoaNrsJd+/zWh3PXbkXZIyZsU=
 github.com/weaveworks/launcher v0.0.2-0.20200715141516-1ca323f1de15/go.mod h1:w9Z1vnQmPobkEZ0F3oyiqRYP+62qDqTGnK6t5uhe1kg=
 github.com/weaveworks/mesh v0.0.0-20170419100114-1f158d31de55/go.mod h1:mcON9Ws1aW0crSErpXWp7U1ErCDEKliDX2OhVlbWRKk=

pkg/actions/cluster/owned.go

@@ -57,7 +57,8 @@ func (c *OwnedCluster) Upgrade(dryRun bool) error {
 		return err
 	}
 
-	if err := c.ctl.ValidateExistingNodeGroupsForCompatibility(c.cfg, c.stackManager); err != nil {
+	nodeGroupService := eks.NodeGroupService{Provider: c.ctl.Provider}
+	if err := nodeGroupService.ValidateExistingNodeGroupsForCompatibility(c.cfg, c.stackManager); err != nil {
 		logger.Critical("failed checking nodegroups", err.Error())
 	}
 

pkg/actions/nodegroup/create.go

@@ -5,7 +5,6 @@ import (
 	"os"
 	"strings"
 
-	"github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector"
 	"github.com/kris-nova/logger"
 	"github.com/pkg/errors"
 
@@ -20,7 +19,6 @@ import (
 	"github.com/weaveworks/eksctl/pkg/vpc"
 
 	api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
-	"github.com/weaveworks/eksctl/pkg/authconfigmap"
 	"github.com/weaveworks/eksctl/pkg/ctl/cmdutils"
 )
 
@@ -34,24 +32,22 @@ type CreateOpts struct {
 	ConfigFileProvided        bool
 }
 
-func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodeGroupFilter) error {
+func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodegroupFilter) error {
 	cfg := m.cfg
 	meta := cfg.Metadata
 	ctl := m.ctl
-
-	// For dry-run.
-	clusterConfigCopy := cfg.DeepCopy()
+	kubeProvider := m.kubeProvider
 
 	if err := checkVersion(ctl, meta); err != nil {
 		return err
 	}
 
-	if err := checkARMSupport(ctl, m.clientSet, cfg, options.SkipOutdatedAddonsCheck); err != nil {
+	if err := m.checkARMSupport(ctl, m.clientSet, cfg, options.SkipOutdatedAddonsCheck); err != nil {
 		return err
 	}
 
 	var isOwnedCluster = true
-	if err := ctl.LoadClusterIntoSpecFromStack(cfg, m.stackManager); err != nil {
+	if err := kubeProvider.LoadClusterIntoSpecFromStack(cfg, m.stackManager); err != nil {
 		switch e := err.(type) {
 		case *manager.StackNotFoundErr:
 			logger.Warning("%s, will attempt to create nodegroup(s) on non eksctl-managed cluster", e.Error())
@@ -66,7 +62,7 @@ func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodeGroupFil
 	}
 
 	// EKS 1.14 clusters created with prior versions of eksctl may not support Managed Nodes
-	supportsManagedNodes, err := ctl.SupportsManagedNodes(cfg)
+	supportsManagedNodes, err := kubeProvider.SupportsManagedNodes(cfg)
 	if err != nil {
 		return err
 	}
@@ -79,14 +75,15 @@ func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodeGroupFil
 		return err
 	}
 
-	nodeGroupService := eks.NewNodeGroupService(ctl.Provider, selector.New(ctl.Provider.Session()))
+	m.init.NewAWSSelectorSession(ctl.Provider)
 	nodePools := cmdutils.ToNodePools(cfg)
-	if err := nodeGroupService.ExpandInstanceSelectorOptions(nodePools, cfg.AvailabilityZones); err != nil {
+
+	if err := m.init.ExpandInstanceSelectorOptions(nodePools, cfg.AvailabilityZones); err != nil {
 		return err
 	}
 
 	if !options.DryRun {
-		if err := nodeGroupService.Normalize(nodePools, cfg.Metadata); err != nil {
+		if err := m.init.Normalize(nodePools, cfg.Metadata); err != nil {
 			return err
 		}
 	}
@@ -97,100 +94,102 @@ func (m *Manager) Create(options CreateOpts, nodegroupFilter filter.NodeGroupFil
 	}
 
 	if isOwnedCluster {
-		if err := ctl.ValidateClusterForCompatibility(cfg, m.stackManager); err != nil {
+		if err := kubeProvider.ValidateClusterForCompatibility(cfg, m.stackManager); err != nil {
 			return errors.Wrap(err, "cluster compatibility check failed")
 		}
 	}
 
-	if err := vpc.ValidateLegacySubnetsForNodeGroups(cfg, ctl.Provider); err != nil {
+	if err := m.init.ValidateLegacySubnetsForNodeGroups(cfg, ctl.Provider); err != nil {
 		return err
 	}
 
-	{
-		if err := nodegroupFilter.SetOnlyLocal(ctl.Provider.EKS(), m.stackManager, cfg); err != nil {
-			return err
-		}
+	if err := m.nodeCreationTasks(options, nodegroupFilter, supportsManagedNodes, isOwnedCluster); err != nil {
+		return err
+	}
 
-		logFiltered := cmdutils.ApplyFilter(cfg, &nodegroupFilter)
-		logFiltered()
-		logMsg := func(resource string, count int) {
-			logger.Info("will create a CloudFormation stack for each of %d %s in cluster %q", count, resource, meta.Name)
-		}
-		if len(cfg.NodeGroups) > 0 {
-			logMsg("nodegroups", len(cfg.NodeGroups))
-		}
+	if err := m.postNodeCreationTasks(m.clientSet, options); err != nil {
+		return err
+	}
 
-		if len(cfg.ManagedNodeGroups) > 0 {
-			logMsg("managed nodegroups", len(cfg.ManagedNodeGroups))
-		}
+	if err := m.init.ValidateExistingNodeGroupsForCompatibility(cfg, m.stackManager); err != nil {
+		logger.Critical("failed checking nodegroups", err.Error())
+	}
 
-		if options.DryRun {
-			// Set filtered nodegroups
-			clusterConfigCopy.NodeGroups = cfg.NodeGroups
-			clusterConfigCopy.ManagedNodeGroups = cfg.ManagedNodeGroups
-			if options.ConfigFileProvided {
-				return cmdutils.PrintDryRunConfig(clusterConfigCopy, os.Stdout)
-			}
-			return cmdutils.PrintNodeGroupDryRunConfig(clusterConfigCopy, os.Stdout)
-		}
+	return nil
+}
 
-		taskTree := &tasks.TaskTree{
-			Parallel: false,
-		}
+func (m *Manager) nodeCreationTasks(options CreateOpts, nodegroupFilter filter.NodegroupFilter, supportsManagedNodes, isOwnedCluster bool) error {
+	cfg := m.cfg
+	meta := cfg.Metadata
+	init := m.init
 
-		if supportsManagedNodes && isOwnedCluster {
-			taskTree.Append(m.stackManager.NewClusterCompatTask())
-		}
+	if err := nodegroupFilter.SetOnlyLocal(m.ctl.Provider.EKS(), m.stackManager, cfg); err != nil {
+		return err
+	}
 
-		awsNodeUsesIRSA, err := eks.DoesAWSNodeUseIRSA(ctl.Provider, m.clientSet)
-		if err != nil {
-			return errors.Wrap(err, "couldn't check aws-node for annotation")
-		}
+	logFiltered := cmdutils.ApplyFilter(cfg, nodegroupFilter)
+	logFiltered()
+	logMsg := func(resource string, count int) {
+		logger.Info("will create a CloudFormation stack for each of %d %s in cluster %q", count, resource, meta.Name)
+	}
+	if len(m.cfg.NodeGroups) > 0 {
+		logMsg("nodegroups", len(cfg.NodeGroups))
+	}
 
-		if !awsNodeUsesIRSA && api.IsEnabled(cfg.IAM.WithOIDC) {
-			logger.Debug("cluster has withOIDC enabled but is not using IRSA for CNI, will add CNI policy to node role")
-		}
+	if len(m.cfg.ManagedNodeGroups) > 0 {
+		logMsg("managed nodegroups", len(cfg.ManagedNodeGroups))
+	}
 
-		var vpcImporter vpc.Importer
-		if isOwnedCluster {
-			vpcImporter = vpc.NewStackConfigImporter(m.stackManager.MakeClusterStackName())
-		} else {
-			vpcImporter = vpc.NewSpecConfigImporter(*ctl.Status.ClusterInfo.Cluster.ResourcesVpcConfig.ClusterSecurityGroupId, cfg.VPC)
+	if options.DryRun {
+		clusterConfigCopy := cfg.DeepCopy()
+		// Set filtered nodegroups
+		clusterConfigCopy.NodeGroups = cfg.NodeGroups
+		clusterConfigCopy.ManagedNodeGroups = cfg.ManagedNodeGroups
+		if options.ConfigFileProvided {
+			return cmdutils.PrintDryRunConfig(clusterConfigCopy, os.Stdout)
 		}
+		return cmdutils.PrintNodeGroupDryRunConfig(clusterConfigCopy, os.Stdout)
+	}
 
-		allNodeGroupTasks := &tasks.TaskTree{
-			Parallel: true,
-		}
-		nodeGroupTasks := m.stackManager.NewUnmanagedNodeGroupTask(cfg.NodeGroups, supportsManagedNodes, !awsNodeUsesIRSA, vpcImporter)
-		if nodeGroupTasks.Len() > 0 {
-			allNodeGroupTasks.Append(nodeGroupTasks)
-		}
-		managedTasks := m.stackManager.NewManagedNodeGroupTask(cfg.ManagedNodeGroups, !awsNodeUsesIRSA, vpcImporter)
-		if managedTasks.Len() > 0 {
-			allNodeGroupTasks.Append(managedTasks)
-		}
+	taskTree := &tasks.TaskTree{
+		Parallel: false,
+	}
 
-		taskTree.Append(allNodeGroupTasks)
-		logger.Info(taskTree.Describe())
-		errs := taskTree.DoAllSync()
-		if len(errs) > 0 {
-			logger.Info("%d error(s) occurred and nodegroups haven't been created properly, you may wish to check CloudFormation console", len(errs))
-			logger.Info("to cleanup resources, run 'eksctl delete nodegroup --region=%s --cluster=%s --name=<name>' for each of the failed nodegroup", meta.Region, meta.Name)
-			for _, err := range errs {
-				if err != nil {
-					logger.Critical("%s\n", err.Error())
-				}
-			}
-			return fmt.Errorf("failed to create nodegroups for cluster %q", meta.Name)
-		}
+	if supportsManagedNodes && isOwnedCluster {
+		taskTree.Append(m.stackManager.NewClusterCompatTask())
 	}
 
-	if err := m.postNodeCreationTasks(m.clientSet, options); err != nil {
-		return err
+	awsNodeUsesIRSA, err := init.DoesAWSNodeUseIRSA(m.ctl.Provider, m.clientSet)
+	if err != nil {
+		return errors.Wrap(err, "couldn't check aws-node for annotation")
 	}
 
-	if err := ctl.ValidateExistingNodeGroupsForCompatibility(cfg, m.stackManager); err != nil {
-		logger.Critical("failed checking nodegroups", err.Error())
+	if !awsNodeUsesIRSA && api.IsEnabled(cfg.IAM.WithOIDC) {
+		logger.Debug("cluster has withOIDC enabled but is not using IRSA for CNI, will add CNI policy to node role")
+	}
+
+	var vpcImporter vpc.Importer
+	if isOwnedCluster {
+		vpcImporter = vpc.NewStackConfigImporter(m.stackManager.MakeClusterStackName())
+	} else {
+		vpcImporter = vpc.NewSpecConfigImporter(*m.ctl.Status.ClusterInfo.Cluster.ResourcesVpcConfig.ClusterSecurityGroupId, cfg.VPC)
+	}
+
+	allNodeGroupTasks := &tasks.TaskTree{
+		Parallel: true,
+	}
+	nodeGroupTasks := m.stackManager.NewUnmanagedNodeGroupTask(cfg.NodeGroups, supportsManagedNodes, !awsNodeUsesIRSA, vpcImporter)
+	if nodeGroupTasks.Len() > 0 {
+		allNodeGroupTasks.Append(nodeGroupTasks)
+	}
+	managedTasks := m.stackManager.NewManagedNodeGroupTask(cfg.ManagedNodeGroups, !awsNodeUsesIRSA, vpcImporter)
+	if managedTasks.Len() > 0 {
+		allNodeGroupTasks.Append(managedTasks)
+	}
+
+	taskTree.Append(allNodeGroupTasks)
+	if err := m.init.DoAllNodegroupStackTasks(taskTree, meta.Region, meta.Name); err != nil {
+		return err
 	}
 
 	return nil
@@ -212,22 +211,14 @@ func (m *Manager) postNodeCreationTasks(clientSet kubernetes.Interface, options
 	}
 
 	if options.UpdateAuthConfigMap {
-		for _, ng := range m.cfg.NodeGroups {
-			// authorise nodes to join
-			if err := authconfigmap.AddNodeGroup(clientSet, ng); err != nil {
-				return err
-			}
-
-			// wait for nodes to join
-			if err := m.ctl.WaitForNodes(clientSet, ng); err != nil {
-				return err
-			}
+		if err := m.kubeProvider.UpdateAuthConfigMap(m.cfg.NodeGroups, clientSet); err != nil {
+			return err
 		}
 	}
 	logger.Success("created %d nodegroup(s) in cluster %q", len(m.cfg.NodeGroups), m.cfg.Metadata.Name)
 
 	for _, ng := range m.cfg.ManagedNodeGroups {
-		if err := m.ctl.WaitForNodes(clientSet, ng); err != nil {
+		if err := m.kubeProvider.WaitForNodes(clientSet, ng); err != nil {
 			if m.cfg.PrivateCluster.Enabled {
 				logger.Info("error waiting for nodes to join the cluster; this command was likely run from outside the cluster's VPC as the API server is not reachable, nodegroup(s) should still be able to join the cluster, underlying error is: %v", err)
 				break
@@ -275,13 +266,14 @@ func checkVersion(ctl *eks.ClusterProvider, meta *api.ClusterMeta) error {
 	return nil
 }
 
-func checkARMSupport(ctl *eks.ClusterProvider, clientSet kubernetes.Interface, cfg *api.ClusterConfig, skipOutdatedAddonsCheck bool) error {
-	rawClient, err := ctl.NewRawClient(cfg)
+func (m *Manager) checkARMSupport(ctl *eks.ClusterProvider, clientSet kubernetes.Interface, cfg *api.ClusterConfig, skipOutdatedAddonsCheck bool) error {
+	kubeProvider := m.kubeProvider
+	rawClient, err := kubeProvider.NewRawClient(cfg)
 	if err != nil {
 		return err
 	}
 
-	kubernetesVersion, err := rawClient.ServerVersion()
+	kubernetesVersion, err := kubeProvider.ServerVersion(rawClient)
 	if err != nil {
 		return err
 	}