runtime: Replace "process is stopped" with "process exits" #465

wking · 2016-05-27T05:57:03Z

proc(5) describes the following state entries in
proc/[pid]/stat (for modern kernels):

R Running
S Sleeping in an interruptible wait
D Waiting in uninterruptible disk sleep
Z Zombie
T Stopped (on a signal)
t Tracing stop
X Dead

and ps(1) has a bit more context (for modern kernels):

D uninterruptible sleep (usually IO)
R running or runnable (on run queue)
S interruptible sleep (waiting for an event to complete)
T stopped by job control signal
t stopped by debugger during the tracing
X dead (should never be seen)
Z defunct ("zombie") process, terminated but not reaped by its
parent

So I expect "stopped" to mean "process still exists but is paused,
e.g. by SIGSTOP". And I expect "exited" to mean "process has finished
and is either a zombie or dead".

After this commit, git grep -i stop only turns up poststop-hook
stuff, a reference in principles.md, a "stoppage" in LICENSE, and some
ChangeLog entries.

This also replaces "container's process" with "container process" to
match usage in the rest of the repository. After this commit:

$ git grep -i "container process" | wc -l
13
$ git grep -i "container's process" | wc -l
1

hqhq · 2016-05-28T01:37:31Z

LGTM

mrunalp · 2016-05-31T16:41:37Z

Looks fine. Needs rebase.

wking · 2016-05-31T17:29:46Z

On Tue, May 31, 2016 at 09:41:40AM -0700, Mrunal Patel wrote:

Looks fine. Needs rebase.

Rebased with 23009bb → a95eb3b, although again 1 I don't see any changes
to the patch.

wking · 2016-05-31T17:50:11Z

Maybe needs a reroll with my suggested updates to #462.

wking · 2016-06-03T20:44:03Z

Rebased and adjusted along the lines of my #464 comments with a95eb3b → 5816f31.

dqminh · 2016-07-13T16:43:57Z

runtime.md

-  * `running` : the container has been created and the user-specified code is running
-  * `stopped` : the container has been created and the user-specified code has been executed but is no longer running
+  * `created` : the container process has neither exited nor executed the user-specified code
+  * `running` : the container process has executed the user-specified code but has not exited


meaning of user-specified code vs container process here seems confusing to me. For example, if container process is some init piece of the runtime, does this mean that the init piece has to be run for the lifetime of the user process because stopped only happens when the container process has exited ? Or did I understand this incorrectly ?

What do you think about replacing user-specified code with container process completely. Something like

* `created` : the container has been created but the container process has not yet been executed. * `running`: the container process has been executed but has not exited. * `stopped`: the container process has exited.

On Wed, Jul 13, 2016 at 09:43:58AM -0700, Daniel, Dao Quang Minh wrote:

@@ -15,9 +15,9 @@ This MUST be unique across all containers on this host.
There is no requirement that it be unique across hosts.

status: (string) is the runtime state of the container.
The value MAY be one of:

* created : the container has been created but the user-specified code has not yet been executed

* running : the container has been created and the user-specified code is running

* stopped : the container has been created and the user-specified code has been executed but is no longer running

* created : the container process has neither exited nor executed the user-specified code

* running : the container process has executed the user-specified code but has not exited

meaning of user-specified code vs container process here seems
confusing to me.

This terminology issue is being hashed out in #466. Maybe we should
handle that first?

@wking Since #466 is closed, I think most of the maintainers like container process more than user-specified code, but I think we can handle that in a separate PR.

wking · 2016-11-05T03:53:43Z

Rebased onto master with 5816f31 → 7797b33.

hqhq · 2016-11-10T12:19:59Z

runtime.md

-    * `stopped`: the container has been created and the user-specified code has been executed but is no longer running
+    * `created`: the container process has neither exited nor executed the user-specified code
+    * `running`: the container process has executed the user-specified code but has not exited
+    * `stopped`: the container process has exited


Why remove the container has been created phrase? I don't think that redundant, especially for the created status.

Maybe the container process crashed during creation, in which case we're stopped before the container has been fully created. And see #507, which builds on this PR and adds creating. In the absence of a creating status (this PR before #507), I think we're created even before creation has finished.

It would be better to state that in the stopped state the container has ran the container's main process and it has exited leaving the container in the stopped state

If create fails the container should be destroyed and will be by the runtimes.

It would be better to state that in the stopped state the container has ran the container's main process…

This is not necessarily true, since the container process may have died during creation or between creation and the start call.

If create fails the container should be destroyed and will be by the runtimes.

Agreed, but we need to define the status passed to any post-stop hooks, which run after the container process exits. That happens even if the container process dies during creation.

crosbymichael · 2016-11-10T17:29:53Z

runtime.md

-    * `created`: the container has been created but the user-specified code has not yet been executed
-    * `running`: the container has been created and the user-specified code is running
-    * `stopped`: the container has been created and the user-specified code has been executed but is no longer running
+    * `created`: the container process has neither exited nor executed the user-specified code


Its not user-specified code, we went over this many times.

This is not a good description of the created state. The container is in a created state when it has been fully initialized but is not yet running the containers main process

Its not user-specified code, we went over this many times.

As I pointed out after #466 was closed, all of the changes proposed by #466 are already live. So I'd rather keep the currently-live “user-specified code” language. If you feel like that language is unclear, adjusting to something else seems orthogonal to the stopped → exists fix that this PR is focused on. Maybe open a different PR?

The container is in a created state when it has been fully initialized but is not yet running the containers main process.

#507 addresses this by adding a creating state, and that seems orthogonal to the stopped → exists fix which I'm focusing on here. Until then, created seems like the best stand in (a container being created is certainly not running or stopped).

crosbymichael · 2016-11-10T17:31:10Z

runtime.md

-    * `running`: the container has been created and the user-specified code is running
-    * `stopped`: the container has been created and the user-specified code has been executed but is no longer running
+    * `created`: the container process has neither exited nor executed the user-specified code
+    * `running`: the container process has executed the user-specified code but has not exited


container process not user-specified code

crosbymichael · 2016-11-10T17:32:50Z

runtime.md

-    * `stopped`: the container has been created and the user-specified code has been executed but is no longer running
+    * `created`: the container process has neither exited nor executed the user-specified code
+    * `running`: the container process has executed the user-specified code but has not exited
+    * `stopped`: the container process has exited


It would be better to state that in the stopped state the container has ran the container's main process and it has exited leaving the container in the stopped state

crosbymichael · 2016-11-10T17:33:18Z

runtime.md

-    * `stopped`: the container has been created and the user-specified code has been executed but is no longer running
+    * `created`: the container process has neither exited nor executed the user-specified code
+    * `running`: the container process has executed the user-specified code but has not exited
+    * `stopped`: the container process has exited


If create fails the container should be destroyed and will be by the runtimes.

crosbymichael · 2016-11-10T17:33:56Z

runtime.md

-5. The container's process is stopped.
-   This MAY happen due to them erroring out, exiting, crashing or the runtime's [`kill`](runtime.md#kill) operation being invoked.
+5. The container process exits.
+   This MAY happen due to erroring out, exiting, crashing or the runtime's [`kill`](runtime.md#kill) operation being invoked.


This is not correct as the runtime's kill command can sent non terminating signals.

This is not correct as the runtime's kill command can sent non terminating signals.

I agree, but this MAY does not list all possible triggers nor does it require listed triggers to cause container exit. And it's the existing language and is orthogonal to the stopped → exited I'm focusing on here. Maybe file a new PR with your suggested wording?

proc(5) describes the following state entries in proc/[pid]/stat [1] (for modern kernels): * R Running * S Sleeping in an interruptible wait * D Waiting in uninterruptible disk sleep * Z Zombie * T Stopped (on a signal) * t Tracing stop * X Dead and ps(1) has a bit more context [2] (for modern kernels): * D uninterruptible sleep (usually IO) * R running or runnable (on run queue) * S interruptible sleep (waiting for an event to complete) * T stopped by job control signal * t stopped by debugger during the tracing * X dead (should never be seen) * Z defunct ("zombie") process, terminated but not reaped by its parent So I expect "stopped" to mean "process still exists but is paused, e.g. by SIGSTOP". And I expect "exited" to mean "process has finished and is either a zombie or dead". After this commit, 'git grep -i stop' only turns up the "stopped" state (which I've left alone for backwards compat), some poststop-hook stuff, a reference in principles.md, a "stoppage" in LICENSE, and some ChangeLog entries. Also replace "container's process" with "container process" to match usage in the rest of the repository. After this commit: $ git grep -i "container process" | wc -l 20 $ git grep -i "container's process" | wc -l 1 Also reword status entries to avoid "running", which is less precise in our spec (e.g. it also includes "sleeping", "waiting", ...). Also removes a "them" leftover from a partial plural -> singular reroll of be59415 (Split create and start, 2016-04-01, opencontainers#384). [1]: http://man7.org/linux/man-pages/man5/proc.5.html [2]: http://man7.org/linux/man-pages/man1/ps.1.html Signed-off-by: W. Trevor King <wking@tremily.us>

wking · 2017-01-06T07:01:52Z

Rebased onto master with 7797b33 → 9eb32c0. Some of @crosbymichael's concerns have been addressed in the meantime (e.g. by #629's “code” → “program”), but some are still unresolved: * Whether ‘created’ applies before the container is fully created [1]. My preferred approach to this is in #507, and until then I think we should use ‘created’ as our best approximation [2]. * Whether ‘stopped’ should depend on the execution (or not) of the user-specified program [3]. I think ‘stopped’ should also include containers which have died during creation, etc. [4]. * Whether the list of possible terminators is exhaustive [5]. I think it is not, and also think that wording is orthogonal to this PR [6]. I'm happy to dig into any of these outstanding points if my positions are unclear or poorly motivated. [1]: #465 (comment) [2]: #465 (comment) [3]: #465 (comment) [4]: #465 (comment) [5]: #465 (comment) [6]: #465 (comment)

mrunalp · 2017-01-18T22:23:37Z

@crosbymichael PTAL

crosbymichael

LGTM

crosbymichael · 2017-01-18T23:50:45Z

LGTM

wking · 2017-01-22T04:31:40Z

@hqhq, @mrunalp, now that @crosbymichael has LGTMed this, do either of you want to re-approve it?

hqhq · 2017-01-22T06:28:59Z

LGTM

wking force-pushed the stop-to-exit branch from 313f6c9 to 23009bb Compare May 27, 2016 05:59

wking force-pushed the stop-to-exit branch from 23009bb to a95eb3b Compare May 31, 2016 17:28

wking mentioned this pull request May 31, 2016

Add a 'status' field to our state struct #462

Merged

wking force-pushed the stop-to-exit branch from a95eb3b to 5816f31 Compare June 3, 2016 20:42

wking mentioned this pull request Jun 23, 2016

runtime: Add 'creating' to state status #507

Merged

dqminh reviewed Jul 13, 2016
View reviewed changes

wking force-pushed the stop-to-exit branch from 5816f31 to 7797b33 Compare November 5, 2016 03:53

hqhq reviewed Nov 10, 2016

View reviewed changes

wking closed this Nov 10, 2016

wking reopened this Nov 10, 2016

crosbymichael requested changes Nov 10, 2016

View reviewed changes

wking force-pushed the stop-to-exit branch from 7797b33 to 9eb32c0 Compare January 6, 2017 06:47

crosbymichael approved these changes Jan 18, 2017

View reviewed changes

hqhq merged commit 579548a into opencontainers:master Jan 22, 2017

vbatts mentioned this pull request Jan 27, 2017

Bump version for 1.0.0-rc4 #672

Merged

wking deleted the stop-to-exit branch January 27, 2017 21:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime: Replace "process is stopped" with "process exits" #465

runtime: Replace "process is stopped" with "process exits" #465

wking commented May 27, 2016

hqhq commented May 28, 2016 •

edited by caniszczyk

Loading

mrunalp commented May 31, 2016

wking commented May 31, 2016

wking commented May 31, 2016

wking commented Jun 3, 2016 via email

dqminh Jul 13, 2016

wking Jul 13, 2016

hqhq Nov 10, 2016

wking commented Nov 5, 2016

hqhq Nov 10, 2016

wking Nov 10, 2016

crosbymichael Nov 10, 2016

crosbymichael Nov 10, 2016

wking Nov 10, 2016

crosbymichael Nov 10, 2016

crosbymichael Nov 10, 2016

wking Nov 10, 2016

wking Nov 10, 2016

crosbymichael Nov 10, 2016

crosbymichael Nov 10, 2016

crosbymichael Nov 10, 2016

crosbymichael Nov 10, 2016

wking Nov 10, 2016

wking commented Jan 6, 2017 via email

mrunalp commented Jan 18, 2017

crosbymichael left a comment

crosbymichael commented Jan 18, 2017 •

edited by caniszczyk

Loading

wking commented Jan 22, 2017

hqhq commented Jan 22, 2017 •

edited by caniszczyk

Loading

runtime: Replace "process is stopped" with "process exits" #465

runtime: Replace "process is stopped" with "process exits" #465

Conversation

wking commented May 27, 2016

hqhq commented May 28, 2016 • edited by caniszczyk Loading

mrunalp commented May 31, 2016

wking commented May 31, 2016

wking commented May 31, 2016

wking commented Jun 3, 2016 via email

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking commented Nov 5, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wking commented Jan 6, 2017 via email

mrunalp commented Jan 18, 2017

crosbymichael left a comment

Choose a reason for hiding this comment

crosbymichael commented Jan 18, 2017 • edited by caniszczyk Loading

wking commented Jan 22, 2017

hqhq commented Jan 22, 2017 • edited by caniszczyk Loading

hqhq commented May 28, 2016 •

edited by caniszczyk

Loading

crosbymichael commented Jan 18, 2017 •

edited by caniszczyk

Loading

hqhq commented Jan 22, 2017 •

edited by caniszczyk

Loading