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

Experimental support for running kubelet in container #6936

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@pmorie
Member

pmorie commented Apr 16, 2015

WIP for #6848

@googlebot

This comment has been minimized.

Show comment
Hide comment
@googlebot

googlebot Apr 16, 2015

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

googlebot commented Apr 16, 2015

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project, in which case you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@googlebot googlebot added the cla: no label Apr 16, 2015

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 16, 2015

Member

So far this PR:

  1. Injects mounters into volume plugins
  2. Adds a new mounter that knows how to nsenter the host's mount namespace from a container
  3. Runs a containerized flag to the kubelet
  4. Turns off kubelet joining cgroup to use devices (will be handled with docker --privileged option) when kubelet is containerized
  5. Injects an nsentering mounter into plugins when kubelet is containerized

It builds; tests are going to be broken.

Next up: make an image that has this kubelet binary and nsenter and test.

Member

pmorie commented Apr 16, 2015

So far this PR:

  1. Injects mounters into volume plugins
  2. Adds a new mounter that knows how to nsenter the host's mount namespace from a container
  3. Runs a containerized flag to the kubelet
  4. Turns off kubelet joining cgroup to use devices (will be handled with docker --privileged option) when kubelet is containerized
  5. Injects an nsentering mounter into plugins when kubelet is containerized

It builds; tests are going to be broken.

Next up: make an image that has this kubelet binary and nsenter and test.

@pmorie

This comment has been minimized.

Show comment
Hide comment
Member

pmorie commented Apr 16, 2015

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 17, 2015

Member

How could I forget @yifan-gu @dchen1107

Member

pmorie commented Apr 17, 2015

How could I forget @yifan-gu @dchen1107

@vishh

This comment has been minimized.

Show comment
Hide comment
@vishh

vishh Apr 17, 2015

Member

This hack LGTM.

Member

vishh commented Apr 17, 2015

This hack LGTM.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 17, 2015

Member

@vishh Yep, this is 99% hack for POC purposes. The mounter injection would be the 1% that we can use.

Member

pmorie commented Apr 17, 2015

@vishh Yep, this is 99% hack for POC purposes. The mounter injection would be the 1% that we can use.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 17, 2015

Member

Now with more hacks. Tmpfs e2e runs locally with kubelet running in container; secrets doesn't. Next step will be to investigate secrets.

Member

pmorie commented Apr 17, 2015

Now with more hacks. Tmpfs e2e runs locally with kubelet running in container; secrets doesn't. Next step will be to investigate secrets.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 20, 2015

Member

Code in this branch works with docker 1.6. Currently, there's a dependency on 1.6 because it switches the mount propogation mode of bind mounts from MOUNT_PRIVATE to MOUNT_SLAVE, so mounts made to host fs volumes (made from the host's mount ns) bind-mounted into containers will be propagated to the bind-mounts. Docker 1.5 uses MOUNT_PRIVATE. I've tested by doing an e2e run locally. There are some cases that don't work, but they appear to be the usual suspects and most of them shouldn't be run anyway against local (will open PRs up to skip these).

One nit that I found: mounts for volumes are not currently cleaned up when the kubelet is run in a container (ie, umount fails with device or resource busy). Those mount points may show as busy because they're under the bind-mount of the kubelet root dir. Need to do more digging on this to follow up.

With that said, I'm curious about how folks feel about some of this code going in so that we can experiment further without carrying the patch. I would want to:

  1. Rebase on top of #6400 when it goes in
  2. Pull the mounter to use up to being a field of the kubelet that the creator of the kubelet can supply (instead of having the getMounter method that would introduce a dependency on `NsenterMounter)
  3. Pull the NsenterMounter out for now or use some other special casing around it which is clearly marked as experimental

Unit and integration tests still broken; will fix those up next.

Thanks to @eparis and @vbatts again for support on getting this working.

Any thoughts @thockin @vishk @smarterclayton?

Member

pmorie commented Apr 20, 2015

Code in this branch works with docker 1.6. Currently, there's a dependency on 1.6 because it switches the mount propogation mode of bind mounts from MOUNT_PRIVATE to MOUNT_SLAVE, so mounts made to host fs volumes (made from the host's mount ns) bind-mounted into containers will be propagated to the bind-mounts. Docker 1.5 uses MOUNT_PRIVATE. I've tested by doing an e2e run locally. There are some cases that don't work, but they appear to be the usual suspects and most of them shouldn't be run anyway against local (will open PRs up to skip these).

One nit that I found: mounts for volumes are not currently cleaned up when the kubelet is run in a container (ie, umount fails with device or resource busy). Those mount points may show as busy because they're under the bind-mount of the kubelet root dir. Need to do more digging on this to follow up.

With that said, I'm curious about how folks feel about some of this code going in so that we can experiment further without carrying the patch. I would want to:

  1. Rebase on top of #6400 when it goes in
  2. Pull the mounter to use up to being a field of the kubelet that the creator of the kubelet can supply (instead of having the getMounter method that would introduce a dependency on `NsenterMounter)
  3. Pull the NsenterMounter out for now or use some other special casing around it which is clearly marked as experimental

Unit and integration tests still broken; will fix those up next.

Thanks to @eparis and @vbatts again for support on getting this working.

Any thoughts @thockin @vishk @smarterclayton?

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 27, 2015

Member

Rebased, will test later. Next up for this is fixing tests. Still waiting on #6400.

Member

pmorie commented Apr 27, 2015

Rebased, will test later. Next up for this is fixing tests. Still waiting on #6400.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 27, 2015

Member

I think I've got a handle on getting unmount to work correctly; I think I need to perform the unmount first in the kubelet container's mount ns, and then on the host.

Member

pmorie commented Apr 27, 2015

I think I've got a handle on getting unmount to work correctly; I think I need to perform the unmount first in the kubelet container's mount ns, and then on the host.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 29, 2015

Member

Sweet, more rebasing later.

Member

pmorie commented Apr 29, 2015

Sweet, more rebasing later.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 29, 2015

Member

Rebased. Hopefully #6400 will be in soon; I'll rebase on top of that and then I think this should be about ready to go. We need to decide how much of this PR we'll let in right now. I would be fine limiting it to injection of the mounter interface and leave out the image, the kubelet cli changes, and script to launch the kubelet.

Any opinions @dchen1107?

Member

pmorie commented Apr 29, 2015

Rebased. Hopefully #6400 will be in soon; I'll rebase on top of that and then I think this should be about ready to go. We need to decide how much of this PR we'll let in right now. I would be fine limiting it to injection of the mounter interface and leave out the image, the kubelet cli changes, and script to launch the kubelet.

Any opinions @dchen1107?

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie
Member

pmorie commented Apr 29, 2015

@googlebot ping

@smarterclayton smarterclayton removed the cla: no label Apr 29, 2015

@googlebot googlebot added the cla: no label Apr 29, 2015

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 29, 2015

Member

Also, the unmount issue I mentioned earlier is now fixed.

Member

pmorie commented Apr 29, 2015

Also, the unmount issue I mentioned earlier is now fixed.

@eparis

This comment has been minimized.

Show comment
Hide comment
@eparis

eparis Apr 30, 2015

Member

the googlebot is not going to like you unless you switch your git author e-mail address to a RH address. Sorry :-(

Member

eparis commented Apr 30, 2015

the googlebot is not going to like you unless you switch your git author e-mail address to a RH address. Sorry :-(

@vmarmol vmarmol assigned vmarmol and unassigned dchen1107 Apr 30, 2015

@vmarmol

This comment has been minimized.

Show comment
Hide comment
@vmarmol

vmarmol Apr 30, 2015

Contributor

I'll take a look while @dchen1107 is away.

Contributor

vmarmol commented Apr 30, 2015

I'll take a look while @dchen1107 is away.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 30, 2015

Member

@vmarmol It's not ready for final review yet; I will need to rebase and refactor on top of #6400, just FYI

Member

pmorie commented Apr 30, 2015

@vmarmol It's not ready for final review yet; I will need to rebase and refactor on top of #6400, just FYI

@vishk

This comment has been minimized.

Show comment
Hide comment
@vishk

vishk Apr 30, 2015

Please exclude me (vishk) from the conversations. I am getting all your emails in my inbox. You probably want to include vishh who has commented above (notice the last letter in the username). Thanks!

vishk commented Apr 30, 2015

Please exclude me (vishk) from the conversations. I am getting all your emails in my inbox. You probably want to include vishh who has commented above (notice the last letter in the username). Thanks!

@eparis

This comment has been minimized.

Show comment
Hide comment
@eparis

eparis Apr 30, 2015

Member

vishk only you can remove yourself. I'm sure it was an accident that @pmorie included you. On the right side, up near the top, is an "unsubscribe" button. You got auto-subscribed when he said @your_user_name (I won't do it again)

Member

eparis commented Apr 30, 2015

vishk only you can remove yourself. I'm sure it was an accident that @pmorie included you. On the right side, up near the top, is an "unsubscribe" button. You got auto-subscribed when he said @your_user_name (I won't do it again)

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie Apr 30, 2015

Member

Apologies, vishk.

Member

pmorie commented Apr 30, 2015

Apologies, vishk.

@vishk

This comment has been minimized.

Show comment
Hide comment
@vishk

vishk Apr 30, 2015

Ok, thanks!

vishk commented Apr 30, 2015

Ok, thanks!

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie May 1, 2015

Member

Rebased onto #6400, and did some preliminary regression testing of normal operation with e2e on my local system. Next up, testing in container.

Member

pmorie commented May 1, 2015

Rebased onto #6400, and did some preliminary regression testing of normal operation with e2e on my local system. Next up, testing in container.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie May 1, 2015

Member

I ran emptyDir and secrets E2Es locally with SELinux enforcing against containerized kubelet, both passed.

Member

pmorie commented May 1, 2015

I ran emptyDir and secrets E2Es locally with SELinux enforcing against containerized kubelet, both passed.

@pmorie pmorie changed the title from WIP: Run kubelet in container to Experimental support for running kubelet in container May 1, 2015

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie May 1, 2015

Member

@vmarmol My rebase party is complete, I think this is ready for final review.

Member

pmorie commented May 1, 2015

@vmarmol My rebase party is complete, I think this is ready for final review.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie May 1, 2015

Member

I'm blocked at the moment from doing the E2E build because of the bug this commit fixes:

rhatdan/moby1@350a636

They're cutting a new 1.6 package for fedora but it will probably be a couple days before it shows up in repos. :-/

Member

pmorie commented May 1, 2015

I'm blocked at the moment from doing the E2E build because of the bug this commit fixes:

rhatdan/moby1@350a636

They're cutting a new 1.6 package for fedora but it will probably be a couple days before it shows up in repos. :-/

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie May 1, 2015

Member

@vmarmol If you are running docker 1.5 locally and could find it in your heart do kick off an e2e run, I would definitely owe you 🍻

Member

pmorie commented May 1, 2015

@vmarmol If you are running docker 1.5 locally and could find it in your heart do kick off an e2e run, I would definitely owe you 🍻

@vmarmol

This comment has been minimized.

Show comment
Hide comment
@vmarmol

vmarmol May 1, 2015

Contributor

Spoke to @pmorie on IRC and we're gonna split the PR into 4:

  • Injecting mounter
  • nsenter mounter
  • Local Dockerized Kubelet
  • Building Dockerized Kubelet
Contributor

vmarmol commented May 1, 2015

Spoke to @pmorie on IRC and we're gonna split the PR into 4:

  • Injecting mounter
  • nsenter mounter
  • Local Dockerized Kubelet
  • Building Dockerized Kubelet
@vmarmol

This comment has been minimized.

Show comment
Hide comment
@vmarmol

vmarmol May 1, 2015

Contributor

@pmorie the default Docker on GCE is still 1.5 so I'd be happy to test :D I'll kickoff an e2e with this branch

Contributor

vmarmol commented May 1, 2015

@pmorie the default Docker on GCE is still 1.5 so I'd be happy to test :D I'll kickoff an e2e with this branch

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie May 4, 2015

Member

Closing this out since we're splitting out separate PRs.

Member

pmorie commented May 4, 2015

Closing this out since we're splitting out separate PRs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment