Merge pull request #8170 from hakman/coninerd-add-1.2.4

containerd: Use containerd 1.2.4 with Docker 18.09.3

nodeup/pkg/model/containerd.go

@@ -38,6 +38,18 @@ type ContainerdBuilder struct {
 var _ fi.ModelBuilder = &ContainerdBuilder{}
 
 var containerdVersions = []packageVersion{
+	// 1.2.4 - Debian Stretch
+	{
+		PackageVersion: "1.2.4",
+		Name:           "containerd.io",
+		Distros:        []distros.Distribution{distros.DistributionDebian9},
+		Architectures:  []Architecture{ArchitectureAmd64},
+		Version:        "1.2.4-1",
+		Source:         "https://download.docker.com/linux/debian/dists/stretch/pool/stable/amd64/containerd.io_1.2.4-1_amd64.deb",
+		Hash:           "48c6ab0c908316af9a183de5aad64703bc516bdf",
+		Dependencies:   []string{"libseccomp2", "pigz"},
+	},
+
 	// 1.2.10 - Debian Stretch
 	{
 		PackageVersion: "1.2.10",
@@ -117,11 +129,9 @@ var containerdVersions = []packageVersion{
 		Dependencies:   []string{"container-selinux", "libseccomp", "pigz"},
 	},
 
-	// TIP: When adding the next version, copy the previous
+	// TIP: When adding the next version, copy the previous version, string replace the version and run:
-	// version, string replace the version, run `VERIFY_HASHES=1
+	//   VERIFY_HASHES=1 go test ./nodeup/pkg/model -run TestContainerdPackageHashes
-	// go test ./nodeup/pkg/model` (you might want to temporarily
+	// (you might want to temporarily comment out older versions on a slower connection and then validate)
-	// comment out older versions on a slower connection), and
-	// then validate the dependencies etc
 }
 
 func (b *ContainerdBuilder) containerdVersion() (string, error) {

nodeup/pkg/model/docker.go

@@ -963,11 +963,9 @@ var dockerVersions = []packageVersion{
 		Dependencies: []string{"libtool-ltdl", "iptables"},
 	},
 
-	// TIP: When adding the next version, copy the previous
+	// TIP: When adding the next version, copy the previous version, string replace the version and run:
-	// version, string replace the version, run `VERIFY_HASHES=1
+	//   VERIFY_HASHES=1 go test ./nodeup/pkg/model -run TestDockerPackageHashes
-	// go test ./nodeup/pkg/model` (you might want to temporarily
+	// (you might want to temporarily comment out older versions on a slower connection and then validate)
-	// comment out older versions on a slower connection), and
-	// then validate the dependencies etc
 }
 
 func (b *DockerBuilder) dockerVersion() string {

pkg/model/components/containerd.go

@@ -19,6 +19,7 @@ package components
 import (
 	"fmt"
 
+	"k8s.io/klog"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
 	"k8s.io/kops/upup/pkg/fi/loader"
@@ -41,64 +42,58 @@ func (b *ContainerdOptionsBuilder) BuildOptions(o interface{}) error {
 
 	containerd := clusterSpec.Containerd
 
-	if fi.StringValue(clusterSpec.Containerd.Version) == "" {
+	if clusterSpec.ContainerRuntime == "containerd" {
-		containerdVersion := ""
+		if b.IsKubernetesLT("1.11") {
+			// Containerd 1.2 is validated against Kubernetes v1.11+
+			// https://github.com/containerd/containerd/blob/master/releases/v1.2.0.toml#L34
+			return fmt.Errorf("kubernetes %s is not compatible with containerd", clusterSpec.KubernetesVersion)
+		} else if b.IsKubernetesLT("1.18") {
+			klog.Warningf("kubernetes %s is untested with containerd", clusterSpec.KubernetesVersion)
+		}
 
-		if clusterSpec.ContainerRuntime == "containerd" {
+		// Set containerd based on Kubernetes version
-			if clusterSpec.KubernetesVersion == "" {
+		if fi.StringValue(containerd.Version) == "" {
-				return fmt.Errorf("Kubernetes version is required")
+			if b.IsKubernetesGTE("1.18") {
+				containerd.Version = fi.String("1.2.10")
+			} else if b.IsKubernetesGTE("1.11") {
+				return fmt.Errorf("containerd version is required")
 			}
+		}
 
-			sv, err := KubernetesVersion(clusterSpec)
+		// Apply defaults for containerd running in container runtime mode
-			if err != nil {
+		containerd.LogLevel = fi.String("warn")
-				return fmt.Errorf("unable to determine kubernetes version from %q", clusterSpec.KubernetesVersion)
+		containerd.ConfigOverride = fi.String("")
-			}
 
-			if sv.Major == 1 && sv.Minor >= 11 {
+	} else if clusterSpec.ContainerRuntime == "docker" {
-				// Containerd 1.2 is validated against Kubernetes v1.11+
+		if fi.StringValue(containerd.Version) == "" {
-				// https://github.com/containerd/containerd/blob/master/releases/v1.2.0.toml#L34
+			// Docker version should always be available
-				containerdVersion = "1.2.10"
-			} else {
-				return fmt.Errorf("unknown version of kubernetes %q (cannot infer containerd version)", clusterSpec.KubernetesVersion)
-			}
-
-		} else if clusterSpec.ContainerRuntime == "docker" {
 			if fi.StringValue(clusterSpec.Docker.Version) == "" {
-				return fmt.Errorf("Docker version is required")
+				return fmt.Errorf("docker version is required")
 			}
 
-			// Set containerd version for known Docker versions
+			// Set the containerd version for known Docker versions
-			dockerVersion := fi.StringValue(clusterSpec.Docker.Version)
+			switch fi.StringValue(clusterSpec.Docker.Version) {
-			switch dockerVersion {
 			case "19.03.4":
-				containerdVersion = "1.2.10"
+				containerd.Version = fi.String("1.2.10")
 			case "18.09.9":
-				containerdVersion = "1.2.10"
+				containerd.Version = fi.String("1.2.10")
 			case "18.09.3":
-				containerdVersion = "1.2.10"
+				containerd.Version = fi.String("1.2.4")
 			default:
-				// Older version of docker
+				// Old version of docker, single package
 				containerd.SkipInstall = true
+				return nil
 			}
-
-		} else {
-			// Unknown container runtime, should not install containerd
-			containerd.SkipInstall = true
 		}
 
-		if containerdVersion != "" {
+		// Apply defaults for containerd running in Docker mode
-			containerd.Version = &containerdVersion
+		containerd.LogLevel = fi.String("warn")
-		}
+		containerd.ConfigOverride = fi.String("disabled_plugins = [\"cri\"]\n")
-	}
 
-	// Apply global containerd defaults
+	} else {
-	containerd.LogLevel = fi.String("warn")
+		// Unknown container runtime, should not install containerd
-
+		containerd.SkipInstall = true
-	configOverride := ""
-	if clusterSpec.ContainerRuntime == "docker" {
-		configOverride += "disabled_plugins = [\"cri\"]\n"
 	}
-	containerd.ConfigOverride = &configOverride
 
 	return nil
 }

pkg/model/components/containerd_test.go

@@ -60,9 +60,37 @@ func Test_Build_Containerd_Unsupported_Version(t *testing.T) {
 	}
 }
 
-func Test_Build_Containerd_Supported_Version(t *testing.T) {
+func Test_Build_Containerd_Untested_Version(t *testing.T) {
 	kubernetesVersions := []string{"1.11.0", "1.11.2", "1.14.0", "1.16.3"}
 
+	for _, v := range kubernetesVersions {
+
+		c := buildContainerdCluster(v)
+		c.Spec.ContainerRuntime = "containerd"
+		b := assets.NewAssetBuilder(c, "")
+
+		version, err := util.ParseKubernetesVersion(v)
+		if err != nil {
+			t.Fatalf("unexpected error from ParseKubernetesVersion %s: %v", v, err)
+		}
+
+		ob := &ContainerdOptionsBuilder{
+			&OptionsContext{
+				AssetBuilder:      b,
+				KubernetesVersion: *version,
+			},
+		}
+
+		err = ob.BuildOptions(&c.Spec)
+		if err == nil {
+			t.Fatalf("expecting error when Kubernetes version >= 1.11 and < 1.18: %s", v)
+		}
+	}
+}
+
+func Test_Build_Containerd_Supported_Version(t *testing.T) {
+	kubernetesVersions := []string{"1.18.0", "1.18.3"}
+
 	for _, v := range kubernetesVersions {
 
 		c := buildContainerdCluster(v)

pkg/model/components/docker.go

@@ -17,8 +17,6 @@ limitations under the License.
 package components
 
 import (
-	"fmt"
-
 	"k8s.io/klog"
 	"k8s.io/kops/pkg/apis/kops"
 	"k8s.io/kops/upup/pkg/fi"
@@ -36,11 +34,6 @@ var _ loader.OptionsBuilder = &DockerOptionsBuilder{}
 func (b *DockerOptionsBuilder) BuildOptions(o interface{}) error {
 	clusterSpec := o.(*kops.ClusterSpec)
 
-	sv, err := KubernetesVersion(clusterSpec)
-	if err != nil {
-		return fmt.Errorf("unable to determine kubernetes version from %q", clusterSpec.KubernetesVersion)
-	}
-
 	if clusterSpec.Docker == nil {
 		clusterSpec.Docker = &kops.DockerConfig{}
 	}
@@ -50,40 +43,31 @@ func (b *DockerOptionsBuilder) BuildOptions(o interface{}) error {
 	// Container runtime is not Docker, should not install
 	if clusterSpec.ContainerRuntime != "docker" {
 		docker.SkipInstall = true
+		return nil
 	}
 
+	// Set the Docker version for known Kubernetes versions
 	if fi.StringValue(clusterSpec.Docker.Version) == "" {
-		if clusterSpec.KubernetesVersion == "" {
+		if b.IsKubernetesGTE("1.17") {
-			return fmt.Errorf("KubernetesVersion is required")
+			docker.Version = fi.String("19.03.4")
+		} else if b.IsKubernetesGTE("1.16") {
+			docker.Version = fi.String("18.09.9")
+		} else if b.IsKubernetesGTE("1.12") {
+			docker.Version = fi.String("18.06.3")
+		} else if b.IsKubernetesGTE("1.9") {
+			docker.Version = fi.String("17.03.2")
+		} else if b.IsKubernetesGTE("1.8") {
+			docker.Version = fi.String("1.13.1")
+		} else if b.IsKubernetesGTE("1.6") {
+			docker.Version = fi.String("1.12.6")
+		} else if b.IsKubernetesGTE("1.5") {
+			docker.Version = fi.String("1.12.3")
+		} else {
+			docker.Version = fi.String("1.11.2")
 		}
-
-		dockerVersion := ""
-		if sv.Major == 1 && sv.Minor >= 17 {
-			dockerVersion = "19.03.4"
-		} else if sv.Major == 1 && sv.Minor >= 16 {
-			dockerVersion = "18.09.9"
-		} else if sv.Major == 1 && sv.Minor >= 12 {
-			dockerVersion = "18.06.3"
-		} else if sv.Major == 1 && sv.Minor >= 9 {
-			dockerVersion = "17.03.2"
-		} else if sv.Major == 1 && sv.Minor >= 8 {
-			dockerVersion = "1.13.1"
-		} else if sv.Major == 1 && sv.Minor >= 6 {
-			dockerVersion = "1.12.6"
-		} else if sv.Major == 1 && sv.Minor >= 5 {
-			dockerVersion = "1.12.3"
-		} else if sv.Major == 1 && sv.Minor <= 4 {
-			dockerVersion = "1.11.2"
-		}
-
-		if dockerVersion == "" {
-			return fmt.Errorf("unknown version of kubernetes %q (cannot infer docker version)", clusterSpec.KubernetesVersion)
-		}
-
-		clusterSpec.Docker.Version = &dockerVersion
 	}
 
-	if sv.Major == 1 && sv.Minor >= 6 {
+	if b.IsKubernetesGTE("1.6") {
 		if len(clusterSpec.Docker.LogOpt) == 0 && clusterSpec.Docker.LogDriver == nil {
 			// Use built-in docker logging, if not configured otherwise (by the user)
 			logDriver := "json-file"

tests/integration/update_cluster/additional_cidr/cloudformation.json.extracted.yaml

@@ -135,9 +135,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1bmastersadditionalcidrex
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -430,9 +427,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesadditionalcidrexamplecom.Propert
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false

tests/integration/update_cluster/additional_user-data/cloudformation.json.extracted.yaml

@@ -144,9 +144,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersadditionaluserda
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -459,9 +456,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesadditionaluserdataexamplecom.Pro
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false

tests/integration/update_cluster/complex/cloudformation.json.extracted.yaml

@@ -135,9 +135,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amasterscomplexexampleco
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -432,9 +429,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodescomplexexamplecom.Properties.Use
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false

tests/integration/update_cluster/containerd-cloudformation/cloudformation.json.extracted.yaml

@@ -139,16 +139,7 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amasterscontainerdexampl
     logLevel: warn
     version: 1.2.10
   docker:
-    ipMasq: false
-    ipTables: false
-    logDriver: json-file
-    logLevel: warn
-    logOpt:
-    - max-size=10m
-    - max-file=5
     skipInstall: true
-    storage: overlay2,overlay,aufs
-    version: 18.06.3
   encryptionConfig: null
   etcdClusters:
     events:
@@ -434,16 +425,7 @@ Resources.AWSAutoScalingLaunchConfigurationnodescontainerdexamplecom.Properties.
     logLevel: warn
     version: 1.2.10
   docker:
-    ipMasq: false
-    ipTables: false
-    logDriver: json-file
-    logLevel: warn
-    logOpt:
-    - max-size=10m
-    - max-file=5
     skipInstall: true
-    storage: overlay2,overlay,aufs
-    version: 18.06.3
   kubeProxy:
     clusterCIDR: 100.96.0.0/11
     cpuRequest: 100m

tests/integration/update_cluster/containerd-cloudformation/in-v1alpha2.yaml

@@ -10,6 +10,8 @@ spec:
   cloudProvider: aws
   configBase: memfs://clusters.example.com/containerd.example.com
   containerRuntime: containerd
+  containerd:
+    version: 1.2.10
   etcdClusters:
   - etcdMembers:
     - instanceGroup: master-us-test-1a

tests/integration/update_cluster/existing_iam_cloudformation/cloudformation.json.extracted.yaml

@@ -135,9 +135,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersminimalexampleco
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -430,9 +427,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesminimalexamplecom.Properties.Use
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false

tests/integration/update_cluster/externallb/cloudformation.json.extracted.yaml

@@ -135,9 +135,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersexternallbexampl
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -430,9 +427,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesexternallbexamplecom.Properties.
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false

tests/integration/update_cluster/minimal-cloudformation/cloudformation.json.extracted.yaml

@@ -135,9 +135,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersminimalexampleco
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -430,9 +427,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesminimalexamplecom.Properties.Use
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false

tests/integration/update_cluster/mixed_instances/cloudformation.json.extracted.yaml

@@ -135,9 +135,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersmixedinstancesex
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -432,9 +429,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1bmastersmixedinstancesex
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -729,9 +723,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1cmastersmixedinstancesex
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -1026,9 +1017,6 @@ Resources.AWSEC2LaunchTemplatenodesmixedinstancesexamplecom.Properties.LaunchTem
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false

tests/integration/update_cluster/mixed_instances_spot/cloudformation.json.extracted.yaml

@@ -135,9 +135,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersmixedinstancesex
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -432,9 +429,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1bmastersmixedinstancesex
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -729,9 +723,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1cmastersmixedinstancesex
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -1026,9 +1017,6 @@ Resources.AWSEC2LaunchTemplatenodesmixedinstancesexamplecom.Properties.LaunchTem
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false

tests/integration/update_cluster/privatecalico/cloudformation.json.extracted.yaml

@@ -135,9 +135,6 @@ Resources.AWSAutoScalingLaunchConfigurationmasterustest1amastersprivatecalicoexa
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false
@@ -428,9 +425,6 @@ Resources.AWSAutoScalingLaunchConfigurationnodesprivatecalicoexamplecom.Properti
   cloudConfig: null
   containerRuntime: docker
   containerd:
-    configOverride: |
-      disabled_plugins = ["cri"]
-    logLevel: warn
     skipInstall: true
   docker:
     ipMasq: false