Skip to content

Conversation

@gao-feng
Copy link
Contributor

@gao-feng gao-feng commented Sep 9, 2015

Hooks are not only available for creating new ns case, but also
for joining existing ns case.

Signed-off-by: Gao feng [email protected]

Hooks are not only available for creating new ns case, but also
for joining existing ns case.

Signed-off-by: Gao feng <[email protected]>
@philips
Copy link
Contributor

philips commented Sep 9, 2015

hrm, I don't follow what you are trying to say with this. I think you are trying to say that it happens after the namespace is created for the application or has been joined by the application?

@mrunalp
Copy link
Contributor

mrunalp commented Sep 9, 2015

@gao-feng I think if we are to support this in any form then they would have to be per namespace and separate from the existing pre-start hooks.

@gao-feng
Copy link
Contributor Author

@philips No, before join the ns.
the command of hook is host command, Just like creating new ns and then call hooks to do some work.
For joining ns case, it's making sure ns is existing and then call hooks.
We don't care if hooks will enter ns or not. it depends on hook itself.
for example, the hook that prepares network environment for container may create veth pairs and pass on veth device to net ns on host, and then enters the net ns to setup ip/router.rename nic etc...

@mrunalp Don't quite understand why hook should per namespace in join ns case.
The things runtime should do are:
1, check which ns types runtime should create, and then create these new namespaces.
2, check which ns types container should join, and then check if the ns path is exist.
3, save pid to state file
4, call hooks one by one, hooks will decide to enter container or not, and enter which namespaces by themself.

If there are something wrong, please let me know,

@dqminh
Copy link
Contributor

dqminh commented Sep 10, 2015

I think the order is wrong here

1, check which ns types runtime should create, and then create these new namespaces.
2, check which ns types container should join, and then check if the ns path is exist.

Runtime should join namespaces first before create new ones. Because if user decided to join an existing user namespace for example, then namespaces should be created after joining userns.

I think it would be nice to list some use case for hooks when joining existing namespaces so we can figure out how hooks and create/join some namespaces can interact with each other.

@wking
Copy link
Contributor

wking commented Sep 10, 2015

On Thu, Sep 10, 2015 at 10:44:27AM -0700, Daniel, Dao Quang Minh wrote:

I think the order is wrong here

1, check which ns types runtime should create, and then create these new namespaces.
2, check which ns types container should join, and then check if the ns path is exist.

Runtime should join namespaces first before create new ones. Because
if user decided to join an existing user namespace for example, then
namespaces should be created after joining userns.

Why is that? I don't see why you'd need to be in a userns to create a
new mount namespace (for example), but maybe you no longer have
sufficient permissions to create namespaces after joining the userns.
In any case, hooks and setup like this should be run from outside
the namespaces (but after the namespaces have been created) 1.

@gao-feng
Copy link
Contributor Author

Runtime should join namespaces first before create new ones. Because if user decided to join an existing user namespace for example, then namespaces should be created after joining userns.

Ok, join first, then create new ones. But this doesn't affect the topic we discussed.

I think it would be nice to list some use case for hooks when joining existing namespaces so we can figure out how hooks and create/join some namespaces can interact with each other.

Allowing to call hooks for joining existing ns case gives user a chance to tweak the host and namespace environment. such as change mount info of existing mnt ns, create special device node for the container process(hook is the only way to archive this now).

@wking
Copy link
Contributor

wking commented Sep 11, 2015

On Fri, Sep 11, 2015 at 03:25:56AM -0700, Gao feng wrote:

… create special device node for the container process(hook is the
only way to archive this now).

This is actually available via runtime.json's linux.devices 1, which
provides mknod functionality alongside cgroup functionality (see
#101).

@gao-feng
Copy link
Contributor Author

This is actually available via runtime.json's linux.devices [1], which
provides mknod functionality alongside cgroup functionality (see
#101).

As #158 said

Also, when a path is specified, a runtime MUST assume that the setup for that particular namespace has already been done and error out if the config specifies anything else related to that namespace

I think this means creating device node for the existing namespace through the config is incorrect.

@wking
Copy link
Contributor

wking commented Sep 11, 2015

On Fri, Sep 11, 2015 at 07:42:44AM -0700, Gao feng wrote:

#158 said

Also, when a path is specified, a runtime MUST assume that the
setup for that particular namespace has already been done and
error out if the config specifies anything else related to that
namespace

I think this means creating device node for the existing namespace
through the config is incorrect.

What's the namespace for device nodes? Maybe that counts as the mount
namespace? If so, maybe this is a use-case that makes it worth
revisiting the punt in #158 1? Or maybe “creating device nodes in
an existing container without a mknod-calling hook” isn't a convincing
enough use-case ;).

Regardless of how we decide to handle config-side mknod, this
discussion in making me concerned about how clear a line we'll be able
to draw around “anything else related to that namespace”. If we end
up needing to revisit this on a case-by-case basis, it might be easier
to drop that language and explicitly list for each field when it's not
compatible with a given namespace path.

@dqminh
Copy link
Contributor

dqminh commented Sep 11, 2015

Ok, join first, then create new ones. But this doesn't affect the topic we discussed.

Yes, the order doesnt affect the feature if i understand it correctly. But because you said hook should be run before joining the namespaces so that's abit weird.

Think for prestart hook in the case of container that wants to join some existing namespaces, the general behavior is essentially the same as one that creates all namespaces i.e we join existing namespaces, then create new ones, then run prestart hooks, then run the application.

I cant think of any cases where running it before each namespaces was joined is useful.

The hook needs to be aware of the current state of container's namespaces though as @gao-feng said.

@wking
Copy link
Contributor

wking commented Sep 11, 2015

On Fri, Sep 11, 2015 at 09:51:06AM -0700, Daniel, Dao Quang Minh wrote:

I cant think of any cases where running it before each namespaces
was joined is useful.

For example, you can't bind mount a host-side filesystem into the
container if your hook has already joined the container's mount
namespace. The hooks should run after the namespaces / cgroups are
created (and joined by a process which will eventually become the
application, because the state needs a PID), but the hooks themselves
run in the host environment.

The hook needs to be aware of the current state of container's
namespaces though as @gao-feng said.

The state JSON passed to the hooks contain's the application's PID,
and you can use that to lookup any namespaces or cgroups you're
interested in [2].

@gao-feng
Copy link
Contributor Author

@wking @dqminh Can you give me your ack? thanks

@wking
Copy link
Contributor

wking commented Sep 14, 2015

On Mon, Sep 14, 2015 at 06:59:31AM -0700, Gao feng wrote:

@wking @dqminh Can you give me your ack? thanks

bff6d8a looks unambiguously useful to me, saying “it doesn't matter if
we're creating namespaces or not, we still run the pre-start hooks”
(so ACK, for what it's worth). But @philips 1 and @mrunalp 2 both
pushed back against your current phrasing, and seem to be asking for
more general clarity on the lifecycle timeline and hook environment.
Maybe we should table this PR until we get more detail on the general
lifecycle? I have some work along those lines in #126 (see 8dae07a
and 7d6e8b3) that may be useful for seeding that discussion.

@crosbymichael
Copy link
Member

-1

Lets keep the wording as is because the new wording add more confusion than clarification.

@conqerAtapple
Copy link

Looking at container lifecycle/state, would it make sense to add finer hooks with details of the namespace in which the hook executes? Currently its ambiguous.

On Sep 11, 2015, at 9:58 AM, W. Trevor King [email protected] wrote:

On Fri, Sep 11, 2015 at 09:51:06AM -0700, Daniel, Dao Quang Minh wrote:

I cant think of any cases where running it before each namespaces
was joined is useful.

For example, you can't bind mount a host-side filesystem into the
container if your hook has already joined the container's mount
namespace. The hooks should run after the namespaces / cgroups are
created (and joined by a process which will eventually become the
application, because the state needs a PID), but the hooks themselves
run in the host environment.

The hook needs to be aware of the current state of container's
namespaces though as @gao-feng said.

The state JSON passed to the hooks contain's the application's PID,
and you can use that to lookup any namespaces or cgroups you're
interested in [2].


Reply to this email directly or view it on GitHub.

@wking
Copy link
Contributor

wking commented Sep 25, 2015

On Fri, Sep 25, 2015 at 02:25:40PM -0700, Jojy George Varghese wrote:

Looking at container lifecycle/state, would it make sense to add
finer hooks with details of the namespace in which the hook
executes? Currently its ambiguous.

I've floated a lifecycle that does this on the list [1](and it
matches my runC tests [2]).

@gao-feng gao-feng deleted the hook-for-join-ns branch September 28, 2015 02:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants