Use namespace.Exec() and namespace.Init() to execute processes in CT #306
Conversation
|
Merged the other PR, feel free to rebase. I'll start the review in the meantime. I think breaking backwards compat is probably fine. Clients (outside of Docker) don't really use the "API" in the same way. Docker has theirs versioned anyways. @crosbymichael can chime in if that is completely off-base :) |
Makefile
Outdated
There was a problem hiding this comment.
I am not a maintainer here, but I really don't like glog. In docker we're using logrus.
There was a problem hiding this comment.
What don't you like about glog?
I'm biased :)
There was a problem hiding this comment.
When I tried to use it - it made my binary not static :) Also configuration is much more opaque, then in logrus. This is just my opinion, but I know that @crosbymichael like logrus too.
There was a problem hiding this comment.
Interesting, we never had the static issue in cAdvisor. I think it was because we were already asking for cgo code to be statically linked. Maybe I'm just used to glog's configuration since it is identical to what we use in Google.
|
Some comments, but nothing major. Looks good overall. Thanks for all the changes @avagin! |
|
@vmarmol yes, docker has libcontainer vendored so we can itterate and change things right now. After we get the api branch up and stable I can update docker so that we can test. Feel free to break backards compat just as long as we do it right and won't have to make more breaking changes in the future ;) |
|
@crosbymichael awesome! Full steam ahead then. |
factory.go
Outdated
There was a problem hiding this comment.
Should we just make this private?
There was a problem hiding this comment.
We can't use fork() from go code. When we want to start a new container, we need to execute a program in a new set of namespaces. This program must execute namespace.Init() and then execute a specified program. So the answer on your question depends on who should create this program. I don't see any problem to provide this program together with libcontainer, but I'm afraid it isn't the golang way, is it? If we decide that a user should create this program, we need to provide StartInitialization() for him.
We are going to import the namespaces package into libcontainer, so libcontainer should not be imported into namespaces. Signed-off-by: Andrey Vagin <[email protected]>
We are going to import the namespaces package into libcontainer, so libcontainer should not be imported into namespaces. Signed-off-by: Andrey Vagin <[email protected]>
Signed-off-by: Andrey Vagin <[email protected]>
Use namespace.Exec() and namespace.Init() to execute processes in CT. Now an init process is actually executed in a new container. This series doesn't change code about creating containers, it only reworks code according with new API. Signed-off-by: Andrey Vagin <[email protected]>
Signed-off-by: Andrey Vagin <[email protected]>
Could someone explain why we should close this fds? Usually users cares about closing them or not. For example exec.Cmd declares them as io.Reader. Signed-off-by: Andrey Vagin <[email protected]>
Signed-off-by: Andrey Vagin <[email protected]>
Signed-off-by: Andrey Vagin <[email protected]>
|
LGTM, merging. |
Use namespace.Exec() and namespace.Init() to execute processes in CT
5 participants
This series is the second step to support the new API. Now an init process is actually executed in a new container. This series doesn't change code about creating containers, it only reworks code according with new API.
We are not going to save backward compatibility with old API, aren't we?