-
Notifications
You must be signed in to change notification settings - Fork 519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update containerd to 1.4.3, runc to 1.0.0-rc93 #1336
update containerd to 1.4.3, runc to 1.0.0-rc93 #1336
Conversation
Drop all backported patches and security fixes, since they are in the new release. Drop the SELinux patches, to take advantage of the improved support. Remove the socat dependency, since it is no longer used for CRI port forwarding. Signed-off-by: Ben Cressey <[email protected]>
This fixes an issue where volume mounts received the `cache_t` label, which is not writable by most processes. Signed-off-by: Ben Cressey <[email protected]>
Set the version and commit strings for `runc -v` output. Drop the /dev/mqueue patch, which was fixed upstream: opencontainers/runc#2558 Signed-off-by: Ben Cressey <[email protected]>
CRI has its own implementation for port forwarding now, and there are no other packages that depend on this. Signed-off-by: Ben Cressey <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice
@bcressey Were you able to verify that these are true for containers launched via Docker as well? Testing in the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd like to see this tested with Docker as well before merge, but the code LGTM.
Docker wouldn't have been affected by any of the CRI changes, so I didn't include it in the test results. I've confirmed that the results are what I expected after landing #1315 and #1318:
I noted one discrepancy between Docker and CRI which is that the former will relabel volume mounts for "shared" usage, meaning the MCS pair is not used, while the latter uses the MCS pair. I have it in my list of things to sort out as part of making MCS isolation work. Containers in Docker have the ulimits from For port forwarding I wasn't quite sure what to test, but I verified that remote connectivity via |
Issue number:
Fixes #1299
Description of changes:
Update containerd to 1.4.3. The preliminary work in #1318 lets us drop our SELinux-related patches in favor of the upstream logic, though we now have a new one to fix volume mount labels. We're also able to drop the dependency on socat and remove that package now that CRI has its own port-forwarding implementation.
Note that MCS isolation is not yet enforced even though containerd 1.4 is allocating the MCS pairs. We need changes to our SELinux policy to enable this.
Update runc to 1.0.0-rc93. The
/dev/mqueue
issue was fixed upstream, so we no longer need our patch.Testing done:
Launched an ECS task and verified that it ran correctly. Ran conformance tests for 1.18 on x86_64, and 1.19 on aarch64. Tests passed.
Verified that process labels for unprivileged containers launched through CRI had MCS pairs by default, and that mount labels for all containers had MCS pairs by default. Privileged containers run as
control_t
and unprivileged containers run ascontainer_t
.Verified that port-forwarding worked by launching a redis pod and connecting to it with a local client. The redis test exposed the issue with volume mount labels - a
SAVE
command returned an error because the database could not write to the/data
path. Confirmed thatSAVE
works correctly with the new CRI patch.Verified that containers launched via CRI have the 65536 soft file descriptor limit by default.
Verified that
runc
shows the new version information.Terms of contribution:
By submitting this pull request, I agree that this contribution is dual-licensed under the terms of both the Apache License, version 2.0, and the MIT license.