Skip to content
This repository was archived by the owner on Dec 13, 2018. It is now read-only.

add spec for exec a new process inside a container #290

Merged
rjnagal merged 3 commits into from
Dec 12, 2014
Merged

add spec for exec a new process inside a container #290

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
19099e0
add spec for exec a new process inside a container
dqminh Dec 11, 2014
e9c1b01
fix some grammar errors
dqminh Dec 11, 2014
eb2ae34
move cgroup requirements to planned additions
dqminh Dec 11, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 24 additions & 1 deletion SPEC.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,4 +318,27 @@ a container.
| Resume | Resume all processes inside the container if paused |
| Exec | Execute a new process inside of the container ( requires setns ) |


### Execute a new process inside of a running container.

User can execute a new process inside of a running container. Any binaries to be
executed must be contained within the container's rootfs.
Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

"contained" or "accessible" from within the container?


The started process is jailed inside the current container's rootfs. Any changes
Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

How about "The started process will run inside the container's rootfs?"

made by the process to the container's filesystem will be persisted after the
Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

s/be persisted/persist

process finished executing.

The started process will join the container's existing namespaces. When the
Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

nit: "join all the"

container is paused, the process will also be paused and will resume when
the container is unpaused. The started process will only run when the container's
primary process (PID 1) is running, and will not be restarted when the container
is restarted.
Copy link
Contributor

@rjnagal rjnagal Dec 11, 2014

Choose a reason for hiding this comment

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

Is PID 1 the parent process for the exec processes? Also mention exec cleanup when PID 1 dies.

Copy link
Contributor Author

@dqminh dqminh Dec 12, 2014

Choose a reason for hiding this comment

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

No, PID 1 is the container's main process. The parent process of the exec process is nsenter-exec


The started process will have its own cgroups nested inside the container's
Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

Is this spec a proposal or a documentation of the existing state?

Copy link
Contributor

@vmarmol vmarmol Dec 11, 2014

Choose a reason for hiding this comment

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

Maybe not the place to argue this, but what bothers me about this approach is that the process can move out of the cgroups and them make this approach moot.

Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

How can the process move out of, if cgroups are not exposed inside the container?

Copy link
Contributor

@vmarmol vmarmol Dec 11, 2014

Choose a reason for hiding this comment

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

You can always expose them inside as cAdvisor does today. We'll probably want to be able to expose them in the future as well to be able to use subcontainers. We had merged a PR for some time that put them in by default, I think we're closer to that world than the one where we don't let the users put the cgroup hierarchies in their container.

Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

Would restricting access to cgroups via the filesystem solve the issue?
Privileged containers can still do anything they want.

On Thu, Dec 11, 2014 at 9:28 AM, Victor Marmol [email protected]
wrote:

In SPEC.md
#290 (diff)
:

+### Execute a new process inside of a running container.
+
+User can execute a new process inside of a running container. Any binaries to be
+executed must be contained within the container's rootfs.
+
+The started process is jailed inside the current container's rootfs. Any changes
+made by the process to the container's filesystem will be persisted after the
+process finished executing.
+
+The started process will join the container's existing namespaces. When the
+container is paused, the process will also be paused and will resume when
+the container is unpaused. The started process will only run when the container's
+primary process (PID 1) is running, and will not be restarted when the container
+is restarted.
+
+The started process will have its own cgroups nested inside the container's

You can always expose them inside as cAdvisor does today. We'll probably
want to be able to expose them in the future as well to be able to use
subcontainers. We had merged a PR for some time that put them in by
default, I think we're closer to that world than the one where we don't let
the users put the cgroup hierarchies in their container.

—
Reply to this email directly or view it on GitHub
https://github.com/docker/libcontainer/pull/290/files#r21692525.

Copy link
Contributor

@vmarmol vmarmol Dec 11, 2014

Choose a reason for hiding this comment

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

I personally don't think this is the right approach. We want to be able to have regular unpriviledged users use subcontainers.

We can always say that this is on a "best-effort" basis. There are already ways for the user to shoot themselves in the foot.

Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

Chatted with @vmarmol offline. I think he is convinced that this is safe for all users who do not have their cgroups mounted as 'rw'. I referred to such users as 'privileged' which Victor does not like. This approach will work for all the regular users who don't mount their cgroups as 'rw'.

Copy link
Contributor

@vmarmol vmarmol Dec 11, 2014

Choose a reason for hiding this comment

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

This should be safe for users who also mount their cgroups 'rw' my only concern is not blocking those that want to mount 'rw'. I do think that this mechanism breaking is an edge case we don't need to handle. The users that break it should handle it. They should, however, be allowed to mount the cgroups 'rw' without it being priviledged.

Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

Agreed, if by 'privileged' one means granting all capabilities.

Copy link
Contributor

@vmarmol vmarmol Dec 11, 2014

Choose a reason for hiding this comment

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

Yes :)

cgroups. This is used for process tracking and optionally resource allocation
handling for the new process. Freezer cgroup is required, the rest of the cgroups
are optional. The process executor must place its pid inside the correct
cgroups before starting the process. This is done so that no child processes or
threads can escape the cgroups.

When the process stopped, all child processes spawned by the process will be
Copy link
Contributor

@vishh vishh Dec 11, 2014

Choose a reason for hiding this comment

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

How about - "When the process is stopped, all its children will also be stopped and the sub-cgroups tracking them will also be removed."

stopped and the process's cgroups will be removed.