Standardize cluster / nodegroup version before doing validations and …

…setting defaults (#5946)

pkg/actions/nodegroup/create.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"strings"
 
 	"github.com/kris-nova/logger"
 	"github.com/pkg/errors"
@@ -40,10 +39,6 @@ func (m *Manager) Create(ctx context.Context, options CreateOpts, nodegroupFilte
 	meta := cfg.Metadata
 	ctl := m.ctl
 
-	if err := checkVersion(ctl, meta); err != nil {
-		return err
-	}
-
 	if cfg.IsControlPlaneOnOutposts() && len(cfg.ManagedNodeGroups) > 0 {
 		const msg = "Managed Nodegroups are not supported on Outposts"
 		if !options.ConfigFileProvided {
@@ -287,40 +282,6 @@ func (m *Manager) postNodeCreationTasks(ctx context.Context, clientSet kubernete
 	return nil
 }
 
-func checkVersion(ctl *eks.ClusterProvider, meta *api.ClusterMeta) error {
-	switch meta.Version {
-	case "auto":
-		break
-	case "":
-		meta.Version = "auto"
-	case "default":
-		meta.Version = api.DefaultVersion
-		logger.Info("will use default version (%s) for new nodegroup(s)", meta.Version)
-	case "latest":
-		meta.Version = api.LatestVersion
-		logger.Info("will use latest version (%s) for new nodegroup(s)", meta.Version)
-	default:
-		if !api.IsSupportedVersion(meta.Version) {
-			if api.IsDeprecatedVersion(meta.Version) {
-				return fmt.Errorf("invalid version, %s is no longer supported, supported values: auto, default, latest, %s\nsee also: https://docs.aws.amazon.com/eks/latest/userguide/kubernetes-versions.html", meta.Version, strings.Join(api.SupportedVersions(), ", "))
-			}
-			return fmt.Errorf("invalid version %s, supported values: auto, default, latest, %s", meta.Version, strings.Join(api.SupportedVersions(), ", "))
-		}
-	}
-
-	if v := ctl.ControlPlaneVersion(); v == "" {
-		return fmt.Errorf("unable to get control plane version")
-	} else if meta.Version == "auto" {
-		meta.Version = v
-		logger.Info("will use version %s for new nodegroup(s) based on control plane version", meta.Version)
-	} else if meta.Version != v {
-		hint := "--version=auto"
-		logger.Warning("will use version %s for new nodegroup(s), while control plane version is %s; to automatically inherit the version use %q", meta.Version, v, hint)
-	}
-
-	return nil
-}
-
 func (m *Manager) checkARMSupport(ctx context.Context, ctl *eks.ClusterProvider, rawClient *kubernetes.RawClient, cfg *api.ClusterConfig, skipOutdatedAddonsCheck bool) error {
 	kubeProvider := m.ctl
 	kubernetesVersion, err := kubeProvider.ServerVersion(rawClient)

pkg/actions/nodegroup/create_test.go

@@ -3,7 +3,6 @@ package nodegroup_test
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	"github.com/aws/aws-sdk-go-v2/aws"
 	"github.com/aws/aws-sdk-go-v2/service/cloudformation"
@@ -120,11 +119,6 @@ var _ = DescribeTable("Create", func(t ngEntry) {
 		t.expectedCalls(k, &ngFilter)
 	}
 },
-	Entry("fails when cluster version is not supported", ngEntry{
-		version:     "1.14",
-		expectedErr: fmt.Errorf("invalid version, %s is no longer supported, supported values: auto, default, latest, %s\nsee also: https://docs.aws.amazon.com/eks/latest/userguide/kubernetes-versions.html", "1.14", strings.Join(api.SupportedVersions(), ", ")),
-	}),
-
 	Entry("when cluster is unowned, fails to load VPC from config if config is not supplied", ngEntry{
 		mockCalls: func(k *fakes.FakeKubeProvider, f *utilFakes.FakeNodegroupFilter, p *mockprovider.MockProvider, _ *fake.Clientset) {
 			k.NewRawClientReturns(&kubernetes.RawClient{}, nil)

pkg/apis/eksctl.io/v1alpha5/defaults.go

@@ -232,23 +232,21 @@ func getDefaultVolumeType(nodeGroupOnOutposts bool) string {
 }
 
 func setContainerRuntimeDefault(ng *NodeGroup, clusterVersion string) {
-	var runtime string
 	if ng.ContainerRuntime != nil {
 		return
 	}
 
+	// since clusterVersion is standardised beforehand, we can safely ignore the error
 	isDockershimDeprecated, _ := utils.IsMinVersion(DockershimDeprecationVersion, clusterVersion)
 
 	if isDockershimDeprecated {
-		runtime = ContainerRuntimeContainerD
+		ng.ContainerRuntime = aws.String(ContainerRuntimeContainerD)
 	} else {
-		runtime = ContainerRuntimeDockerD
+		ng.ContainerRuntime = aws.String(ContainerRuntimeDockerD)
 		if IsWindowsImage(ng.AMIFamily) {
-			runtime = ContainerRuntimeDockerForWindows
+			ng.ContainerRuntime = aws.String(ContainerRuntimeDockerForWindows)
 		}
 	}
-
-	ng.ContainerRuntime = &runtime
 }
 
 func setIAMDefaults(iamConfig *NodeGroupIAM) {

pkg/ctl/cmdutils/cmd.go

@@ -84,6 +84,13 @@ func (c *Cmd) InitializeClusterConfig() error {
 // NewProviderForExistingCluster is a wrapper for NewCtl that also validates that the cluster exists and is not a
 // registered/connected cluster.
 func (c *Cmd) NewProviderForExistingCluster(ctx context.Context) (*eks.ClusterProvider, error) {
+	return c.NewProviderForExistingClusterHelper(ctx, func(ctl *eks.ClusterProvider, meta *api.ClusterMeta) error {
+		return nil
+	})
+}
+
+// NewProviderForExistingClusterHelper allows formating cluster K8s version to a standard value before doing nodegroup validations and initializations
+func (c *Cmd) NewProviderForExistingClusterHelper(ctx context.Context, standardizeClusterVersionFormat func(ctl *eks.ClusterProvider, meta *api.ClusterMeta) error) (*eks.ClusterProvider, error) {
 	clusterProvider, err := eks.New(ctx, &c.ProviderConfig, c.ClusterConfig)
 	if err != nil {
 		return nil, fmt.Errorf("could not create cluster provider from options: %w", err)
@@ -95,6 +102,10 @@ func (c *Cmd) NewProviderForExistingCluster(ctx context.Context) (*eks.ClusterPr
 		return nil, err
 	}
 
+	if err := standardizeClusterVersionFormat(clusterProvider, c.ClusterConfig.Metadata); err != nil {
+		return nil, err
+	}
+
 	if err := c.InitializeClusterConfig(); err != nil {
 		return nil, err
 	}

pkg/ctl/create/cluster.go

@@ -60,6 +60,23 @@ func createClusterCmd(cmd *cmdutils.Cmd) {
 	})
 }
 
+func checkClusterVersion(cfg *api.ClusterConfig) error {
+	switch cfg.Metadata.Version {
+	case "auto":
+		cfg.Metadata.Version = api.DefaultVersion
+	case "latest":
+		cfg.Metadata.Version = api.LatestVersion
+	}
+
+	if err := api.ValidateClusterVersion(cfg); err != nil {
+		return err
+	}
+	if cfg.Metadata.Version == "" {
+		cfg.Metadata.Version = api.DefaultVersion
+	}
+	return nil
+}
+
 func createClusterCmdWithRunFunc(cmd *cmdutils.Cmd, runFunc func(cmd *cmdutils.Cmd, ngFilter *filter.NodeGroupFilter, params *cmdutils.CreateClusterCmdParams) error) {
 	cfg := api.NewClusterConfig()
 	ng := api.NewNodeGroup()
@@ -75,6 +92,10 @@ func createClusterCmdWithRunFunc(cmd *cmdutils.Cmd, runFunc func(cmd *cmdutils.C
 		if err := cmdutils.NewCreateClusterLoader(cmd, ngFilter, ng, params).Load(); err != nil {
 			return err
 		}
+		err := checkClusterVersion(cmd.ClusterConfig)
+		if err != nil {
+			return err
+		}
 		return runFunc(cmd, ngFilter, params)
 	}
 
@@ -150,20 +171,6 @@ func doCreateCluster(cmd *cmdutils.Cmd, ngFilter *filter.NodeGroupFilter, params
 		cmdutils.LogRegionAndVersionInfo(meta)
 	})
 
-	switch cfg.Metadata.Version {
-	case "auto":
-		cfg.Metadata.Version = api.DefaultVersion
-	case "latest":
-		cfg.Metadata.Version = api.LatestVersion
-	}
-
-	if err := api.ValidateClusterVersion(cfg); err != nil {
-		return err
-	}
-	if cfg.Metadata.Version == "" {
-		cfg.Metadata.Version = api.DefaultVersion
-	}
-
 	if err := cfg.ValidatePrivateCluster(); err != nil {
 		return err
 	}

pkg/ctl/create/cluster_test.go

@@ -22,6 +22,7 @@ import (
 	api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
 	"github.com/weaveworks/eksctl/pkg/ctl/cmdutils"
 	"github.com/weaveworks/eksctl/pkg/ctl/cmdutils/filter"
+	"github.com/weaveworks/eksctl/pkg/ctl/ctltest"
 	"github.com/weaveworks/eksctl/pkg/eks"
 	"github.com/weaveworks/eksctl/pkg/eks/fakes"
 	"github.com/weaveworks/eksctl/pkg/kubernetes"
@@ -212,6 +213,50 @@ var _ = Describe("create cluster", func() {
 		expectedErr string
 	}
 
+	Describe("[Outposts] cluster version issues", func() {
+		Describe("version not set", func() {
+			It("should return an error", func() {
+				cfg := &api.ClusterConfig{
+					TypeMeta: api.ClusterConfigTypeMeta(),
+					Metadata: &api.ClusterMeta{
+						Name:    "cluster-1",
+						Region:  "us-west-2",
+						Version: "",
+					},
+					Outpost: &api.Outpost{
+						ControlPlaneOutpostARN: "arn:aws:outposts:us-west-2:1234:outpost/op-1234",
+					},
+				}
+
+				cmd := newDefaultCmd("cluster", "--config-file", ctltest.CreateConfigFile(cfg))
+				_, err := cmd.execute()
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(MatchError(ContainSubstring("cluster version must be explicitly set to 1.21 for Outposts clusters as only version 1.21 is currently supported")))
+			})
+		})
+
+		Describe("version set to unsupported version", func() {
+			It("should return an error", func() {
+				cfg := &api.ClusterConfig{
+					TypeMeta: api.ClusterConfigTypeMeta(),
+					Metadata: &api.ClusterMeta{
+						Name:    "cluster-1",
+						Region:  "us-west-2",
+						Version: "1.20",
+					},
+					Outpost: &api.Outpost{
+						ControlPlaneOutpostARN: "arn:aws:outposts:us-west-2:1234:outpost/op-1234",
+					},
+				}
+
+				cmd := newDefaultCmd("cluster", "--config-file", ctltest.CreateConfigFile(cfg))
+				_, err := cmd.execute()
+				Expect(err).To(HaveOccurred())
+				Expect(err).To(MatchError(ContainSubstring("only version 1.21 is supported on Outposts")))
+			})
+		})
+	})
+
 	DescribeTable("doCreateCluster", func(ce createClusterEntry) {
 		p := mockprovider.NewMockProvider()
 		defaultProviderMocks(p, defaultOutput, ce.mockOutposts)
@@ -270,30 +315,6 @@ var _ = Describe("create cluster", func() {
 	},
 		Entry("standard cluster", createClusterEntry{}),
 
-		Entry("[Outposts] cluster version not set", createClusterEntry{
-			updateClusterConfig: func(c *api.ClusterConfig) {
-				c.Metadata.Version = ""
-				c.Outpost = &api.Outpost{
-					ControlPlaneOutpostARN: "arn:aws:outposts:us-west-2:1234:outpost/op-1234",
-				}
-			},
-			mockOutposts: true,
-
-			expectedErr: "cluster version must be explicitly set to 1.21 for Outposts clusters as only version 1.21 is currently supported",
-		}),
-
-		Entry("[Outposts] cluster version set to an unsupported version", createClusterEntry{
-			updateClusterConfig: func(c *api.ClusterConfig) {
-				c.Metadata.Version = api.Version1_22
-				c.Outpost = &api.Outpost{
-					ControlPlaneOutpostARN: "arn:aws:outposts:us-west-2:1234:outpost/op-1234",
-				}
-			},
-			mockOutposts: true,
-
-			expectedErr: "only version 1.21 is supported on Outposts",
-		}),
-
 		Entry("[Outposts] control plane on Outposts with valid config", createClusterEntry{
 			updateClusterConfig: func(c *api.ClusterConfig) {
 				c.Metadata.Version = api.Version1_21

pkg/ctl/create/nodegroup.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"strings"
 
 	"github.com/aws/amazon-ec2-instance-selector/v2/pkg/selector"
 
@@ -16,6 +17,7 @@ import (
 	api "github.com/weaveworks/eksctl/pkg/apis/eksctl.io/v1alpha5"
 	"github.com/weaveworks/eksctl/pkg/ctl/cmdutils"
 	"github.com/weaveworks/eksctl/pkg/ctl/cmdutils/filter"
+	"github.com/weaveworks/eksctl/pkg/eks"
 	"github.com/weaveworks/eksctl/pkg/utils/names"
 )
 
@@ -51,7 +53,7 @@ func createNodeGroupCmd(cmd *cmdutils.Cmd) {
 		}
 
 		ctx := context.Background()
-		ctl, err := cmd.NewProviderForExistingCluster(ctx)
+		ctl, err := cmd.NewProviderForExistingClusterHelper(ctx, checkNodeGroupVersion)
 		if err != nil {
 			return fmt.Errorf("could not create cluster provider from options: %w", err)
 		}
@@ -123,3 +125,37 @@ func createNodeGroupCmdWithRunFunc(cmd *cmdutils.Cmd, runFunc runFn) {
 
 	cmdutils.AddCommonFlagsForAWS(cmd, &cmd.ProviderConfig, true)
 }
+
+func checkNodeGroupVersion(ctl *eks.ClusterProvider, meta *api.ClusterMeta) error {
+	switch meta.Version {
+	case "auto":
+		break
+	case "":
+		meta.Version = "auto"
+	case "default":
+		meta.Version = api.DefaultVersion
+		logger.Info("will use default version (%s) for new nodegroup(s)", meta.Version)
+	case "latest":
+		meta.Version = api.LatestVersion
+		logger.Info("will use latest version (%s) for new nodegroup(s)", meta.Version)
+	default:
+		if !api.IsSupportedVersion(meta.Version) {
+			if api.IsDeprecatedVersion(meta.Version) {
+				return fmt.Errorf("invalid version, %s is no longer supported, supported values: auto, default, latest, %s\nsee also: https://docs.aws.amazon.com/eks/latest/userguide/kubernetes-versions.html", meta.Version, strings.Join(api.SupportedVersions(), ", "))
+			}
+			return fmt.Errorf("invalid version %s, supported values: auto, default, latest, %s", meta.Version, strings.Join(api.SupportedVersions(), ", "))
+		}
+	}
+
+	if v := ctl.ControlPlaneVersion(); v == "" {
+		return fmt.Errorf("unable to get control plane version")
+	} else if meta.Version == "auto" {
+		meta.Version = v
+		logger.Info("will use version %s for new nodegroup(s) based on control plane version", meta.Version)
+	} else if meta.Version != v {
+		hint := "--version=auto"
+		logger.Warning("will use version %s for new nodegroup(s), while control plane version is %s; to automatically inherit the version use %q", meta.Version, v, hint)
+	}
+
+	return nil
+}