Run post-stop hooks before the container sandbox is deleted.

[vishh]

One of the main use cases of post-stop is to scrape filesystem contents with a container's mount namespace, before deleting the container.
Running post-stop after the container is deleted is equivalent to deleting the container and running the post-stop hook outside of the spec context.

cc @mrunalp @crosbymichael @philips

[wking]

On Fri, Apr 22, 2016 at 01:43:57PM -0700, Vish Kannan wrote:

One of the main use cases of post-stop is to scrape filesystem
contents with a container's mount namespace, before deleting the
container. Running post-stop after the container is deleted is
equivalent to deleting the container and running the post-stop hook
outside of the spec context.

The post-stop hooks landed in #34, and a comment there explicitly
suggests bind-mounting in a pre-start hook if you want to preserve a
particular namespace 1. Requring the runtime to do something like
that to pin all namespaces for all containers seems like a lot of work
to save a bit of typing for folks who need the pinned workflow.

I agree that this makes the current post-stop hooks pretty useless,
but my preferred solution to that is dropping them from the spec ;).
If sub-container workflows like 2 and #391 are too awkward, I'd
rather make them easier (for folks who want them) than saddle all
users with pinned namespaces.

[vishh]

The Spec should enable simple programming patterns. If users/clients are
expected to understand the internals of each runtime, what is the use of
the Spec then?

On Fri, Apr 22, 2016 at 1:54 PM, W. Trevor King notifications@github.com
wrote:

On Fri, Apr 22, 2016 at 01:43:57PM -0700, Vish Kannan wrote:

One of the main use cases of post-stop is to scrape filesystem
contents with a container's mount namespace, before deleting the
container. Running post-stop after the container is deleted is
equivalent to deleting the container and running the post-stop hook
outside of the spec context.

The post-stop hooks landed in #34, and a comment there explicitly
suggests bind-mounting in a pre-start hook if you want to preserve a
particular namespace 1. Requring the runtime to do something like
that to pin all namespaces for all containers seems like a lot of work
to save a bit of typing for folks who need the pinned workflow.

I agree that this makes the current post-stop hooks pretty useless,
but my preferred solution to that is dropping them from the spec ;).
If sub-container workflows like 2 and #391 are too awkward, I'd
rather make them easier (for folks who want them) than saddle all
users with pinned namespaces.

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#395 (comment)

[wking]

On Fri, Apr 22, 2016 at 02:47:32PM -0700, Vish Kannan wrote:
“The Spec should enable simple programming patterns. If users/clients are
expected to understand the internals of each runtime, what is the use of
the Spec then?”

The sub-container suggestion is:

If you want to pin part of your sandbox, set it up with a parent
container. Then launch a subcontainer that joins the parent,
optionally adds additional sandboxing, and does the real work.
After the subcontainer dies, take what you want from the parent
container and kill the parent.

That requires zero knowledge of runtime internals, but does call for a
convenient way to launch sub-containers. #391 is my best guess at
joining an existing container, and it's already got some rough
edges. I think we have some work to do before join-and-extend
(e.g. “join container 123 and create a child PID namespace”) is
convenient. But both #391 and join-and-extend may be workflows that
are made easy via higher-level tooling based on a more basic runtime
spec. We dropped ‘exec’ in #388 because it was too hard to agree on
what it meant. I'd like to see #391 or similar get polished to the
point that implementing ‘exec’ via the more generic runtime-spec
tooling is convenient. I expect join-and-extend falls into the same
category, and we want to leave the spec alone but publish some
implementation suggestions that don't look too hideous.

[julz]

I think the thing to notice is with sub containers you can get either the behaviour where pid 1 exiting automatically cleans things up or the behaviour where namespaces exist longer than a container - it can express both. Unless I'm missing something if you require a post stop hook run before the namespaces are cleaned up you can't support the pid1-exit-cleans-everything flow (because you'll need to bind mount them). It seems like a shame to give up this flexibility (not to mention the currently most common and understood semantics of what pid 1 exit means) when the alternative approach supports both scenarios..

[vishh]

I don't see why invoking a delete after the init process exits is an
issue in reality. Why do we have to avoid that step, other than the fact
that it is technically possible?

I don't like the fact that clients have to deal with bind mounts of
namespaces. To me, the Spec has failed to achieve its goals if it requires
clients to reverse engineer underlying runtime implementations.

An explicit delete step when supported across OSes will let users run
post-stop hooks without OS (or runtime) specific logic.

On Mon, Apr 25, 2016 at 11:51 AM, Julian Friedman notifications@github.com
wrote:

I think the thing to notice is with sub containers you can get either
the behaviour where pid 1 exiting automatically cleans things up or the
behaviour where namespaces exist longer than a container - it can express
both. Unless I'm missing something if you require a post stop hook run
before the namespaces are cleaned up you can't support the
pid1-exit-cleans-everything flow (because you'll need to bind mount them).
It seems like a shame to give up this flexibility (not to mention the
currently most common and understood semantics of what pid 1 exit means)
when the alternative approach supports both scenarios..

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#395 (comment)

[crosbymichael]

The change as is LGTM

[julz]

Doesn't a user already have to understand pretty much all of this to do exec (now we've dropped that from the spec in favour of explicitly entering namespaces via paths)? If we're OK with exec not being in the spec it seems like we're already ok with users needing to know about namespace paths..

[vishh]

An exec re-uses fields in the Spec that the users have to understand
anyways.
Whereas, bind mounting namespaces requires understanding of the linux
kernel which IMHO the Spec should try to avoid.

On Mon, Apr 25, 2016 at 12:08 PM, Julian Friedman notifications@github.com
wrote:

Doesn't a user already have to understand pretty much all of this to do
exec (now we've dropped that from the spec in favour of explicitly entering
namespaces via paths)? If we're OK with exec not being in the spec it seems
like we're already ok with users needing to know about namespace paths..

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#395 (comment)

[wking]

On Mon, Apr 25, 2016 at 01:08:40PM -0700, Vish Kannan wrote:
“An exec re-uses fields in the Spec that the users have to
understand anyways. Whereas, bind mounting namespaces requires
understanding of the linux kernel which IMHO the Spec should try to
avoid.”

Sub-containers does not require out-of-spec knowledge 1.

[julz]

Like @wking says, if the test is whether we only use fields that're already in the spec sub-containers work for all the above use cases (not just some) without needing to understand anything other than namespace and cgroup paths, which we're already ok with users needing to understand for exec. (Indeed really exec is just a variant of this flow that enters all namespaces..)

[vishh]

May be it will help if someone can clarify how one can run post-stop hooks
using just the Spec today, without modifying the root bundle, in a
reasonably portable manner? Some pseudo code instead of just words will
help facilitate this conversation.

On Mon, Apr 25, 2016 at 1:50 PM, Julian Friedman notifications@github.com
wrote:

Like @wking https://github.com/wking says, if the test is whether we
only use fields that're already in the spec sub-containers work for all the
above use cases (not just some) without needing to understand anything
other than namespace and cgroup paths, which we're already ok with users
needing to understand for exec. (Indeed really exec is just a variant of
this flow that enters all namespaces..)

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#395 (comment)

[wking]

On Mon, Apr 25, 2016 at 02:01:42PM -0700, Vish Kannan wrote:
“May be it will help if someone can clarify how one can run post-stop
hooks using just the Spec today…”

I'm guessing you mean “run shell commands in the container namespaces
after the container exits” 1. #391 (linked above 2) already
explains how to do things like:

1. Parent creates a new mount namespace:

   {
   …
   "linux": {
   "namespaces": [
   {"type": "mount"}
   ]
   }
   }

2. Child joins that mount namespace:

   {
   …
   "linux": {
   "namespaces": [
   {"type": "mount", "path": "/proc/1234/ns/mnt"},
   …
   ]
   }
   }

3. Child does work and dies.

4. Extract data from the parent mount namespace with ‘nsenter -m
   /proc/1234/ns/mnt …’ or another child along the lines of step 2.

5. Kill the parent container.

In that workflow, step 4 is where the post-stop hooks (as you envision
them in this PR) would go.

However, instead of just joining and/or creating namespaces, some
users will probably want to join-and-create (e.g. so both the parent
container and child containers can be PID 1 in their own PID
namespace, with the child's PID namespace descending from the parent's
PID namespace). I don't think join-and-extend is well-supported by
the current spec 3. I have fuzzy memories of previous requests for
that sort of functionality getting responses like “just join the
namespaces and call the runtime from there”, translating to something
like ‘nsenter … runc start …’ [4]. However, it makes life more
difficult for the caller if the namespace paths in their config have
to resolve in the nsenter'ed mount namespace, and the nsenter might
break things like state registration [5,6](e.g. after ‘nsenter … runc
start my-container’, ‘nsenter … runc state my-container’ would
probably work, but a bare ‘runc state my-container’ might not).
Couple this with the joining-order issues 7, and I end up wanting
namespace handling closer to this (tweaking 8):

Namespaces are specified as an array of entries inside the
namespaces root field. The following parameters can be specified to
setup namespaces:

• type …

• path …

  If ‘path’ is specified, that particular file (resolved in the
  runtime namespace 9) is used to join that type of namespace.
  Namespaces are created or joined (depending on ‘path’) in the listed
  order.

Although the implementation would be non-trivial and probably involve
a mix of setns and clone, see the caveats for PID, user, and mount
namespaces in setns(2) 10.

With that approach, you could us a parent config like:

{
…
"linux": {
"namespaces": [
{"type": "pid"},
{"type": "mount"}
}
}
}

to create your parent container, and:

{
…
"linux": {
"namespaces": [
{"type": "pid", "path": "/proc/1234/ns/pid"},
{"type": "pid"},
{"type": "mount", "path": "/proc/1234/ns/mnt"},
}
}
}

to mean “Join the parent's PID namespace, create a new child PID
namespace, and join the parent's mount namespace”, where the parent is
process 1234 in the runtime PID namespace, and /proc in the runtime
mount namespace belongs to the runtime PID namespace.

[4]: I haven't been able to dig up a reference to an earlier exchange like this though, so maybe I'm just making it up.

 Although wording to about that never landed in the spec.

 Subject: [Linux] Specify namespace-joining order?  (can be important for user namespaces)
 Date: Mon, 18 Apr 2016 21:53:00 -0700
 Message-ID: <20160419045300.GT23066@odin.tremily.us>

[wking]

I think we can close this in light of last week's meeting 1. The
consensus for supporting this workflow seems to be autoremove (landed
in #384 by @duglin around here 2) or similar. I've argued for “or
similar” 3, but regardless of how that shakes out, the use case
described in this PR will have to be addressed by #384 before it
lands.

[tianon]

What's the status on this one? It looks like it's got one approval (@crosbymichael), but there was some discussion following. Is this change still something we should be considering?

[wking]

On Fri, Nov 04, 2016 at 03:04:59PM -0700, Tianon Gravi wrote:

What's the status on this one?

The autoremove stuff I referenced earlier 1 never landed, so folks
who want to preserve their mount namespaces past container-process
death are currently back to pinning the mount namespace on their own
before calling ‘start’. I'd floated some exec notes in #391, which
would have allowed folks to use wrapper containers to do the pinning
with subcontainers to do the work. #391 had some positive vibes in
June 2, but runC still has neither required-mount support nor
unprivileged-user support as far as I'm aware ;).

[mrunalp]

@vishh Do we need this?

[vishh]

@mrunalp This will definitely be helpful/useful if technically feasible.

[vbatts]

We're currently okay with the language in the spec. Closing this issue.
Open a new issue for particular updates needed.