-
Notifications
You must be signed in to change notification settings - Fork 729
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
Update to work with libcontainer #41
Conversation
Actually can you just use the PID from docker inspect? I fixed the lxc issue where it was not reporting back the correct nspid so it should work now for both drivers. |
Awesome, much much cleaner |
Done |
Yes but — this won't work anymore for plain LXC containers (and I know a few people are using pipework for that). Do you think you could also keep the code to handle non-Docker containers...? Thanks a lot anyway! |
Yes but — this won't work anymore for plain LXC containers (and I know a few people are using pipework for that). Do you think you could also keep the code to handle non-Docker containers...? Thanks a lot anyway! |
Did you test it with LXC? It should still work with it. I changed the part the deals with Docker and put a check to see if the DOCKERPID was set later on and use that, else we do all the lookups for lxc stuff. |
Oops, you're right, and I'm wrong. OK, I'll try to do a quick test when I can, and merge that if everything goes fine. Thanks, and sorry! |
Shamelessly copy jpetazzo#41
Hrmm, sorry, I again merged things in the wrong order it seems :-) Could you please rebase this? Then I'll do this quick test and merge as well... Thank you! |
Makes pipework work with the new docker native exec driver (libcontainer).
Since docker returns a pid for the container use that instead of doing all the other hackery.
Done |
Tested and works for me, but got a report that it didn't work correctly with an older version of Docker :-/ I'll need a few more days to investigate. (I wonder if it might be because PID is not available in older versions... But again, I have to investigate.) Thanks nonetheless! |
Yes, @crosbymichael mentioned that PID did not work correctly in older Docker versions. |
o Shamelessly copy jpetazzo#41, and modify it to more simply fall-back to docker if cgroups lookup doesn't work. o Ensure we properly test for failures when seeking data from cgroups or docker, to avoid silent failure due to the "set -e" o Consolidate all indentation to use spaces instead of inconsistent tabs
any progress on this? seems to be stalled. using latest version of pipework along with docker 0.9.1 |
@inthecloud247 What do you mean it can't show containers? Pipework doesn't ever show containers. |
BTW, this will work 100% with at least Docker 0.9+ |
Sorry for not being clear. I tested out your branch of pipework and it seems to work fine with docker 0.9.1 running on ubuntu 13.10 When it wasn't working on the current master, the error message was showing "cannot find container xxxxxx". That's what I was referring to. Sent from my iPhone
|
Update to work with libcontainer
Thanks! |
Makes pipework work with the new docker native exec driver (libcontainer).