Update command can update multiple nodegroups (#3914)

`Update` command can update multiple nodegroups * Add unit tests for updating multiple ng * Unpend updateConfig integration test * Remove validation because UpdateConfig is never nil * Refactor Update ng into private funcs
eksctl-io · Jul 1, 2021 · 6ffc878 · 6ffc878
2 parents d7813e5 + ef534c7
commit 6ffc878
Show file tree

Hide file tree

Showing 4 changed files with 107 additions and 31 deletions.
diff --git a/integration/tests/managed/managed_nodegroup_test.go b/integration/tests/managed/managed_nodegroup_test.go
@@ -423,7 +423,7 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() {
 		})
 
 		Context("and creating a nodegroup with an update config", func() {
-			PIt("defining the UpdateConfig field in the cluster config", func() {
+			It("defining the UpdateConfig field in the cluster config", func() {
 				By("creating it")
 				updateConfig := &api.NodeGroupUpdateConfig{
 					MaxUnavailable: aws.Int(2),

diff --git a/pkg/actions/nodegroup/update.go b/pkg/actions/nodegroup/update.go
@@ -6,14 +6,24 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/eks"
 	"github.com/kris-nova/logger"
-	"github.com/pkg/errors"
 
+	api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
 	"github.com/weaveworks/eksctl/pkg/managed"
 )
 
 func (m *Manager) Update() error {
-	ng := m.cfg.ManagedNodeGroups[0]
-	describeNodegroupOutput, err := m.ctl.Provider.EKS().DescribeNodegroup(&eks.DescribeNodegroupInput{
+	for _, ng := range m.cfg.ManagedNodeGroups {
+		if err := m.updateNodegroup(ng); err != nil {
+			return err
+		}
+	}
+	return nil
+}
+
+func (m *Manager) updateNodegroup(ng *api.ManagedNodeGroup) error {
+	logger.Info("checking that nodegroup %s is a managed nodegroup", ng.Name)
+
+	_, err := m.ctl.Provider.EKS().DescribeNodegroup(&eks.DescribeNodegroupInput{
 		ClusterName:   &m.cfg.Metadata.Name,
 		NodegroupName: &ng.Name,
 	})
@@ -26,22 +36,12 @@ func (m *Manager) Update() error {
 	}
 
 	if ng.UpdateConfig == nil {
-		return fmt.Errorf("the submitted config didn't contain changes for nodegroup %s", ng.Name)
-	}
-
-	if describeNodegroupOutput.Nodegroup.UpdateConfig == nil {
-		return errors.New("cannot update updateConfig because the nodegroup is not configured to use one")
+		return fmt.Errorf("the submitted config does not contain any changes for nodegroup %s", ng.Name)
 	}
 
-	logger.Info("updating nodegroup %s's UpdateConfig", ng.Name)
-	updateConfig := &eks.NodegroupUpdateConfig{}
-
-	if ng.UpdateConfig.MaxUnavailable != nil {
-		updateConfig.MaxUnavailable = aws.Int64(int64(*ng.UpdateConfig.MaxUnavailable))
-	}
-
-	if ng.UpdateConfig.MaxUnavailablePercentage != nil {
-		updateConfig.MaxUnavailablePercentage = aws.Int64(int64(*ng.UpdateConfig.MaxUnavailablePercentage))
+	updateConfig, err := updateUpdateConfig(ng)
+	if err != nil {
+		return err
 	}
 
 	_, err = m.ctl.Provider.EKS().UpdateNodegroupConfig(&eks.UpdateNodegroupConfigInput{
@@ -56,3 +56,18 @@ func (m *Manager) Update() error {
 	logger.Info("nodegroup %s successfully updated", ng.Name)
 	return nil
 }
+
+func updateUpdateConfig(ng *api.ManagedNodeGroup) (*eks.NodegroupUpdateConfig, error) {
+	logger.Info("updating nodegroup %s's UpdateConfig", ng.Name)
+	updateConfig := &eks.NodegroupUpdateConfig{}
+
+	if ng.UpdateConfig.MaxUnavailable != nil {
+		updateConfig.MaxUnavailable = aws.Int64(int64(*ng.UpdateConfig.MaxUnavailable))
+	}
+
+	if ng.UpdateConfig.MaxUnavailablePercentage != nil {
+		updateConfig.MaxUnavailablePercentage = aws.Int64(int64(*ng.UpdateConfig.MaxUnavailablePercentage))
+	}
+
+	return updateConfig, nil
+}
diff --git a/pkg/actions/nodegroup/update_test.go b/pkg/actions/nodegroup/update_test.go
@@ -49,7 +49,7 @@ var _ = Describe("Update", func() {
 		Expect(err).To(MatchError(ContainSubstring("could not find managed nodegroup with name \"my-ng\"")))
 	})
 
-	It("[happy path] successfully updates nodegroup with updateConfig and maxUnavailable", func() {
+	It("[happy path] successfully updates a nodegroup with updateConfig and maxUnavailable", func() {
 		p.MockEKS().On("DescribeNodegroup", &awseks.DescribeNodegroupInput{
 			ClusterName:   &clusterName,
 			NodegroupName: &ngName,
@@ -77,4 +77,63 @@ var _ = Describe("Update", func() {
 		err := m.Update()
 		Expect(err).NotTo(HaveOccurred())
 	})
+
+	It("[happy path] successfully updates multiple nodegroups with updateConfig and maxUnavailable", func() {
+		p.MockEKS().On("DescribeNodegroup", &awseks.DescribeNodegroupInput{
+			ClusterName:   &clusterName,
+			NodegroupName: &ngName,
+		}).Return(&awseks.DescribeNodegroupOutput{
+			Nodegroup: &awseks.Nodegroup{
+				UpdateConfig: &awseks.NodegroupUpdateConfig{
+					MaxUnavailable: aws.Int64(4),
+				},
+			},
+		}, nil)
+
+		p.MockEKS().On("DescribeNodegroup", &awseks.DescribeNodegroupInput{
+			ClusterName:   &clusterName,
+			NodegroupName: aws.String("ng-2"),
+		}).Return(&awseks.DescribeNodegroupOutput{
+			Nodegroup: &awseks.Nodegroup{
+				UpdateConfig: &awseks.NodegroupUpdateConfig{
+					MaxUnavailable: aws.Int64(4),
+				},
+			},
+		}, nil)
+
+		p.MockEKS().On("UpdateNodegroupConfig", &awseks.UpdateNodegroupConfigInput{
+			UpdateConfig: &awseks.NodegroupUpdateConfig{
+				MaxUnavailable: aws.Int64(6),
+			},
+			ClusterName:   &clusterName,
+			NodegroupName: &ngName,
+		}).Return(nil, nil)
+
+		p.MockEKS().On("UpdateNodegroupConfig", &awseks.UpdateNodegroupConfigInput{
+			UpdateConfig: &awseks.NodegroupUpdateConfig{
+				MaxUnavailable: aws.Int64(6),
+			},
+			ClusterName:   &clusterName,
+			NodegroupName: aws.String("ng-2"),
+		}).Return(nil, nil)
+
+		cfg.ManagedNodeGroups[0].UpdateConfig = &api.NodeGroupUpdateConfig{
+			MaxUnavailable: aws.Int(6),
+		}
+
+		newNg := &api.ManagedNodeGroup{
+			NodeGroupBase: &api.NodeGroupBase{
+				Name: "ng-2",
+			},
+			UpdateConfig: &api.NodeGroupUpdateConfig{
+				MaxUnavailable: aws.Int(6),
+			},
+		}
+
+		cfg.ManagedNodeGroups = append(cfg.ManagedNodeGroups, newNg)
+
+		m = New(cfg, &eks.ClusterProvider{Provider: p}, nil)
+		err := m.Update()
+		Expect(err).NotTo(HaveOccurred())
+	})
 })
diff --git a/pkg/ctl/cmdutils/configfile.go b/pkg/ctl/cmdutils/configfile.go
@@ -750,22 +750,24 @@ func NewUpdateNodegroupLoader(cmd *Cmd) ClusterConfigLoader {
 			return ErrMustBeSet("managedNodeGroups field")
 		}
 
-		logger.Info("validating nodegroup %q", l.ClusterConfig.ManagedNodeGroups[0].Name)
+		for _, ng := range l.ClusterConfig.ManagedNodeGroups {
+			logger.Info("validating nodegroup %q", ng.Name)
 
-		var unsupportedFields []string
-		ng := l.ClusterConfig.ManagedNodeGroups[0]
-		var err error
-		if unsupportedFields, err = validateSupportedConfigFields(*ng.NodeGroupBase, []string{"Name"}, unsupportedFields); err != nil {
-			return err
-		}
+			var unsupportedFields []string
+			var err error
+			if unsupportedFields, err = validateSupportedConfigFields(*ng.NodeGroupBase, []string{"Name"}, unsupportedFields); err != nil {
+				return err
+			}
 
-		if unsupportedFields, err = validateSupportedConfigFields(*ng, []string{"NodeGroupBase", "UpdateConfig"}, unsupportedFields); err != nil {
-			return err
-		}
+			if unsupportedFields, err = validateSupportedConfigFields(*ng, []string{"NodeGroupBase", "UpdateConfig"}, unsupportedFields); err != nil {
+				return err
+			}
 
-		if len(unsupportedFields) > 0 {
-			logger.Warning("unchanged fields: the following fields remain unchanged; they are not supported by `eksctl update nodegroup`: %s", strings.Join(unsupportedFields[:], ", "))
+			if len(unsupportedFields) > 0 {
+				logger.Warning("unchanged fields for nodegroup %s: the following fields remain unchanged; they are not supported by `eksctl update nodegroup`: %s", ng.Name, strings.Join(unsupportedFields[:], ", "))
+			}
 		}
+
 		return nil
 	}