only allow unmanaged nodegroups to add container runtime configuration

eksctl-io · Aug 5, 2021 · 9fdcb98 · 9fdcb98
1 parent cbe00f6
commit 9fdcb98
Show file tree

Hide file tree

Showing 13 changed files with 139 additions and 79 deletions.
diff --git a/integration/tests/crud/creategetdelete_test.go b/integration/tests/crud/creategetdelete_test.go
@@ -64,6 +64,13 @@ var _ = Describe("(Integration) Create, Get, Scale & Delete", func() {
 	)
 
 	commonTimeout := 10 * time.Minute
+	makeClusterConfig := func() *api.ClusterConfig {
+		clusterConfig := api.NewClusterConfig()
+		clusterConfig.Metadata.Name = params.ClusterName
+		clusterConfig.Metadata.Region = params.Region
+		clusterConfig.Metadata.Version = params.Version
+		return clusterConfig
+	}
 
 	BeforeSuite(func() {
 		params.KubeconfigTemp = false
@@ -387,6 +394,52 @@ var _ = Describe("(Integration) Create, Get, Scale & Delete", func() {
 			})
 		})
 
+		Context("and creating a nodegroup with containerd runtime", func() {
+			var (
+				nodegroupName string
+			)
+			BeforeEach(func() {
+				nodegroupName = "test-containerd"
+			})
+			AfterEach(func() {
+				cmd := params.EksctlDeleteCmd.WithArgs(
+					"nodegroup",
+					"--verbose", "4",
+					"--cluster", params.ClusterName,
+					"--wait",
+					nodegroupName,
+				)
+				Expect(cmd).To(RunSuccessfully())
+			})
+			It("should create the nodegroup without problems", func() {
+				clusterConfig := makeClusterConfig()
+				clusterConfig.NodeGroups = []*api.NodeGroup{
+					{
+						NodeGroupBase: &api.NodeGroupBase{
+							Name:      "test-containerd",
+							AMIFamily: api.NodeImageFamilyAmazonLinux2,
+							SSH: &api.NodeGroupSSH{
+								Allow:     api.Disabled(),
+								EnableSSM: api.Disabled(),
+							},
+							InstanceType: "p2.xlarge",
+						},
+						ContainerRuntime: aws.String(api.ContainerRuntimeContainerD),
+					},
+				}
+
+				cmd := params.EksctlCreateCmd.
+					WithArgs(
+						"nodegroup",
+						"--config-file", "-",
+						"--verbose", "4",
+					).
+					WithoutArg("--region", params.Region).
+					WithStdin(testutils.ClusterConfigReader(clusterConfig))
+				Expect(cmd).To(RunSuccessfully())
+			})
+		})
+
 		Context("and add a second (GPU) nodegroup", func() {
 			It("should not return an error", func() {
 				cmd := params.EksctlCreateCmd.WithArgs(

diff --git a/integration/tests/managed/managed_nodegroup_test.go b/integration/tests/managed/managed_nodegroup_test.go
@@ -550,31 +550,6 @@ var _ = Describe("(Integration) Create Managed Nodegroups", func() {
 			})
 		})
 
-		Context("and creating a nodegroup with containerd runtime", func() {
-			It("should pass that on to the container bootstrap script", func() {
-				By("creating it")
-				clusterConfig := makeClusterConfig()
-				clusterConfig.ManagedNodeGroups = []*api.ManagedNodeGroup{
-					{
-						NodeGroupBase: &api.NodeGroupBase{
-							Name:             "containerd",
-							ContainerRuntime: aws.String(api.ContainerRuntimeContainerD),
-						},
-					},
-				}
-
-				cmd := params.EksctlCreateCmd.
-					WithArgs(
-						"nodegroup",
-						"--config-file", "-",
-						"--verbose", "4",
-					).
-					WithoutArg("--region", params.Region).
-					WithStdin(testutils.ClusterConfigReader(clusterConfig))
-				Expect(cmd).To(RunSuccessfully())
-			})
-		})
-
 		Context("and deleting the cluster", func() {
 			It("should not return an error", func() {
 				cmd := params.EksctlDeleteClusterCmd.WithArgs(

diff --git a/pkg/apis/eksctl.io/v1alpha5/assets/schema.json b/pkg/apis/eksctl.io/v1alpha5/assets/schema.json
@@ -924,11 +924,6 @@
           "description": "specifies settings for Bottlerocket nodes",
           "x-intellij-html-description": "specifies settings for Bottlerocket nodes"
         },
-        "containerRuntime": {
-          "type": "string",
-          "description": "defines the runtime to use for the container.",
-          "x-intellij-html-description": "defines the runtime to use for the container."
-        },
         "desiredCapacity": {
           "type": "integer"
         },
@@ -1147,7 +1142,6 @@
         "efaEnabled",
         "instanceSelector",
         "bottlerocket",
-        "containerRuntime",
         "instanceTypes",
         "spot",
         "taints",
@@ -1249,8 +1243,8 @@
         },
         "containerRuntime": {
           "type": "string",
-          "description": "defines the runtime to use for the container.",
-          "x-intellij-html-description": "defines the runtime to use for the container."
+          "description": "defines the runtime (CRI) to use for containers on the node",
+          "x-intellij-html-description": "defines the runtime (CRI) to use for containers on the node"
         },
         "cpuCredits": {
           "type": "string",
@@ -1464,7 +1458,6 @@
         "efaEnabled",
         "instanceSelector",
         "bottlerocket",
-        "containerRuntime",
         "instancesDistribution",
         "asgMetricsCollection",
         "cpuCredits",
@@ -1473,7 +1466,8 @@
         "taints",
         "updateConfig",
         "clusterDNS",
-        "kubeletExtraConfig"
+        "kubeletExtraConfig",
+        "containerRuntime"
       ],
       "additionalProperties": false,
       "description": "holds configuration attributes that are specific to a nodegroup",

diff --git a/pkg/apis/eksctl.io/v1alpha5/defaults.go b/pkg/apis/eksctl.io/v1alpha5/defaults.go
@@ -110,7 +110,7 @@ func SetNodeGroupDefaults(ng *NodeGroup, meta *ClusterMeta) {
 		ng.SecurityGroups.WithShared = Enabled()
 	}
 
-	setContainerRuntimeDefault(ng.NodeGroupBase)
+	setContainerRuntimeDefault(ng)
 }
 
 // SetManagedNodeGroupDefaults sets default values for a ManagedNodeGroup
@@ -191,7 +191,7 @@ func setVolumeDefaults(ng *NodeGroupBase, template *LaunchTemplate) {
 	}
 }
 
-func setContainerRuntimeDefault(ng *NodeGroupBase) {
+func setContainerRuntimeDefault(ng *NodeGroup) {
 	if ng.ContainerRuntime == nil {
 		ng.ContainerRuntime = &DefaultContainerRuntime
 	}

diff --git a/pkg/apis/eksctl.io/v1alpha5/schema.go b/pkg/apis/eksctl.io/v1alpha5/schema.go
diff --git a/pkg/apis/eksctl.io/v1alpha5/types.go b/pkg/apis/eksctl.io/v1alpha5/types.go
@@ -896,6 +896,18 @@ type NodeGroup struct {
 	// [Customize `kubelet` config](/usage/customizing-the-kubelet/)
 	// +optional
 	KubeletExtraConfig *InlineDocument `json:"kubeletExtraConfig,omitempty"`
+
+	// ContainerRuntime defines the runtime (CRI) to use for containers on the node
+	// +optional
+	ContainerRuntime *string `json:"containerRuntime,omitempty"`
+}
+
+// GetContainerRuntime returns the container runtime.
+func (n *NodeGroup) GetContainerRuntime() string {
+	if n.ContainerRuntime != nil {
+		return *n.ContainerRuntime
+	}
+	return ""
 }
 
 func (n *NodeGroup) InstanceTypeList() []string {
@@ -1361,25 +1373,13 @@ type NodeGroupBase struct {
 	// This is a hack, will be removed shortly. When this is true for Ubuntu and
 	// AL2 images a legacy bootstrapper will be used.
 	CustomAMI bool `json:"-"`
-
-	// ContainerRuntime defines the runtime to use for the container.
-	// +optional
-	ContainerRuntime *string `json:"containerRuntime,omitempty"`
 }
 
 // Placement specifies placement group information
 type Placement struct {
 	GroupName string `json:"groupName,omitempty"`
 }
 
-// GetContainerRuntime returns the container runtime.
-func (n *NodeGroupBase) GetContainerRuntime() string {
-	if n.ContainerRuntime != nil {
-		return *n.ContainerRuntime
-	}
-	return ""
-}
-
 // ListOptions returns metav1.ListOptions with label selector for the nodegroup
 func (n *NodeGroupBase) ListOptions() metav1.ListOptions {
 	return metav1.ListOptions{

diff --git a/pkg/apis/eksctl.io/v1alpha5/validation.go b/pkg/apis/eksctl.io/v1alpha5/validation.go
@@ -269,18 +269,6 @@ func validateNodeGroupBase(ng *NodeGroupBase, path string) error {
 		return fmt.Errorf("AMI Family %s is not supported - use one of: %s", ng.AMIFamily, strings.Join(supportedAMIFamilies(), ", "))
 	}
 
-	if ng.ContainerRuntime != nil {
-		if *ng.ContainerRuntime == ContainerRuntimeContainerD && ng.AMIFamily != NodeImageFamilyAmazonLinux2 {
-			// check if it's dockerd or containerd
-			return fmt.Errorf("%s as runtime is only support for AL2 ami family", ContainerRuntimeContainerD)
-		}
-		if *ng.ContainerRuntime != ContainerRuntimeDockerD {
-			if *ng.ContainerRuntime != ContainerRuntimeContainerD {
-				return fmt.Errorf("only %s and %s are supported for container runtime", ContainerRuntimeContainerD, ContainerRuntimeDockerD)
-			}
-		}
-	}
-
 	return nil
 }
 
@@ -457,6 +445,16 @@ func ValidateNodeGroup(i int, ng *NodeGroup) error {
 		return err
 	}
 
+	if ng.ContainerRuntime != nil {
+		if *ng.ContainerRuntime == ContainerRuntimeContainerD && ng.AMIFamily != NodeImageFamilyAmazonLinux2 {
+			// check if it's dockerd or containerd
+			return fmt.Errorf("%s as runtime is only support for AL2 ami family", ContainerRuntimeContainerD)
+		}
+		if *ng.ContainerRuntime != ContainerRuntimeDockerD && *ng.ContainerRuntime != ContainerRuntimeContainerD {
+			return fmt.Errorf("only %s and %s are supported for container runtime", ContainerRuntimeContainerD, ContainerRuntimeDockerD)
+		}
+	}
+
 	return nil
 }
 

diff --git a/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go b/pkg/apis/eksctl.io/v1alpha5/zz_generated.deepcopy.go
diff --git a/pkg/ctl/cmdutils/configfile_test.go b/pkg/ctl/cmdutils/configfile_test.go
@@ -118,6 +118,30 @@ var _ = Describe("cmdutils configfile", func() {
 			})
 		})
 
+		Describe("managed nodegroup with container runtime", func() {
+			When("container runtime is set for a managed nodegroup", func() {
+				It("fails validation", func() {
+					cfg := api.NewClusterConfig()
+					cobraCmd := newCmd()
+					name := "foo-2"
+					cobraCmd.SetArgs([]string{"--name", name})
+
+					cmd := &Cmd{
+						ClusterConfig:     cfg,
+						NameArg:           name,
+						CobraCommand:      cobraCmd,
+						ClusterConfigFile: "test_data/managed-nodegroup-with-container-runtime.yaml",
+						ProviderConfig:    api.ProviderConfig{},
+					}
+					l := newCommonClusterConfigLoader(cmd)
+					l.flagsIncompatibleWithConfigFile.Delete("name")
+
+					err := l.Load()
+					Expect(err).To(MatchError(ContainSubstring("error unmarshaling JSON: while decoding JSON: json: unknown field \"containerRuntime\"")))
+				})
+			})
+		})
+
 		It("load all of example file", func() {
 			examples, err := filepath.Glob(examplesDir + "*.yaml")
 			Expect(err).ToNot(HaveOccurred())

diff --git a/pkg/ctl/cmdutils/test_data/managed-nodegroup-with-container-runtime.yaml b/pkg/ctl/cmdutils/test_data/managed-nodegroup-with-container-runtime.yaml
@@ -0,0 +1,14 @@
+# A simple example of ClusterConfig object:
+---
+apiVersion: eksctl.io/v1alpha5
+kind: ClusterConfig
+
+metadata:
+  name: test-cluster-1
+  region: eu-north-1
+
+managedNodeGroups:
+  - name: ng-1
+    instanceType: m5.large
+    desiredCapacity: 1
+    containerRuntime: containerd
diff --git a/pkg/nodebootstrap/al2_test.go b/pkg/nodebootstrap/al2_test.go
@@ -138,9 +138,9 @@ CONTAINER_RUNTIME=`,
 		Entry("container runtime set", bootScriptEntry{
 			ng: &api.NodeGroup{
 				NodeGroupBase: &api.NodeGroupBase{
-					ContainerRuntime: aws.String(api.ContainerRuntimeContainerD),
-					SSH:              &api.NodeGroupSSH{},
+					SSH: &api.NodeGroupSSH{},
 				},
+				ContainerRuntime: aws.String(api.ContainerRuntimeContainerD),
 			},
 			expectedUserData: `CLUSTER_NAME=userdata-test
 API_SERVER_URL=

diff --git a/pkg/nodebootstrap/ubuntu_test.go b/pkg/nodebootstrap/ubuntu_test.go
@@ -49,8 +49,7 @@ var _ = Describe("Ubuntu User Data", func() {
 API_SERVER_URL=
 B64_CLUSTER_CA=
 NODE_LABELS=
-NODE_TAINTS=
-CONTAINER_RUNTIME=`, "\n")))
+NODE_TAINTS=`, "\n")))
 			Expect(cloudCfg.WriteFiles[1].Permissions).To(Equal("0644"))
 		})
 

diff --git a/pkg/nodebootstrap/userdata.go b/pkg/nodebootstrap/userdata.go
@@ -169,12 +169,11 @@ func makeKubeletExtraConf(kubeletExtraConf *api.InlineDocument) (cloudconfig.Fil
 func makeBootstrapEnv(clusterConfig *api.ClusterConfig, np api.NodePool) cloudconfig.File {
 	ng := np.BaseNodeGroup()
 	variables := map[string]string{
-		"CLUSTER_NAME":      clusterConfig.Metadata.Name,
-		"API_SERVER_URL":    clusterConfig.Status.Endpoint,
-		"B64_CLUSTER_CA":    base64.StdEncoding.EncodeToString(clusterConfig.Status.CertificateAuthorityData),
-		"NODE_LABELS":       formatLabels(ng.Labels),
-		"NODE_TAINTS":       utils.FormatTaints(np.NGTaints()),
-		"CONTAINER_RUNTIME": np.BaseNodeGroup().GetContainerRuntime(),
+		"CLUSTER_NAME":   clusterConfig.Metadata.Name,
+		"API_SERVER_URL": clusterConfig.Status.Endpoint,
+		"B64_CLUSTER_CA": base64.StdEncoding.EncodeToString(clusterConfig.Status.CertificateAuthorityData),
+		"NODE_LABELS":    formatLabels(ng.Labels),
+		"NODE_TAINTS":    utils.FormatTaints(np.NGTaints()),
 	}
 
 	if ng.MaxPodsPerNode > 0 {
@@ -185,6 +184,10 @@ func makeBootstrapEnv(clusterConfig *api.ClusterConfig, np api.NodePool) cloudco
 		variables["CLUSTER_DNS"] = unmanaged.ClusterDNS
 	}
 
+	if unmanaged, ok := np.(*api.NodeGroup); ok && ng.AMIFamily == api.NodeImageFamilyAmazonLinux2 {
+		variables["CONTAINER_RUNTIME"] = unmanaged.GetContainerRuntime()
+	}
+
 	return cloudconfig.File{
 		Path:    configDir + envFile,
 		Content: makeKeyValues(variables, "\n"),