Images Improvements [Breaking Changes] #461

BenTheElder · 2019-04-29T19:59:04Z

Overall this PR will make cluster boot faster and lighter, however it is a breaking change to the images versus the kind binary, newer images will require a newer kind binary and a newer kind binary will require newer images.

Base Image Changes:

switch to Ubuntu 19.04 base (newer dependencies)
switch from dockerd to containerd (lighter, faster, easier to debug)
remove unnecessary packages
add crictl v1.14.0
remove more unnecessary files in clean-install & fix shellcheck lint
entrypoint is now a small shellscript instead of a go binary
- easier portability / reproducibility (COPY only, no host building)
- also smaller, less dependency

Node Image Changes:

preload images in to containerd's content store instead of storing archives (smaller & faster!)

Cluster Create Changes:

stop waiting for dockerd to be ready (containerd boots ~instantly)
stop loading tar archives (already loaded at build time)
No more node fixups on the host side (proxy settings, mounts etc.. entrypoint now handles these)
- No more SIGUSR1
product_name is overridden

The entrypoint changes should make #148 much simpler, we only need to tackle the IP issues.

BenTheElder · 2019-04-29T19:59:37Z

Just making this a breaking change cut down complexity quite a bit, will clean this up some more along with some other fixes and get it in to unblock other work like #453

BenTheElder · 2019-04-29T20:15:16Z

[these will fail until a base image is pushed, also I think < 1.12 bazel build image tags needs addressing, though perhaps we don't need to keep supporting v1.11?]

…

On Mon, Apr 29, 2019 at 1:13 PM Kubernetes Prow Robot < ***@***.***> wrote: @BenTheElder <https://github.com/BenTheElder>: The following tests *failed*, say /retest to rerun them all: Test name Commit Details Rerun command pull-kind-verify 51da10b <51da10b> link <https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/kubernetes-sigs_kind/461/pull-kind-verify/1122953529917444097> /test pull-kind-verify pull-kind-conformance-parallel-1-13 51da10b <51da10b> link <https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/sigs.k8s.io_kind/461/pull-kind-conformance-parallel-1-13/1122953529917444098/> /test pull-kind-conformance-parallel-1-13 pull-kind-conformance-parallel-1-12 51da10b <51da10b> link <https://prow.k8s.io/view/gcs/kubernetes-jenkins/pr-logs/pull/sigs.k8s.io_kind/461/pull-kind-conformance-parallel-1-12/1122953529917444100/> /test pull-kind-conformance-parallel-1-12 Full PR test history <https://prow.k8s.io/pr-history?org=kubernetes-sigs&repo=kind&pr=461>. Your PR dashboard <https://gubernator.k8s.io/pr/BenTheElder>. Please help us cut down on flakes by linking to <https://git.k8s.io/community/contributors/devel/flaky-tests.md#filing-issues-for-flaky-tests> an open issue <https://github.com/kubernetes-sigs/kind/issues?q=is:issue+is:open> when you hit one in your PR. Instructions for interacting with me using PR comments are available here <https://git.k8s.io/community/contributors/guide/pull-requests.md>. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra <https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:> repository. I understand the commands that are listed here <https://go.k8s.io/bot-commands>. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#461 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHADK3C3SEEXKPNXFZ3JOLPS5JGDANCNFSM4HJGJJUA> .

aojea · 2019-04-29T20:39:04Z

images/base/Dockerfile

-      conntrack iptables iproute2 ethtool socat util-linux mount ebtables udev kmod aufs-tools \
-      bash rsync \
+      containerd \
+      conntrack iptables iproute2 ethtool socat util-linux mount ebtables udev kmod \


we need gnupg2 in order to build the node images with apt

/kind build node-image --base-image kindest/base:containerd --image kindest/node:containerd --type apt E: gnupg, gnupg2 and gnupg1 do not seem to be installed, but one of them is required for this operation (23) Failed writing body ERRO[22:37:41] Adding Kubernetes apt key failed! exit status 255 ERRO[22:37:41] Image build Failed! exit status 255 Error: error building node image: exit status 255

alternatively type=apt can install gnupg2 first if we decide not to get rid of it.

BenTheElder · 2019-04-29T20:46:07Z

We need to remove the apt node tbh.

…

On Mon, Apr 29, 2019, 13:39 Antonio Ojea ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In images/base/Dockerfile <#461 (comment)>: > systemd systemd-sysv libsystemd0 \ - conntrack iptables iproute2 ethtool socat util-linux mount ebtables udev kmod aufs-tools \ - bash rsync \ + containerd \ + conntrack iptables iproute2 ethtool socat util-linux mount ebtables udev kmod \ we need gnupg2 in order to build the node images with apt /kind build node-image --base-image kindest/base:containerd --image kindest/node:containerd --type apt E: gnupg, gnupg2 and gnupg1 do not seem to be installed, but one of them is required for this operation (23) Failed writing body ERRO[22:37:41] Adding Kubernetes apt key failed! exit status 255 ERRO[22:37:41] Image build Failed! exit status 255 Error: error building node image: exit status 255 — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#461 (review)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAHADK6EX4U7NLILLPK4UCDPS5MGVANCNFSM4HJGJJUA> .

pkg/build/node/node.go

BenTheElder · 2019-04-29T22:33:32Z

pkg/build/node/node.go

@@ -255,7 +255,7 @@ func (c *BuildContext) buildImage(dir string) error {
 		}()
 	}
 	if err != nil {
-		log.Errorf("Image build Failed! %v", err)
+		log.Errorf("Image build Failed! Failed to create build container: %v", err)


I wound up adding some more details to these to aid debugging while working on this. They're not strictly related, but it's a small change.

pkg/cluster/nodes/node.go

BenTheElder · 2019-04-29T22:40:30Z

pkg/cluster/internal/create/nodes.go

-	}
-
-	// load the docker image artifacts into the docker daemon
-	node.LoadImages()


the equivilant is now done at image build time

BenTheElder · 2019-04-30T00:02:01Z

pkg/cluster/internal/create/actions/config/config.go

@@ -57,7 +57,7 @@ func (a *Action) Execute(ctx *actions.ActionContext) error {
 	kubeVersion, err := node.KubeVersion()
 	if err != nil {
 		// TODO(bentheelder): logging here
-		return errors.Wrap(err, "failed to get kubernetes version from node: %v")
+		return errors.Wrap(err, "failed to get kubernetes version from node")


this formatting was incorrect

BenTheElder · 2019-04-30T00:18:27Z

pkg/cluster/nodes/nodes.go

+
+// helper that calls `try()`` in a loop until the deadline `until`
+// has passed or `try()`returns true, returns wether try ever returned true
+func tryUntil(until time.Time, try func() bool) bool {


this is the same code, but the only user is over here now (directly above) so I moved it.

BenTheElder · 2019-04-30T00:19:29Z

pkg/cluster/nodes/create.go

-	// Deletes the machine-id embedded in the node image and regenerate a new one.
-	// This is necessary because both kubelet and other components like weave net
-	// use machine-id internally to distinguish nodes.
-	if err := handle.Command("rm", "-f", "/etc/machine-id").Run(); err != nil {


entrypoint does this now

BenTheElder · 2019-04-30T00:19:48Z

pkg/cluster/internal/create/nodes.go

-	// we need to change a few mounts once we have the container
-	// we'd do this ahead of time if we could, but --privileged implies things
-	// that don't seem to be configurable, and we need that flag
-	if err := node.FixMounts(); err != nil {


entrypoint does this now

BenTheElder · 2019-04-30T00:19:56Z

pkg/cluster/internal/create/nodes.go

-		return err
-	}
-
-	if nodes.NeedProxy() {


entrypoint does this now

BenTheElder · 2019-04-30T00:20:04Z

pkg/cluster/internal/create/nodes.go

-	}
-
-	// signal the node container entrypoint to continue booting into systemd
-	if err := node.SignalStart(); err != nil {


there's nothing to signal now

BenTheElder · 2019-04-30T00:20:54Z

pkg/cluster/nodes/node.go

-// SignalStart sends SIGUSR1 to the node, which signals our entrypoint to boot
-// see images/node/entrypoint
-func (n *Node) SignalStart() error {
-	return docker.Kill("SIGUSR1", n.name)


we should probably remove the docker.Kill call since this was the only user afaik. that can maybe be another PR though, this PR is already a bit big

BenTheElder · 2019-04-30T00:21:22Z

pkg/cluster/nodes/node.go

@@ -135,105 +125,6 @@ func (n *Node) CopyFrom(source, dest string) error {
 	return docker.CopyFrom(n.name, source, dest)
 }

-// WaitForDocker waits for Docker to be ready on the node
-// it returns true on success, and false on a timeout
-func (n *Node) WaitForDocker(until time.Time) bool {


unnecessary now, containerd starts very very very fast in testing so far (not worth measuring), also we don't need to wait to load images, because we don't need to load images

BenTheElder · 2019-04-30T00:21:33Z

pkg/cluster/nodes/node.go

-}
-
-// LoadImages loads image tarballs stored on the node into docker on the node
-func (n *Node) LoadImages() {


image loading is at build time now

BenTheElder · 2019-04-30T22:48:11Z

/hold cancel
will need follow-ups but this should move things forward a lot and help unblock, will follow-up with:

code cleanup (mostly removing some now dead-code)
HA rework
docs updates
kind build node-image --type=apt fixes
...

amwat

/hold

k8s-ci-robot · 2019-04-30T22:53:24Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: amwat, aojea, BenTheElder

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~~ [BenTheElder]

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

BenTheElder · 2019-04-30T22:53:48Z

/hold cancel

nickolaev · 2019-05-05T09:08:02Z

pkg/cluster/nodes/node.go

-	if err := n.Command("mount", "-o", "remount,ro", "/sys").Run(); err != nil {
-		return err
-	}
-	// kubernetes needs shared mount propagation


@BenTheElder these are omitted in fix_mount in the entrypoint. We are adopting kind as a cluster provider in Network Service Mesh https://github.com/networkservicemesh/networkservicemesh/blob/master/docs/guide-kind.md and the latest version is not working for us. The issues is that we are using /var/lib mounts which is relying on / being a shared mount.
What would be the best approach here? Shall we consider updating fix_mounts?

we can update fix_mounts, sorry about that!

These were a hold over from some previous docker-in-docker tooling predating kind, I haven't found much (anything?) documenting why you'd need to do this on the kubernetes host and conformance was passing without them, so I removed them as voodoo figuring kubelet must handle this 🤦‍♂
Looking again now I do see: https://kubernetes.io/docs/concepts/storage/volumes/#configuration

side note:

It is worth noting that kind as any other Kubernetes deployment tool would expect that the machine that hosts the Docker has at least 4 CPU cores and 4 GB of RAM. That is specifically pointed for OSX users in the official docs.

Filed #485, we should make those more accurate in our docs and then send y'all an update 🙃

filed #486, will send a fix momentarily

nickolaev · 2019-05-05T20:59:20Z

Thanks, @BenTheElder, indeed I have found these mentions about the MountFlags=shared all over the place but was puzzled how this maps to kind. Apparently, it needs to be done manually here.

Signed-off-by: Jean-Christophe Sirot <[email protected]>

Now use official kind node images and adapt how the compose-on-kube images are loaded in kind cluster in e2e tests (images archives in /kind/images are not loaded at cluster creation anymore, see kubernetes-sigs/kind#461) Signed-off-by: Jean-Christophe Sirot <[email protected]>

…sigs#461) * version v0.18.0-alpha * update docs for v0.17.0 * fix kind version in readme * comments-update-buildcontext * trying get oci versions * integrated default behaviour * added managing for chart version * fixed getting last version * updated dependendies * fixed versions sorting * formated error * remove clusterOperatorChart variable * Added cluster_operator_image_version * Updated CHANGELOG --------- Co-authored-by: Benjamin Elder <[email protected]> Co-authored-by: Daman <[email protected]> Co-authored-by: Kubernetes Prow Robot <[email protected]>

k8s-ci-robot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Apr 29, 2019

k8s-ci-robot requested review from munnerz and neolit123 April 29, 2019 19:59

k8s-ci-robot added approved Indicates a PR has been approved by an approver from all required OWNERS files. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 29, 2019

aojea reviewed Apr 29, 2019

View reviewed changes

pkg/build/node/node.go Show resolved Hide resolved

remove manpages in clean-install

273dcae

BenTheElder force-pushed the containerd branch from 51da10b to 4c7269c Compare April 29, 2019 22:31

BenTheElder commented Apr 29, 2019

View reviewed changes

pkg/cluster/nodes/node.go Outdated Show resolved Hide resolved

BenTheElder commented Apr 29, 2019

View reviewed changes

BenTheElder force-pushed the containerd branch from 4c7269c to ac4be47 Compare April 29, 2019 22:41

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Apr 29, 2019

BenTheElder requested review from amwat and krzyzacy April 30, 2019 00:01

BenTheElder commented Apr 30, 2019

View reviewed changes

pkg/cluster/internal/create/nodes.go

return err

}

if nodes.NeedProxy() {

Copy link

Member Author

BenTheElder Apr 30, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entrypoint does this now

BenTheElder commented Apr 30, 2019

View reviewed changes

BenTheElder force-pushed the containerd branch from bbdf9dd to e05bc7e Compare April 30, 2019 21:19

upgrade kindest/node to v1.14.1

0c5176a

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 30, 2019

k8s-ci-robot assigned amwat Apr 30, 2019

amwat approved these changes Apr 30, 2019

View reviewed changes

k8s-ci-robot added do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels Apr 30, 2019

k8s-ci-robot removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 30, 2019

k8s-ci-robot merged commit 194cbde into kubernetes-sigs:master Apr 30, 2019

BenTheElder deleted the containerd branch April 30, 2019 23:07

nickolaev reviewed May 5, 2019

View reviewed changes

BenTheElder mentioned this pull request May 5, 2019

mount fixes should include shared mounts #486

Closed

aojea mentioned this pull request May 6, 2019

Errors while building and running kind #493

Closed

jcsirot pushed a commit to jcsirot/compose-on-kubernetes that referenced this pull request Jul 11, 2019

Adapt pre-loaded images handling due to kubernetes-sigs/kind#461

b7fbef4

Signed-off-by: Jean-Christophe Sirot <[email protected]>

jcsirot pushed a commit to jcsirot/compose-on-kubernetes that referenced this pull request Jul 11, 2019

Adapt pre-loaded images handling due to kubernetes-sigs/kind#461

13fd2df

Signed-off-by: Jean-Christophe Sirot <[email protected]>

jcsirot pushed a commit to jcsirot/compose-on-kubernetes that referenced this pull request Jul 11, 2019

Adapt pre-loaded images handling due to kubernetes-sigs/kind#461

784cf7d

Signed-off-by: Jean-Christophe Sirot <[email protected]>

pacoxu mentioned this pull request Mar 31, 2022

Define how to handle kinder base image kubernetes/kubeadm#1759

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Images Improvements [Breaking Changes] #461

Images Improvements [Breaking Changes] #461

BenTheElder commented Apr 29, 2019 •

edited

Loading

BenTheElder commented Apr 29, 2019

BenTheElder commented Apr 29, 2019 via email

aojea Apr 29, 2019

BenTheElder Apr 30, 2019

BenTheElder commented Apr 29, 2019 via email

BenTheElder Apr 29, 2019

BenTheElder Apr 29, 2019

BenTheElder Apr 30, 2019

BenTheElder Apr 30, 2019

BenTheElder Apr 30, 2019

BenTheElder Apr 30, 2019

BenTheElder Apr 30, 2019

BenTheElder Apr 30, 2019

BenTheElder Apr 30, 2019

BenTheElder Apr 30, 2019

BenTheElder Apr 30, 2019

BenTheElder commented Apr 30, 2019

amwat left a comment

k8s-ci-robot commented Apr 30, 2019

BenTheElder commented Apr 30, 2019

nickolaev May 5, 2019

BenTheElder May 5, 2019

BenTheElder May 5, 2019

nickolaev commented May 5, 2019

Images Improvements [Breaking Changes] #461

Images Improvements [Breaking Changes] #461

Conversation

BenTheElder commented Apr 29, 2019 • edited Loading

BenTheElder commented Apr 29, 2019

BenTheElder commented Apr 29, 2019 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenTheElder commented Apr 29, 2019 via email

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

Choose a reason for hiding this comment

BenTheElder commented Apr 30, 2019

amwat left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Apr 30, 2019

BenTheElder commented Apr 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nickolaev commented May 5, 2019

BenTheElder commented Apr 29, 2019 •

edited

Loading