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

runtime toggle #551

Merged
merged 7 commits into from Aug 12, 2016

Conversation

Projects
None yet
9 participants
@pbx0
Member

pbx0 commented Jun 21, 2016

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

This comment has been minimized.

Show comment
Hide comment
@yifan-gu
Member

yifan-gu commented Jun 21, 2016

Show outdated Hide outdated single-node/user-data Outdated
@yifan-gu

This comment has been minimized.

Show comment
Hide comment
@yifan-gu

yifan-gu Jun 23, 2016

Member

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

Member

yifan-gu commented Jun 23, 2016

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

Show outdated Hide outdated single-node/user-data Outdated
Show outdated Hide outdated single-node/user-data Outdated
@Quentin-M

This comment has been minimized.

Show comment
Hide comment
@Quentin-M

Quentin-M commented Jul 11, 2016

Bump @pbx0

@pbx0

This comment has been minimized.

Show comment
Hide comment
@pbx0

pbx0 Jul 11, 2016

Member

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

Member

pbx0 commented Jul 11, 2016

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

@Quentin-M

This comment has been minimized.

Show comment
Hide comment
@Quentin-M

Quentin-M commented Jul 11, 2016

Thanks.

@Quentin-M

This comment has been minimized.

Show comment
Hide comment
@Quentin-M

Quentin-M Jul 11, 2016

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.

Quentin-M commented Jul 11, 2016

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

This comment has been minimized.

Show comment
Hide comment
@aaronlevy

aaronlevy Jul 11, 2016

Member

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

Member

aaronlevy commented Jul 11, 2016

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

@pbx0

This comment has been minimized.

Show comment
Hide comment
@pbx0

pbx0 Jul 11, 2016

Member

rebased

Member

pbx0 commented Jul 11, 2016

rebased

Show outdated Hide outdated single-node/user-data Outdated
Show outdated Hide outdated single-node/user-data Outdated
@pbx0

This comment has been minimized.

Show comment
Hide comment
@pbx0

pbx0 Jul 14, 2016

Member

think I addressed the comments. Whats left to do:

  • Docs
  • #572
  • Figure out kube-proxy or ship it as a systemd unit.
Member

pbx0 commented Jul 14, 2016

think I addressed the comments. Whats left to do:

  • Docs
  • #572
  • Figure out kube-proxy or ship it as a systemd unit.
@robszumski

This comment has been minimized.

Show comment
Hide comment
@robszumski

robszumski Jul 14, 2016

Member

@pbx0 What are you thinking for docs? Are we moving everything over? Or is it an additional guide?

Member

robszumski commented Jul 14, 2016

@pbx0 What are you thinking for docs? Are we moving everything over? Or is it an additional guide?

@pbx0

This comment has been minimized.

Show comment
Hide comment
@pbx0

pbx0 Jul 15, 2016

Member

@robszumski I'm not entirely sure yet. Updating the docs for coreos-kubernetes is fairly straightforward, I just instruct people to set the CONTAINER_RUNTIME flag and use a recent CoreOS version. Additionally, the more generic guide about configuring rkt as a runtime on kubernetes mostly just talks about the flags you set on the kubelet. See: rkt/rkt#2918

I don't know what to do about the manual kubernetes on coreos guides. It seems not great to pepper in a bunch what are essentially if statements in the guide that amount to do X if your configuring docker as the runtime, do Y if your configuring rkt as the runtime.

Member

pbx0 commented Jul 15, 2016

@robszumski I'm not entirely sure yet. Updating the docs for coreos-kubernetes is fairly straightforward, I just instruct people to set the CONTAINER_RUNTIME flag and use a recent CoreOS version. Additionally, the more generic guide about configuring rkt as a runtime on kubernetes mostly just talks about the flags you set on the kubelet. See: rkt/rkt#2918

I don't know what to do about the manual kubernetes on coreos guides. It seems not great to pepper in a bunch what are essentially if statements in the guide that amount to do X if your configuring docker as the runtime, do Y if your configuring rkt as the runtime.

Show outdated Hide outdated multi-node/generic/worker-install.sh Outdated
Show outdated Hide outdated single-node/user-data Outdated
Show outdated Hide outdated single-node/user-data Outdated
@colhom

This comment has been minimized.

Show comment
Hide comment
@colhom

colhom Aug 11, 2016

Contributor

@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.

Contributor

colhom commented Aug 11, 2016

@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.

@pbx0

This comment has been minimized.

Show comment
Hide comment
@pbx0

pbx0 Aug 11, 2016

Member

@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?

Member

pbx0 commented Aug 11, 2016

@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?

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

This comment has been minimized.

@dghubble

dghubble Aug 11, 2016

Contributor

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

@dghubble

dghubble Aug 11, 2016

Contributor

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

This comment has been minimized.

@pbx0

pbx0 Aug 11, 2016

Member

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?

@pbx0

pbx0 Aug 11, 2016

Member

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?

This comment has been minimized.

@dghubble

dghubble Aug 12, 2016

Contributor

Yeah, you can disregard now.

@dghubble

dghubble Aug 12, 2016

Contributor

Yeah, you can disregard now.

RestartSec=10
[Install]
RequiredBy=kubelet.service

This comment has been minimized.

@dghubble

dghubble Aug 11, 2016

Contributor

Requires and After kubelet.service under [Unit] here?

@dghubble

dghubble Aug 11, 2016

Contributor

Requires and After kubelet.service under [Unit] here?

This comment has been minimized.

@dghubble

dghubble Aug 11, 2016

Contributor

er, nvm. It would be a change in the Kubelet unit.

@dghubble

dghubble Aug 11, 2016

Contributor

er, nvm. It would be a change in the Kubelet unit.

This comment has been minimized.

@dghubble

dghubble Aug 11, 2016

Contributor

Alright. I see you'd rather it be here because this is a bash script that supports both docker and rkt and you'd rather condition the child units, than the [Unit] section of the kubelet.service. Disregard.

@colhom

@dghubble

dghubble Aug 11, 2016

Contributor

Alright. I see you'd rather it be here because this is a bash script that supports both docker and rkt and you'd rather condition the child units, than the [Unit] section of the kubelet.service. Disregard.

@colhom

@pbx0

This comment has been minimized.

Show comment
Hide comment
@pbx0

pbx0 Aug 11, 2016

Member

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

Member

pbx0 commented Aug 11, 2016

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

@colhom

This comment has been minimized.

Show comment
Hide comment
@colhom

colhom Aug 11, 2016

Contributor

@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.

Contributor

colhom commented Aug 11, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@colhom

colhom Aug 11, 2016

Contributor

After that, LGTM from me.

Contributor

colhom commented Aug 11, 2016

After that, LGTM from me.

pbx0 added some commits May 27, 2016

all: kube-proxy use stage1-fly
Previously the /sys mount wouldn't work without stage1 fly and docker
didn't like the /proc mount point.
all: use cni by default
Must disable passing the bridge ip parameter from flannel to docker
directly or else two bridge with the same IP will be created.
all: rkt runtime requires CoreOS alpha
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.
all: rktnetes hack for rkt gc to work
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
@pbx0

This comment has been minimized.

Show comment
Hide comment
@pbx0

pbx0 Aug 12, 2016

Member

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

Member

pbx0 commented Aug 12, 2016

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

@colhom

This comment has been minimized.

Show comment
Hide comment
@colhom

colhom Aug 12, 2016

Contributor

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

Contributor

colhom commented Aug 12, 2016

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

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

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pbx0 pbx0 deleted the pbx0:rktnetes branch Aug 12, 2016

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

examples/k8s: Use CNI for Kubernetes clusters
* Set the Docker bridge IP and IP masq to empty string
* coreos/coreos-kubernetes#551
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment