Merge pull request #10428 from hakman/update-cri-config

Update container runtime service files

nodeup/pkg/model/containerd.go

@@ -22,6 +22,7 @@ import (
 	"regexp"
 	"strings"
 
+	"github.com/blang/semver/v4"
 	"k8s.io/klog/v2"
 	"k8s.io/kops/nodeup/pkg/model/resources"
 	"k8s.io/kops/pkg/apis/kops"
@@ -109,7 +110,25 @@ func (b *ContainerdBuilder) Build(c *fi.ModelBuilderContext) error {
 		}
 	}
 
-	c.AddTask(b.buildSystemdService())
+	var containerRuntimeVersion string
+	if b.Cluster.Spec.ContainerRuntime == "containerd" {
+		if b.Cluster.Spec.Containerd != nil {
+			containerRuntimeVersion = fi.StringValue(b.Cluster.Spec.Containerd.Version)
+		} else {
+			return fmt.Errorf("error finding contained version")
+		}
+	} else {
+		if b.Cluster.Spec.Docker != nil {
+			containerRuntimeVersion = fi.StringValue(b.Cluster.Spec.Docker.Version)
+		} else {
+			return fmt.Errorf("error finding Docker version")
+		}
+	}
+	sv, err := semver.ParseTolerant(containerRuntimeVersion)
+	if err != nil {
+		return fmt.Errorf("error parsing container runtime version %q: %v", containerRuntimeVersion, err)
+	}
+	c.AddTask(b.buildSystemdService(sv))
 
 	if err := b.buildSysconfig(c); err != nil {
 		return err
@@ -126,8 +145,8 @@ func (b *ContainerdBuilder) Build(c *fi.ModelBuilderContext) error {
 	return nil
 }
 
-func (b *ContainerdBuilder) buildSystemdService() *nodetasks.Service {
-	// Based on https://github.com/containerd/cri/blob/master/contrib/systemd-units/containerd.service
+func (b *ContainerdBuilder) buildSystemdService(sv semver.Version) *nodetasks.Service {
+	// Based on https://github.com/containerd/containerd/blob/master/containerd.service
 
 	manifest := &systemd.Manifest{}
 	manifest.Set("Unit", "Description", "containerd container runtime")
@@ -145,21 +164,27 @@ func (b *ContainerdBuilder) buildSystemdService() *nodetasks.Service {
 	manifest.Set("Service", "ExecStartPre", "-/sbin/modprobe overlay")
 	manifest.Set("Service", "ExecStart", "/usr/bin/containerd -c /etc/containerd/config-kops.toml \"$CONTAINERD_OPTS\"")
 
-	manifest.Set("Service", "Restart", "always")
-	manifest.Set("Service", "RestartSec", "5")
+	// notify the daemon's readiness to systemd
+	if (b.Cluster.Spec.ContainerRuntime == "containerd" && sv.GTE(semver.MustParse("1.3.4"))) || sv.GTE(semver.MustParse("19.3.13")) {
+		manifest.Set("Service", "Type", "notify")
+	}
 
 	// set delegate yes so that systemd does not reset the cgroups of containerd containers
 	manifest.Set("Service", "Delegate", "yes")
 	// kill only the containerd process, not all processes in the cgroup
 	manifest.Set("Service", "KillMode", "process")
-	// make killing of processes of this unit under memory pressure very unlikely
-	manifest.Set("Service", "OOMScoreAdjust", "-999")
 
-	manifest.Set("Service", "LimitNOFILE", "1048576")
+	manifest.Set("Service", "Restart", "always")
+	manifest.Set("Service", "RestartSec", "5")
+
 	manifest.Set("Service", "LimitNPROC", "infinity")
 	manifest.Set("Service", "LimitCORE", "infinity")
+	manifest.Set("Service", "LimitNOFILE", "infinity")
 	manifest.Set("Service", "TasksMax", "infinity")
 
+	// make killing of processes of this unit under memory pressure very unlikely
+	manifest.Set("Service", "OOMScoreAdjust", "-999")
+
 	manifest.Set("Install", "WantedBy", "multi-user.target")
 
 	manifestString := manifest.Render()

nodeup/pkg/model/containerd_test.go

@@ -28,6 +28,14 @@ import (
 	"k8s.io/kops/util/pkg/distributions"
 )
 
+func TestContainerdBuilder_Docker_19_03_13(t *testing.T) {
+	runContainerdBuilderTest(t, "from_docker_19.03.11")
+}
+
+func TestContainerdBuilder_Docker_19_03_14(t *testing.T) {
+	runContainerdBuilderTest(t, "from_docker_19.03.14")
+}
+
 func TestContainerdBuilder_Simple(t *testing.T) {
 	runContainerdBuilderTest(t, "simple")
 }

nodeup/pkg/model/docker.go

@@ -187,7 +187,6 @@ func (b *DockerBuilder) buildSystemdService(dockerVersion semver.Version) *nodet
 	manifest.Set("Unit", "Description", "Docker Application Container Engine")
 	manifest.Set("Unit", "Documentation", "https://docs.docker.com")
 	if dockerVersion.GTE(semver.MustParse("18.9.0")) {
-		manifest.Set("Unit", "BindsTo", "containerd.service")
 		manifest.Set("Unit", "After", "network-online.target firewalld.service containerd.service")
 	} else {
 		manifest.Set("Unit", "After", "network-online.target firewalld.service")
@@ -198,19 +197,18 @@ func (b *DockerBuilder) buildSystemdService(dockerVersion semver.Version) *nodet
 	manifest.Set("Service", "EnvironmentFile", "/etc/sysconfig/docker")
 	manifest.Set("Service", "EnvironmentFile", "/etc/environment")
 
+	manifest.Set("Service", "Type", "notify")
 	// Restore the default SELinux security contexts for the Docker binaries
 	if b.Distribution.IsRHELFamily() && b.Cluster.Spec.Docker != nil && fi.BoolValue(b.Cluster.Spec.Docker.SelinuxEnabled) {
 		manifest.Set("Service", "ExecStartPre", "/bin/sh -c 'restorecon -v /usr/bin/docker*'")
 	}
-
 	// the default is not to use systemd for cgroups because the delegate issues still
 	// exists and systemd currently does not support the cgroup feature set required
 	// for containers run by docker
-	manifest.Set("Service", "Type", "notify")
 	manifest.Set("Service", "ExecStart", "/usr/bin/dockerd -H fd:// \"$DOCKER_OPTS\"")
 	manifest.Set("Service", "ExecReload", "/bin/kill -s HUP $MAINPID")
 	manifest.Set("Service", "TimeoutSec", "0")
-	manifest.Set("Service", "RestartSec", "2s")
+	manifest.Set("Service", "RestartSec", "2")
 	manifest.Set("Service", "Restart", "always")
 
 	// Note that StartLimit* options were moved from "Service" to "Unit" in systemd 229.
@@ -237,6 +235,7 @@ func (b *DockerBuilder) buildSystemdService(dockerVersion semver.Version) *nodet
 
 	// kill only the docker process, not all processes in the cgroup
 	manifest.Set("Service", "KillMode", "process")
+	manifest.Set("Service", "OOMScoreAdjust", "-500")
 
 	manifest.Set("Install", "WantedBy", "multi-user.target")
 

nodeup/pkg/model/tests/containerdbuilder/from_docker_19.03.11/cluster.yaml

@@ -0,0 +1,39 @@
+apiVersion: kops.k8s.io/v1alpha2
+kind: Cluster
+metadata:
+  name: minimal.example.com
+spec:
+  kubernetesApiAccess:
+    - 0.0.0.0/0
+  channel: stable
+  cloudProvider: aws
+  configBase: memfs://clusters.example.com/minimal.example.com
+  containerRuntime: docker
+  docker:
+    version: 19.03.11
+  etcdClusters:
+    - etcdMembers:
+        - instanceGroup: master-us-test-1a
+          name: master-us-test-1a
+      name: main
+    - etcdMembers:
+        - instanceGroup: master-us-test-1a
+          name: master-us-test-1a
+      name: events
+  kubernetesVersion: v1.19.0
+  masterInternalName: api.internal.minimal.example.com
+  masterPublicName: api.minimal.example.com
+  networkCIDR: 172.20.0.0/16
+  networking:
+    kubenet: {}
+  nonMasqueradeCIDR: 100.64.0.0/10
+  sshAccess:
+    - 0.0.0.0/0
+  topology:
+    masters: public
+    nodes: public
+  subnets:
+    - cidr: 172.20.32.0/19
+      name: us-test-1a
+      type: Public
+      zone: us-test-1a