Use systemd as the containerd cgroup driver

[stevehipwell]

aws/containers-roadmap#1210

Description of changes:
This PR sets systemd as the cgroup driver when using containerd, as recommended by the Kubernetes docs. This is complimentary to #593 which adds support for the systemd cgroup driver to Docker but shouldn't be blocked by legacy eksctl constraints as containerd is newer than the systemd cgroup support there.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[stevehipwell]

/assign @abeer91

[stevehipwell]

FYI @ravisinha0506

[stevehipwell]

@Callisto13 related to your #593 (comment), I assume that the changes in here aren't likely to cause issue with the legacy path as containerd wouldn't be being used?

[Callisto13]

@stevehipwell from what I can see that is correct. We are not exposing the containerd option in the legacy codepath, so there should be no driver conflict. I am happy to verify with a test build before you release if you have any concerns, but I think we should be fine.

[stevehipwell]

Thanks @Callisto13 I think we should be good, just waiting to hear from a maintainer on this.

[stevehipwell]

@abeer91 could you take a look at this?

[stevehipwell]

@abeer91 if you're not the right person to look into this could you let me know who is?

[stevehipwell]

Could one of the project maintainers please review or offer some feedback on this?

[rtripat]

@stevehipwell Thanks for submitting the PR. We are prioritizing the testing of cgroup driver to systemd based on your changes. I don't have an ECD to share currently but will update within 2 weeks.

[stevehipwell]

Thanks @rtripat, hopefully we can get this solved ASAP.

[stevehipwell]

@rtripat how is the testing going?

[stevehipwell]

@rtripat it's been almost 2 months since you said you'd get back in 2 weeks, how is this going?

[rtripat]

@stevehipwell We are actively working on the change. Appreciate the patience.

[stevehipwell]

@rtripat are you by any chance also looking at cgroup v2?

[cartermckinnon]

We need to address this kubelet issue by updating our runc. That change is in motion, and I'll keep you updated!

Regarding cgroup v2, we are unable to make this change until a newer version of systemd is available to us. That will most likely happen in the next major release of Amazon Linux.

[stevehipwell]

@cartermckinnon after having had a look through that issue it looks like it's only relevant for Kubernetes v1.22? If so and as EKS v1.22 isn't due until "early 2022" can't this be added for the current EKS versions?

RE cgroup v2 is it worth adding a new issue to this repo to track the progress?

[cartermckinnon]

My understanding is:

• The runc library, libcontainer, which is vendored in k/k, needed to be updated to 1.0.2 to fix #104280. This fix has only been back-ported to 1.21 as of 1.21.5.
• The runc binary needs to be updated to at least 1.0.1 to avoid a similar (but more serious) issue. This is actually the more important issue to address.

Older Kubernetes release lines are affected by the first point regardless of the runc binary's version, because of libcontainer. However, as long as the binary is at 1.0.1+, the issue is not critical (and quite rare to begin with).

I've created #824 to track cgroup v2.

[stevehipwell]

@rtripat could we get a status update on this?

[cartermckinnon]

Apologies for the delay, our packaging team has had to prioritize issues related to the log4j vulnerabilities. We will proceed as soon as the runc available to us is at 1.0.1+, and we expect that to happen soon.

Thanks for your patience! 🙏

[cartermckinnon]

Unfortunately, we aren't able to make this change in the near future. Our packaging team has to prioritize stability across a variety of Amazon Linux distributions, and the changelog of runc requires non-trivial evaluation and testing. I can assure you that we will make this change as soon as we are able to.

In the meantime, if anyone has experienced the sort of stability issues hinted at in the Kubernetes documentation, we would love to hear about it.

[stevehipwell]

@cartermckinnon I'm interested in why this isn't seen as a high priority and what the blockers are here?

I guess this is the final nail in the coffin of running K8s workloads on AL 2 (and I assume AL 2022) and we should all move to Bottlerocket ASAP to stay inline with the community.

[reegnz]

EDIT: ok, I looked at the runc changelog, that's a bummer.

If you don't intend to update runc as high prio though (as I understand that's the blocker for this issue), does that mean you'll also ignore security patches for runc? There has been some serious CVE-s in the past for runc and that worries me.
Maybe I'm just misunderstanding your comment.

[cartermckinnon]

I'm not an authority on the priorities of the Amazon Linux packaging folks, but I can say: we have many users, and I'm not aware of any case in which we've attributed instability to the presence of two cgroup managers. Again, if you have observed this type of instability, we're very interested in hearing about it.

The runc available on Amazon Linux contains (and will continue to receive) security patches.

[cartermckinnon]

@stevehipwell I have good news! We now have a version of runc (1.0.3-2.amzn2) that lets us move forward with this change.

Two things we need to get this merged:

• Resolve this branch's conflicts
• Use jq to set "cgroupDriver": "systemd" instead of replacing the whole kubelet config. Example here: https://github.com/awslabs/amazon-eks-ami/blob/master/files/bootstrap.sh#L412

I'm happy to open a separate PR if you don't have the cycles. 😄

[stevehipwell]

@cartermckinnon I'll update my PR.

[stevehipwell]

@cartermckinnon this should be good now. I've copied the shell script patterns from the rest of the script rather than the ones I'd have used, but can update the PR to make them more correct if required.

[cartermckinnon]

Thanks, @stevehipwell, LGTM! I'll do some sanity tests and try to get this merged this week.

[cartermckinnon]

I built 1.19-1.22 AMIs from this branch, and created a nodegroup using each as follows:

---
apiVersion: eksctl.io/v1alpha5
kind: ClusterConfig
metadata:
  name: systemd-test-120
  region: us-west-2
  version: "1.20"
managedNodeGroups:
  - name: "119"
    ami: ami-0d4655ebca7f3a80a
    ssh:
      allow: true
    minSize: 1
    maxSize: 1
    desiredCapacity: 1
    overrideBootstrapCommand: |
      #!/bin/bash
      source /var/lib/cloud/scripts/eksctl/bootstrap.helper.sh
      /etc/eks/bootstrap.sh systemd-test-120 --container-runtime containerd --kubelet-extra-args "--node-labels=${NODE_LABELS}"
  - name: "120"
    ami: ami-0538a5514d4cfe90f
    ssh:
      allow: true
    minSize: 1
    maxSize: 1
    desiredCapacity: 1
    overrideBootstrapCommand: |
      #!/bin/bash
      source /var/lib/cloud/scripts/eksctl/bootstrap.helper.sh
      /etc/eks/bootstrap.sh systemd-test-120 --container-runtime containerd --kubelet-extra-args "--node-labels=${NODE_LABELS}"      

---
apiVersion: eksctl.io/v1alpha5
kind: ClusterConfig
metadata:
  name: systemd-test-122
  region: us-west-2
  version: "1.22"
managedNodeGroups:
  - name: "121"
    ami: ami-080237cf3a00f2e67
    ssh:
      allow: true
    minSize: 1
    maxSize: 1
    desiredCapacity: 1
    overrideBootstrapCommand: |
      #!/bin/bash
      source /var/lib/cloud/scripts/eksctl/bootstrap.helper.sh
      /etc/eks/bootstrap.sh systemd-test-122 --container-runtime containerd --kubelet-extra-args "--node-labels=${NODE_LABELS}"
  - name: "122"
    ami: ami-021b3b95e7c2eace1
    ssh:
      allow: true
    minSize: 1
    maxSize: 1
    desiredCapacity: 1
    overrideBootstrapCommand: |
      #!/bin/bash
      source /var/lib/cloud/scripts/eksctl/bootstrap.helper.sh
      /etc/eks/bootstrap.sh systemd-test-122 --container-runtime containerd --kubelet-extra-args "--node-labels=${NODE_LABELS}"

Pods came up, LGTM.

I want to ship this change in a dedicated AMI release; so I'll wait to merge until we ship #921.

@stevehipwell would you mind addressing the conflicts that came from #921 ?

[stevehipwell]

@cartermckinnon I've rebased this PR so it should be good to merge when you're ready.

[svyatoslavmo]

Any updates on this? Is there ETA for release?
Thanks

[stevehipwell]

@cartermckinnon could we get this merged and released?

[suket22]

Lgtm!

[mmerkes]

LGTM

[cartermckinnon]

I'm going to merge this.

We have an AMI build currently making its way through our release process, and this change won't make it into that version; it will land in the subsequent version. I'll update here once it's available.

Thanks for all your work on this, @stevehipwell! 🙏

[kishoregv]

This is creating a mismatch in the kubelet and container cgroupDriver when creating unmananged nodegroups with eksctl.

[kishoregv]

This is effecting clusters using eksctl v0.90.0(or less).

until this version eksctl makes its own kubelet config which does not have cgroupDriver setting, this is causing a mis-match where containerd using SystemdCgroup but kubelet does not have systemd

[Callisto13]

AFAIK older versions of eksctl are not supported, so the solution here is to not use old versions and download the latest binary.