containerd: Add support for tar.gz package #8199

hakman · 2019-12-24T17:07:49Z

Each containerd release comes with a tar.gz that can be used to install it with minimal effort.
Ref: #7986 (comment)

Summary of changes:

Archive task will extract each required file to the desired location - easier to control what goes where, but may need versioning in the future
the package contains static binaries, so no need to provide a distro like with each rpm/deb
having a single package means dependencies need to go to Packages
support for CentOS should be much simpler now that container-selinux is installed from repo and not vault (vault package is still needed for RHEL and Amazon Linux)

PS1: Docker could get some similar upgrade by moving the deps to Packages and would open the way to multi-arch.

PS2: Amazon Linux 2 doesn't have the packages required to run the latest containerd (and Docker). At least the deps for container-selinux from CentOS are missing. I suspect this distro will need special care if support for it is desired.

k8s-ci-robot · 2019-12-24T17:07:57Z

Hi @hakman. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

hakman · 2019-12-24T17:09:15Z

/cc @justinsb

tanjunchen · 2019-12-25T09:29:29Z

/ok-to-test

johngmyers · 2019-12-25T16:42:27Z

/test pull-kops-verify-packages

tanjunchen · 2019-12-26T10:23:13Z

/test pull-kops-e2e-kubernetes-aws

johngmyers · 2019-12-26T19:13:07Z

/test pull-kops-verify-generated

hakman · 2019-12-29T16:25:05Z

/assign @justinsb

justinsb · 2020-01-11T20:10:03Z

nodeup/pkg/model/containerd.go

+					Hash:      dv.Hash,
+					TargetDir: "/",
+					MapFiles: map[string]string{
+						"./usr/local/bin/ctr":                     "/usr/bin",


I'm wondering if we should map directories, as this likely makes it harder to support new tar.gz bundles we don't yet know about (they may have added a file)...

Very good point. Updated and works perfectly.

justinsb · 2020-01-11T20:12:30Z

nodeup/pkg/model/containerd.go


 	manifest.Set("Service", "LimitNOFILE", "1048576")
 	manifest.Set("Service", "LimitNPROC", "infinity")
 	manifest.Set("Service", "LimitCORE", "infinity")
 	manifest.Set("Service", "TasksMax", "infinity")

-	manifest.Set("Service", "Restart", "always")
-	manifest.Set("Service", "RestartSec", "2s")
-	manifest.Set("Service", "StartLimitInterval", "0")


We might actually need these to prevent systemd marking the unit as failed and not always restarting e.g. https://serverfault.com/questions/736624/systemd-service-automatic-restart-after-startlimitinterval

However, I think with RestartSec = 5 we're OK with the default values, but it is possible that systemd is not running with the default values.

Also annoying is that the field was renamed from StartLimitInterval to StartLimitIntervalSec at some stage; I don't know if any of our OSes uses StartLimitInterval.

We might need a helper function - so we can probably leave this as-is for now :-)

I also saw that Kops doesn't use StartLimitInterval anywhere. Besides the name, it was also moved from [Service] to [Unit] at some point.
https://lists.freedesktop.org/archives/systemd-devel/2017-July/039255.html

Unless someone will complain, I also think we can skip it for now.

justinsb · 2020-01-11T20:37:25Z

nodeup/pkg/model/docker.go

@@ -188,7 +188,7 @@ var dockerVersions = []packageVersion{
 				Hash:    "a6b0243af348140236ed96f2e902b259c590eefa",
 			},
 		},
-		Dependencies: []string{"libtool-ltdl", "libseccomp"},
+		Dependencies: []string{"libtool-ltdl"},


These are the ones we're moving into packages.go?

Yes. Moving to packages.go.

justinsb · 2020-01-11T20:48:14Z

upup/pkg/fi/nodeup/nodetasks/archive.go

+					return fmt.Errorf("error creating directories %q: %v", targetDir, err)
+				}
+
+				args := []string{"tar", "xf", localFile, "-C", targetDir, "--strip-components=" + strconv.Itoa(stripCount), src}


This is going to be slow I think, because we have to read the whole tar file every time (tar files aren't indexed, unlike zip files).

In theory, yes. In practice it's faster than yum install containerd:

I0112 04:43:20.381756 652 files.go:100] Hash matched for "/var/cache/nodeup/archives/containerd.io": sha1:f451d46280104588f236bee277bca1da8babc0e8 I0112 04:43:20.381805 652 archive.go:189] running command [tar xf /var/cache/nodeup/archives/containerd.io -C /usr --strip-components=3 ./usr/local/bin] I0112 04:43:22.038539 652 archive.go:189] running command [tar xf /var/cache/nodeup/archives/containerd.io -C /usr --strip-components=3 ./usr/local/sbin] I0112 04:43:23.601597 652 executor.go:103] Tasks: 2 done / 60 total; 45 can run ``

justinsb · 2020-01-11T20:50:21Z

nodeup/pkg/model/containerd.go

@@ -107,14 +102,6 @@ var containerdVersions = []packageVersion{
 		Version:        "1.2.10",
 		Source:         "https://download.docker.com/linux/centos/7/x86_64/stable/Packages/containerd.io-1.2.10-3.2.el7.x86_64.rpm",
 		Hash:           "f6447e84479df3a58ce04a3da87ccc384663493b",
-		ExtraPackages: map[string]packageInfo{
-			"container-selinux": {


I guess the problem here is that container-selinux might be tied to the behaviour of a particular version of containerd. i.e. a new version of containerd introduces a new required permission (?)

My assumption was that newer container-selinux will only add permissions, which would make it backwards compatible with any containerd or docker version. In a regular install. where one has a valid subscription, any package upgrade would grab the latest version.

justinsb · 2020-01-11T20:51:17Z

Thanks @hakman ... generally looks good but I have some questions!

hakman · 2020-01-12T05:21:52Z

Thanks @justinsb. I think this will make it simpler to support multi-arch. I see that the guys working on containerd are improving their support for multi-arch and hopefully will release official builds soon.
containerd/containerd#3758

Would it make sense to try and move Docker deps to Packages and add a tar.gz package there also?

justinsb · 2020-01-26T15:22:44Z

Thanks for the tweaks - looks great!

/approve
/lgtm

k8s-ci-robot · 2020-01-26T15:23:06Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: hakman, justinsb

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [justinsb]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

justinsb · 2020-01-26T15:24:11Z

For docker, my inclination would be to leave it as-is, and gradually let it age-out out of our code, as I think people will switch to pure containerd. But if there's an advantage to refactoring it, we can do that!

hakman · 2020-01-26T16:37:01Z

/retest

hakman · 2020-01-27T05:39:26Z

/retest

hakman · 2020-01-27T05:46:29Z

/test pull-kops-e2e-kubernetes-aws

johngmyers · 2020-01-27T05:53:55Z

/test pull-kops-e2e-kubernetes-aws

hakman · 2020-01-27T06:19:41Z

/retest

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Dec 24, 2019

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Dec 24, 2019

k8s-ci-robot requested review from joshbranham and robinpercy December 24, 2019 17:08

k8s-ci-robot requested a review from justinsb December 24, 2019 17:09

hakman force-pushed the containerd-tgz branch from 4908f06 to 923c768 Compare December 25, 2019 05:36

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Dec 25, 2019

hakman force-pushed the containerd-tgz branch from 923c768 to dcef0b0 Compare December 26, 2019 09:18

hakman force-pushed the containerd-tgz branch from dcef0b0 to 3f1ae23 Compare December 26, 2019 11:54

hakman changed the title ~~containerd: Add support for tar.gz package~~ [WIP] containerd: Add support for tar.gz package Dec 26, 2019

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 26, 2019

hakman force-pushed the containerd-tgz branch 3 times, most recently from b836268 to 878eec5 Compare December 29, 2019 16:24

k8s-ci-robot assigned justinsb Dec 29, 2019

hakman changed the title ~~[WIP] containerd: Add support for tar.gz package~~ containerd: Add support for tar.gz package Dec 29, 2019

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 29, 2019

hakman force-pushed the containerd-tgz branch from 878eec5 to 08b504c Compare January 11, 2020 10:06

justinsb reviewed Jan 11, 2020

View reviewed changes

Add support for containerd tar.gz package

2a6aeaf

hakman force-pushed the containerd-tgz branch from 08b504c to 2a6aeaf Compare January 12, 2020 04:54

hakman requested a review from justinsb January 12, 2020 05:13

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jan 26, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jan 26, 2020

k8s-ci-robot merged commit 82b3bd5 into kubernetes:master Jan 27, 2020

k8s-ci-robot added this to the v1.18 milestone Jan 27, 2020

hakman deleted the containerd-tgz branch January 28, 2020 04:41

hakman mentioned this pull request Jan 28, 2020

Update support for Amazon Linux 2 #8425

Merged

hakman mentioned this pull request Feb 11, 2020

Update support for docker tgz package #8525

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

containerd: Add support for tar.gz package #8199

containerd: Add support for tar.gz package #8199

hakman commented Dec 24, 2019 •

edited

Loading

k8s-ci-robot commented Dec 24, 2019

hakman commented Dec 24, 2019

tanjunchen commented Dec 25, 2019

johngmyers commented Dec 25, 2019

tanjunchen commented Dec 26, 2019

johngmyers commented Dec 26, 2019

hakman commented Dec 29, 2019

justinsb Jan 11, 2020

hakman Jan 12, 2020

justinsb Jan 11, 2020

hakman Jan 12, 2020

justinsb Jan 11, 2020

hakman Jan 12, 2020

justinsb Jan 11, 2020

hakman Jan 12, 2020

justinsb Jan 11, 2020

hakman Jan 12, 2020

justinsb commented Jan 11, 2020

hakman commented Jan 12, 2020

justinsb commented Jan 26, 2020

k8s-ci-robot commented Jan 26, 2020

justinsb commented Jan 26, 2020

hakman commented Jan 26, 2020

hakman commented Jan 27, 2020

hakman commented Jan 27, 2020

johngmyers commented Jan 27, 2020

hakman commented Jan 27, 2020

containerd: Add support for tar.gz package #8199

containerd: Add support for tar.gz package #8199

Conversation

hakman commented Dec 24, 2019 • edited Loading

k8s-ci-robot commented Dec 24, 2019

hakman commented Dec 24, 2019

tanjunchen commented Dec 25, 2019

johngmyers commented Dec 25, 2019

tanjunchen commented Dec 26, 2019

johngmyers commented Dec 26, 2019

hakman commented Dec 29, 2019

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

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

justinsb commented Jan 11, 2020

hakman commented Jan 12, 2020

justinsb commented Jan 26, 2020

k8s-ci-robot commented Jan 26, 2020

justinsb commented Jan 26, 2020

hakman commented Jan 26, 2020

hakman commented Jan 27, 2020

hakman commented Jan 27, 2020

johngmyers commented Jan 27, 2020

hakman commented Jan 27, 2020

hakman commented Dec 24, 2019 •

edited

Loading