runtime toggle #551

peebs · 2016-06-21T22:31:20Z

This adds the ability to toggle container runtime in coreos-kubernetes.

This is still a WIP but mostly complete and blocked on some external factors. To use rktnetes you just set the runtime flag to rkt and the release channel to alpha if using kube-aws.

One temporary workaround is escaping the container chroot for invocations of rkt. I expect to merge this for now though.

Updates #501

yifan-gu · 2016-06-21T22:45:30Z

cc @tmrts @s-urbaniak @euank

yifan-gu · 2016-06-21T22:49:45Z

multi-node/aws/pkg/config/templates/cloud-config-worker

        ExecStart=/usr/lib/coreos/kubelet-wrapper \
        --api-servers={{.SecureAPIServers}} \
        --network-plugin-dir=/etc/kubernetes/cni/net.d \
        --network-plugin={{.K8sNetworkPlugin}} \
+        --container-runtime=rkt \
+        --rkt-path=/usr/bin/rkt \
+        --rkt-stage1-image=/usr/lib/rkt/stage1-images/stage1-coreos.aci \


Will need to change this to the image name in the next beta: coreos.com/rkt/stage1-coreos to enable stage1 caching.

What do you mean by next beta? Should just changing that path to https://github.com/coreos/rkt/blob/master/Documentation/configuration.md work? I don't have this working locally yet.

@pbx0 I mean since now kubernetes/kubernetes#27707 is merged, so instead of using the path to the aci, we will change to using the stage1's name (to enable caching instead of unpacking the image every time)

@pbx0 This will take effect after we build another k8s release

yifan-gu · 2016-06-23T00:03:30Z

@pbx0 Could you point me a guide on how to use this?

yifan-gu · 2016-06-23T20:39:01Z

multi-node/aws/pkg/config/templates/cloud-config-controller

        ExecStart=/usr/lib/coreos/kubelet-wrapper \
        --api-servers=http://localhost:8080 \
        --network-plugin-dir=/etc/kubernetes/cni/net.d \
        --network-plugin={{.K8sNetworkPlugin}} \
+        --container-runtime={{.ContainerRuntime}} \
+        --rkt-path=/usr/bin/rkt \
+        --rkt-stage1-image=/usr/lib/rkt/stage1-images/stage1-coreos.aci \


Needs to be changed to coreos.com/rkt/stage1-coreos

Quentin-M · 2016-07-11T16:20:44Z

Bump @pbx0

peebs · 2016-07-11T17:34:45Z

@Quentin-M updated the description to match the current state of this PR

Quentin-M · 2016-07-11T17:40:56Z

Thanks.

Quentin-M · 2016-07-11T17:41:47Z

As a side note, we might have to disable rkt-gc when the CNI plugin is being used on the kubelet to let recycle the pods/networks itself.

aaronlevy · 2016-07-11T17:42:24Z

@pbx0 I just merged the v1.3 changes -- would you be able to rebase this to minimize the changeset?

peebs · 2016-07-11T21:52:53Z

rebased

euank · 2016-07-11T22:53:19Z

multi-node/generic/controller-install.sh

@@ -33,6 +33,9 @@ export DNS_SERVICE_IP=10.3.0.10
 # Whether to use Calico for Kubernetes network policy.
 export USE_CALICO=false

+# Determines the container runtime for kubernetes to use. Accepts 'docker' or 'rkt'.
+export RUNTIME=docker


nit, CONTAINER_RUNTIME seems clearer.

For consistency, should this be K8S_CONTAINER_RUNTIME (to match the network-plugin one)?

I'll change it to CONTAINER_RUNTIME. The K8S prefix seems a bit repetitive since we are clearly (to me at least) configuring kubernetes. The use of that prefix already seems inconsistent in my mind anyway.

aaronlevy · 2016-07-14T00:16:19Z

multi-node/aws/pkg/config/config.go

@@ -151,7 +153,7 @@ func (c Cluster) Config() (*Config, error) {
 	config.APIServers = fmt.Sprintf("http://%s:8080", c.ControllerIP)
 	config.SecureAPIServers = fmt.Sprintf("https://%s:443", c.ControllerIP)
 	config.APIServerEndpoint = fmt.Sprintf("https://%s", c.ExternalDNSName)
-	if config.UseCalico {
+	if config.UseCalico || config.ContainerRuntime == "rkt" {


Unless there's a specific reason, we should just start always using CNI

Added a commit that moves everything to cni and takes away the complication here.

colhom · 2016-08-11T23:17:18Z

single-node/user-data


 [Install]
+RequiredBy=rkt-api.service
+Before=rkt-api.service
+WantedBy=multi-user.target


colhom · 2016-08-11T23:18:43Z

@pbx0 looks good minus the WantedBy stuff. Anywhere you have a service that is already RequiredBy dependents, you don't want to say WantedBy: multi-user.target.

peebs · 2016-08-11T23:26:11Z

@colhom Thanks for the massive help fixing my systemd units. But, since kubelet.service has dependent units, we also want to git rid of the multi-user.target thing there as well, right?

Or rather is it because kubelet.service isn't a dependancy for another unit, we do want it?

dghubble · 2016-08-11T23:32:23Z

multi-node/generic/controller-install.sh

+ExecStart=/usr/bin/rkt fetch /usr/lib/rkt/stage1-images/stage1-coreos.aci /usr/lib/rkt/stage1-images/stage1-fly.aci  --insecure-options=image
+
+[Install]
+RequiredBy=rkt-api.service


Why not have rkt-api use a Requires and After under unit? Since these requirements are the rkt-api's expectations.

If we do that, then we have to start the rkt-api and start the kubelet. The way we have it now, we just start the kubelet and systemd will start the rkt-api for us.

I think this is right, but, systemd units have been confusing me today. @colhom?

Yeah, you can disregard now.

peebs · 2016-08-11T23:49:13Z

OK, I believe all comments are addressed up to this point and we've settled on the unit stuff.

colhom · 2016-08-11T23:51:10Z

multi-node/aws/pkg/config/templates/cloud-config-controller

+        RestartSec=10
+        [Install]
+        RequiredBy=kubelet.service
+        Before=kubelet.service


Oops sorry @pbx0, Before and After both go in the [Unit] section, not [Install]

Man, I am terrible at following directions regarding systemd units. Fixing now.

colhom · 2016-08-11T23:53:25Z

@pbx0 missed something in my review, @dghubble pointed it out to me. Before= directive goes in the [Unit] section. Anywhere you have it in [Install], just move it up.

colhom · 2016-08-11T23:53:36Z

After that, LGTM from me.

Must disable passing the bridge ip parameter from flannel to docker directly or else two bridge with the same IP will be created.

Previously the /sys mount wouldn't work without stage1 fly and docker didn't like the /proc mount point.

For single-node and mult-node vagrant, we will default back to CoreOS alpha. For kube-aws, we will exit if the image version is less then 1122.0.0 when using rkt as the runtime.

derekparker · 2016-08-12T00:18:19Z

multi-node/aws/pkg/config/templates/cloud-config-controller

+      #!/bin/sh
+      # This is bind mounted into the kubelet rootfs and all rkt shell-outs go
+      # through this rkt wrapper. It essentially enters the host mount namespace
+      # (which is it already in) only for the purpose of breaking out of the chroot


nit: s/is it/it is/

Calling rkt from the kubelet fly container will call rkt in such a way that its invocation will break out of the container chroot. This should be removed once the write-api stuff is finalized in rktnetes or made a rkt fly feature. See: rkt/rkt#2878

peebs · 2016-08-12T16:55:07Z

OK, now I think we've got the unit file stuff sorted out.

colhom · 2016-08-12T21:48:27Z

Thanks @pbx0 for shepherding this through, great job rktnetes team.

* Set the Docker bridge IP and IP masq to empty string * coreos/coreos-kubernetes#551

peebs force-pushed the rktnetes branch from e7cc212 to 5d6b160 Compare June 21, 2016 22:32

yifan-gu reviewed Jun 21, 2016
View reviewed changes

peebs force-pushed the rktnetes branch from 5d6b160 to 7409035 Compare June 22, 2016 00:34

yifan-gu reviewed Jun 23, 2016
View reviewed changes

peebs mentioned this pull request Jul 5, 2016

incompatibilities for containerized kubelet with rkt as container runtime rkt/rkt#2629

Closed

peebs force-pushed the rktnetes branch 2 times, most recently from 64c5172 to e27d527 Compare July 7, 2016 05:04

peebs force-pushed the rktnetes branch from e27d527 to 0baa5a3 Compare July 11, 2016 21:52

euank reviewed Jul 11, 2016
View reviewed changes

steveej mentioned this pull request Jul 14, 2016

cni plugin + flannel on v1.3: pods can't route to service IPs #572

Closed

aaronlevy reviewed Jul 14, 2016
View reviewed changes

colhom reviewed Aug 11, 2016
View reviewed changes

single-node/user-data

[Install]

RequiredBy=rkt-api.service

Before=rkt-api.service

WantedBy=multi-user.target

Copy link

Contributor

colhom Aug 11, 2016

Choose a reason for hiding this comment

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

nix

peebs force-pushed the rktnetes branch from 69286d8 to e4a5673 Compare August 11, 2016 23:24

dghubble reviewed Aug 11, 2016
View reviewed changes

colhom reviewed Aug 11, 2016
View reviewed changes

peebs force-pushed the rktnetes branch 2 times, most recently from 2127dcd to 7f582cc Compare August 12, 2016 00:08

Patrick Baxter added 6 commits August 11, 2016 17:08

all: use cni by default

7ad1347

Must disable passing the bridge ip parameter from flannel to docker directly or else two bridge with the same IP will be created.

all: enable runtime toggle to support rkt

4f0a9eb

all: kube-proxy use stage1-fly

9c9c422

Previously the /sys mount wouldn't work without stage1 fly and docker didn't like the /proc mount point.

vendor: add github.com/coreos/go-semver/semver

5809f31

all: rkt runtime requires CoreOS alpha

7b9f1f0

For single-node and mult-node vagrant, we will default back to CoreOS alpha. For kube-aws, we will exit if the image version is less then 1122.0.0 when using rkt as the runtime.

docs: document using rkt as container runtime

31e3341

peebs force-pushed the rktnetes branch from 7f582cc to d622c69 Compare August 12, 2016 00:08

derekparker reviewed Aug 12, 2016
View reviewed changes

all: rktnetes hack for rkt gc to work

78acfe9

Calling rkt from the kubelet fly container will call rkt in such a way that its invocation will break out of the container chroot. This should be removed once the write-api stuff is finalized in rktnetes or made a rkt fly feature. See: rkt/rkt#2878

peebs force-pushed the rktnetes branch from d622c69 to 78acfe9 Compare August 12, 2016 00:36

dghubble mentioned this pull request Aug 12, 2016

examples: Add Kubernetes with rkt runtime (rktnetes) poseidon/matchbox#313

Merged

colhom merged commit 80c11ad into coreos:master Aug 12, 2016

peebs deleted the rktnetes branch August 12, 2016 22:06

aaronlevy mentioned this pull request Sep 28, 2016

rkt: add a flag to enable rkt support #501

Closed

dghubble added a commit to poseidon/matchbox that referenced this pull request Oct 9, 2016

examples/k8s: Use CNI for Kubernetes clusters

4daf997

* Set the Docker bridge IP and IP masq to empty string * coreos/coreos-kubernetes#551

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime toggle #551

runtime toggle #551

peebs commented Jun 21, 2016 •

edited

Loading

yifan-gu commented Jun 21, 2016

yifan-gu Jun 21, 2016

peebs Jun 21, 2016

yifan-gu Jun 22, 2016

yifan-gu Jun 22, 2016

yifan-gu commented Jun 23, 2016

yifan-gu Jun 23, 2016

Quentin-M commented Jul 11, 2016

peebs commented Jul 11, 2016

Quentin-M commented Jul 11, 2016

Quentin-M commented Jul 11, 2016

aaronlevy commented Jul 11, 2016

peebs commented Jul 11, 2016

euank Jul 11, 2016

peebs Jul 11, 2016

aaronlevy Jul 14, 2016

peebs Jul 14, 2016

colhom Aug 11, 2016

colhom commented Aug 11, 2016 •

edited

Loading

peebs commented Aug 11, 2016 •

edited

Loading

dghubble Aug 11, 2016

peebs Aug 11, 2016

dghubble Aug 12, 2016

peebs commented Aug 11, 2016

colhom Aug 11, 2016

peebs Aug 11, 2016

colhom commented Aug 11, 2016

colhom commented Aug 11, 2016

derekparker Aug 12, 2016

peebs Aug 12, 2016

peebs commented Aug 12, 2016

colhom commented Aug 12, 2016

runtime toggle #551

runtime toggle #551

Conversation

peebs commented Jun 21, 2016 • edited Loading

yifan-gu commented Jun 21, 2016

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

yifan-gu commented Jun 23, 2016

Choose a reason for hiding this comment

Quentin-M commented Jul 11, 2016

peebs commented Jul 11, 2016

Quentin-M commented Jul 11, 2016

Quentin-M commented Jul 11, 2016

aaronlevy commented Jul 11, 2016

peebs commented Jul 11, 2016

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

colhom commented Aug 11, 2016 • edited Loading

peebs commented Aug 11, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peebs commented Aug 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

colhom commented Aug 11, 2016

colhom commented Aug 11, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peebs commented Aug 12, 2016

colhom commented Aug 12, 2016

peebs commented Jun 21, 2016 •

edited

Loading

colhom commented Aug 11, 2016 •

edited

Loading

peebs commented Aug 11, 2016 •

edited

Loading