al2023: Use base_runtime_spec to set default rlimits

[cartermckinnon]

Issue #, if available:

Fixes #1746

Description of changes:

This uses the base_runtime_spec option of containerd to set RLMIT_NOFILE defaults that are much lower than 2^63-1, the default on AL2023. This approach lets us define the limits for containers without changing containerd's own limits.

I've gone with 65536:1048576 with the understanding that programs using select(2) will need lower limits to be defined by the user to avoid undefined behavior. In practice, it's more common for our users to run programs that do not raise their limits appropriately--such as Envoy--so I think this is an acceptable default today. Bottlerocket is using the same values, so the consistency is beneficial: https://github.com/bottlerocket-os/bottlerocket/blob/fcf71a47c0ff005327ecde870d8a70877fc89196/sources/models/shared-defaults/oci-defaults-containerd-cri-resource-limits.toml

These defaults can be overridden by a new NodeConfig field, Spec.Containerd.BaseRuntimeSpec. This field behaves the same as Spec.Kubelet.Config -- it's an unstructured JSON/YAML document that will be merged on top of the default, allowing partial overrides.

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

[cartermckinnon]

/ci
+workflow:os_distros al2023

[github-actions]

@cartermckinnon roger that! I've dispatched a workflow. 👍

[ndbaker1]

appreciate making this configurable, but do we have to set a default?
asking based on the time we updated limits in #1535 in al2, but I see the benefit of making this a breaking al2023 change to set the default and follow bottlerocket's example

[cartermckinnon]

Yeah, we have to set a lower default in order to fix #1746. We don't want folks to have to pass baseRuntimeSpec to run stuff as common as Redis.

The issues caused by #1535 were mostly related to the 1024 effective soft limit (which is the "correct" config but is nevertheless incompatible with a lot of software written in the container age). That's why we use a much higher soft limit here, as well as a higher hard limit. I think this is fairly unlikely to be disruptive; and there's an easy mitigation if it does (setting baseRuntimeSpec).

[github-actions]

@cartermckinnon the workflow that you requested has completed. 🎉

AMI variant	Build	Test
1.23 / al2023	success ✅	failure ❌
1.24 / al2023	success ✅	failure ❌
1.25 / al2023	success ✅	failure ❌
1.26 / al2023	success ✅	failure ❌
1.27 / al2023	success ✅	failure ❌
1.28 / al2023	success ✅	failure ❌
1.29 / al2023	success ✅	failure ❌
1.30 / al2023	failure ❌	skipped ⏭️

[cartermckinnon]

Argh, we have to pass a complete OCI spec instead of just process.rlimits. I'm going to merge the NodeConfig's baseRuntimeSpec with a fully-formed default, because if someone wants to override the rlimits, they shouldn't have to pass all the other junk.

[ps-jay]

Thanks for your work on this @cartermckinnon

[cartermckinnon]

/ci
+workflow:os_distros al2023

[github-actions]

@cartermckinnon roger that! I've dispatched a workflow. 👍

[github-actions]

@cartermckinnon the workflow that you requested has completed. 🎉

AMI variant	Build	Test
1.23 / al2023	success ✅	failure ❌
1.24 / al2023	success ✅	failure ❌
1.25 / al2023	success ✅	failure ❌
1.26 / al2023	success ✅	failure ❌
1.27 / al2023	success ✅	failure ❌
1.28 / al2023	success ✅	failure ❌
1.29 / al2023	success ✅	failure ❌
1.30 / al2023	success ✅	failure ❌

[dims]

@cartermckinnon is this the same limits used in bottlerocket?

[cartermckinnon]

@dims yep!

[cartermckinnon]

Need to fix something up with the capabilities block in the latest rev

[sig-node] Security Context When creating a pod with privileged should run the container as unprivileged when false [LinuxOnly] [NodeConformance] [Conformance]

[cartermckinnon]

/ci
+workflow:os_distros al2023

[github-actions]

@cartermckinnon roger that! I've dispatched a workflow. 👍

[github-actions]

@cartermckinnon the workflow that you requested has completed. 🎉

AMI variant	Build	Test
1.23 / al2023	success ✅	failure ❌
1.24 / al2023	success ✅	failure ❌
1.25 / al2023	success ✅	failure ❌
1.26 / al2023	success ✅	failure ❌
1.27 / al2023	success ✅	failure ❌
1.28 / al2023	success ✅	failure ❌
1.29 / al2023	success ✅	failure ❌
1.30 / al2023	success ✅	failure ❌

[cartermckinnon]

/ci
+workflow:os_distros al2023

[github-actions]

@cartermckinnon roger that! I've dispatched a workflow. 👍

[github-actions]

@cartermckinnon the workflow that you requested has completed. 🎉

AMI variant	Build	Test
1.23 / al2023	success ✅	success ✅
1.24 / al2023	success ✅	success ✅
1.25 / al2023	success ✅	success ✅
1.26 / al2023	success ✅	success ✅
1.27 / al2023	success ✅	success ✅
1.28 / al2023	success ✅	success ✅
1.29 / al2023	success ✅	success ✅
1.30 / al2023	success ✅	success ✅

[cartermckinnon]

Alright, there we go. Problem was CAP_NET_ADMIN.

[Issacwww]

out of curiosity, how this been generated? Is there any reference I can read to understand more in case we need to debug sth for this?

[cartermckinnon]

I derived this from a few sources:

1. The default base OCI spec given by ctr oci spec.
2. An actual spec for a running container grabbed with ctr container info.
3. The base spec used by Bottlerocket for this same purpose (providing RLIMIT overrides): https://github.com/bottlerocket-os/bottlerocket/blob/b3e476f382dbaf6bb6bea20d99d3bf143662cef6/packages/containerd/containerd-cri-base-json#L4

The spec itself is thoroughly documented here: https://github.com/opencontainers/runtime-spec

[ndbaker1]

LGTM, thanks for tackling this 👏

[bjhaid]

when will an AMI containing this be released?

[bjhaid]

There was a pre-release 3 hours ago https://github.com/awslabs/amazon-eks-ami/releases/tag/v20240605 that does not contain this PR. @ndbaker1 @Issacwww what needs to be done to get this changes in the next AMI release?

[cartermckinnon]

This will be included in an AMI next week.

[msvechla]

Is there an update on when this will be released? We are waiting for this hotfix

[bjhaid]

@cartermckinnon the week is almost over 😢 . We are actively blocked from completing our upgrade and this AMI will unblock us

[cartermckinnon]

This is going out today in v20240615: https://github.com/awslabs/amazon-eks-ami/releases/tag/v20240615