Standalone containerd #4757

brandond · 2021-12-15T23:33:19Z

Proposed Changes

Build and ship standalone containerd

Types of Changes

packaging

Verification

ls -la /var/lib/rancher/k3s/data/current/bin/* | grep containerd

Linked Issues

Cut 1.23.0+k3s1 #4689

User-Facing Change

The embedded containerd binary is no longer part of the K3s multicall bundle.

Further Comments

Oats87 · 2021-12-15T23:37:22Z

scripts/build

-# CGO_ENABLED=0 "${GO}" build -tags "$TAGS" -ldflags "$VERSIONFLAGS $LDFLAGS $STATIC" -o bin/containerd ./cmd/containerd/
+
+echo Building k3s
+CGO_ENABLED=1 "${GO}" build -tags "$TAGS" -ldflags "$VERSIONFLAGS $LDFLAGS $STATIC" -o bin/k3s ./cmd/server/main.go


It's great to move to k3s instead of the existing containerd multi-call binary thing we have now.

That being said, rancher/rancher#35723 will pretty much immediately get broken by this change (in addition to any other downstream consumer of K3s that doesn't realize we're making a change like this)

From what @ibuildthecloud said a while back, apparently containerd has (or had at some point) some internal checks as to what the actual filename of the running binary is, so the main multicall entrypoint had to be named containerd. I don't know that we've tried unwinding this at any point, but if we can keep this change even after re-adding it, that would be great.

You are correct that it will break that rancher PR, if they try to update to 1.23 at any point. We'll need to coordinate that.

Yeah, I'm aware of the tribal knowledge there as to why we are using containerd vs. k3s, I'm just trying to call out that we may accidentally break downstream consumers of the project. I think we can manage that on our end (i.e. Rancher projects that consume K3s in "advanced" manners), but there may be others out there that get affected by this. Perhaps we should release note it? but then again, it doesn't affect the user... hmm

i like the release note idea

i'm not too worried about breaking an implied contract, that being where and what executables are in the docker container image.

Oats87 · 2021-12-15T23:39:13Z

scripts/build

-PKG_CRICTL="github.com/kubernetes-sigs/cri-tools"
+PKG_CRICTL="github.com/kubernetes-sigs/cri-tools/pkg"
+PKG_K8S_BASE="k8s.io/component-base"
+PKG_K8S_CLIENT="k8s.io/client-go/pkg"


Are we going to backport these changes to other K3s branches or is this going to be a 1.23+ thing?

If not, maybe we should make these specific changes/optimizations in a separate PR that is then backported to the rest of the branches.

I think I would like to change this, and the move of the other VERSION_ definitions out to versions.sh, across all the branches, just for consistency's sake. I can move them to a separate commit that can be cherry-picked.

Moved out to a separate commit for ease of cherry-picking

Oats87 · 2021-12-15T23:40:02Z

scripts/binary_size_check.sh

@@ -8,7 +8,8 @@ fi

 . ./scripts/version.sh

-MAX_BINARY_SIZE=61000000
+# Try to keep the K3s binary under 60 megabytes
+MAX_BINARY_SIZE=$((60 * 1024 * 1024))


Can we not just fill this in as a constant?

Where would you see it living? The only place we use it is in this script to fail CI if we've bloated things too much.

I mean fill it in statically - i.e. set to 62914560 instead of evaluating it every time the script is run. This is a nit though.

Ah. I figured it was easier on us as maintainers in this format; I don't off the top of my head know how many MiB 62914560 bytes is.

k3s binary dist/artifacts/k3s size 63610880 exceeds max acceptable size of 62914560 bytes

Maybe time to let bloat set this to 64 MB? 🤣

It definitely is more readable, but IMO it could be a comment.

Either way, it doesn't block anything I was just looking to see if that is something we might want to do.

Regardless, I think the value needs to be bumped anyway because CI appears to be failing due to it being too low.

CI should pass now that I've rebased on top of the k3s-root update. I also bumped it to 64MB for additional headroom.

brandond · 2021-12-16T00:46:21Z

CI is currently failing because this commit violates the size limit if merged without #4758 landing first. Updating k3s-root gets us those precious kbytes back.

k3s binary dist/artifacts/k3s size 63610880 exceeds max acceptable size of 62914560 bytes

Oats87 · 2021-12-16T00:48:33Z

CI is currently failing because this commit violates the size limit if merged without #4758 landing first. Updating k3s-root gets us those precious kbytes back.
k3s binary dist/artifacts/k3s size 63610880 exceeds max acceptable size of 62914560 bytes

How much space is saved with the k3s-root PR?

I (personally) am not opposed to increasing the binary size limit to something like 64 MB, especially if we're going to decrease size quickly afterwards with the other PR.

brandond · 2021-12-16T01:27:38Z

How much space is saved with the k3s-root PR

Just did the math at #4758 (comment)

Oats87 · 2021-12-16T17:12:37Z

So,
63610880-1376256 = 62234624
which leaves us with a 680 KB until we pass the threshold. I don't think that's reasonable.

We know that this PR/enhancement is going to change the binary size of K3s in a non-trivial manner so I don't see why we don't just bump our upper limit to 64 from 60 MB

* Move runc and k3s-root versions into versions.sh * Remove commented-out cruft from build script * Other minor tweaks Signed-off-by: Brad Davidson <[email protected]>

Signed-off-by: Brad Davidson <[email protected]>

brandond · 2021-12-16T19:59:34Z

cgroupv2 flaked; merging

brandond requested a review from a team as a code owner December 15, 2021 23:33

brandond mentioned this pull request Dec 15, 2021

Bump k3s-root to v0.10.1 #4758

Merged

Oats87 reviewed Dec 15, 2021

View reviewed changes

dweomer approved these changes Dec 15, 2021

View reviewed changes

brandond force-pushed the standalone_containerd branch from aa4a541 to 4591dc9 Compare December 16, 2021 00:31

brandond requested a review from Oats87 December 16, 2021 00:32

galal-hussein approved these changes Dec 16, 2021

View reviewed changes

Build script cleanups

86a46cc

* Move runc and k3s-root versions into versions.sh * Remove commented-out cruft from build script * Other minor tweaks Signed-off-by: Brad Davidson <[email protected]>

brandond force-pushed the standalone_containerd branch from 4591dc9 to b34f781 Compare December 16, 2021 17:37

brandond added 2 commits December 16, 2021 09:48

Build standalone containerd

01c1cd8

Signed-off-by: Brad Davidson <[email protected]>

Add ADR

8380426

Signed-off-by: Brad Davidson <[email protected]>

brandond force-pushed the standalone_containerd branch from b34f781 to 8380426 Compare December 16, 2021 17:49

Oats87 approved these changes Dec 16, 2021

View reviewed changes

brandond merged commit baf865b into k3s-io:master Dec 16, 2021

brandond mentioned this pull request Dec 16, 2021

Update Kubernetes to v1.23 #4623

Merged

euank mentioned this pull request Jan 30, 2022

[wip] k3s: 1.22.3+k3s1 -> 1.23.3+k3s1 NixOS/nixpkgs#157004

Closed

brandond mentioned this pull request Apr 21, 2023

Bump Runc and Containerd #7339

Merged

This was referenced May 3, 2023

Move containerd back into the main multicall binary #7419

Closed

Bump containerd to v1.7.0 and move back into multicall binary #7418

Merged

[release-1.26] Bump containerd to v1.7.0 and move back into multicall binary #7444

Merged

brandond deleted the standalone_containerd branch June 6, 2024 21:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standalone containerd #4757

Standalone containerd #4757

brandond commented Dec 15, 2021

Oats87 Dec 15, 2021

brandond Dec 15, 2021 •

edited

Loading

Oats87 Dec 15, 2021

dweomer Dec 15, 2021

dweomer Dec 15, 2021

Oats87 Dec 15, 2021

brandond Dec 15, 2021 •

edited

Loading

brandond Dec 16, 2021

Oats87 Dec 15, 2021

brandond Dec 15, 2021 •

edited

Loading

Oats87 Dec 16, 2021

brandond Dec 16, 2021 •

edited

Loading

Oats87 Dec 16, 2021

Oats87 Dec 16, 2021

brandond Dec 16, 2021

brandond commented Dec 16, 2021

Oats87 commented Dec 16, 2021

brandond commented Dec 16, 2021

Oats87 commented Dec 16, 2021

brandond commented Dec 16, 2021

Standalone containerd #4757

Standalone containerd #4757

Conversation

brandond commented Dec 15, 2021

Proposed Changes

Types of Changes

Verification

Linked Issues

User-Facing Change

Further Comments

Choose a reason for hiding this comment

brandond Dec 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandond Dec 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandond Dec 15, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandond Dec 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brandond commented Dec 16, 2021

Oats87 commented Dec 16, 2021

brandond commented Dec 16, 2021

Oats87 commented Dec 16, 2021

brandond commented Dec 16, 2021

brandond Dec 15, 2021 •

edited

Loading

brandond Dec 15, 2021 •

edited

Loading

brandond Dec 15, 2021 •

edited

Loading

brandond Dec 16, 2021 •

edited

Loading