Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

runtime: Replace "user-specified process" with "user-specified code" in 'create' #466

Closed
wants to merge 1 commit into from

Conversation

wking
Copy link
Contributor

@wking wking commented May 27, 2016

Because we do want to create the process at create-time, we just
want to idle it running runtime-specified code until start-time. The
language I'm changing to already exists in the lifecycle and start
sections.

Since there were a number of links to the process spec, I replaced the
inline targets with a reference-style target. And while I was doing
that, I added a missing close-paren to lifecycle step 2.

…in 'create'

Because we *do* want to create the process at create-time, we just
want to idle it running runtime-specified code until start-time.  The
language I'm changing to already exists in the lifecycle and start
sections.

Since there were a number of links to the process spec, I replaced the
inline targets with a reference-style target.  And while I was doing
that, I added a missing close-paren to lifecycle step 2.

Signed-off-by: W. Trevor King <wking@tremily.us>
@@ -72,7 +72,7 @@ This operation MUST return the state of a container as specified in the [State](
This operation MUST generate an error if it is not provided a path to the bundle and the container ID to associate with the container.
If the ID provided is not unique across all containers within the scope of the runtime, or is not valid in any other way, the implementation MUST generate an error and a new container MUST not be created.
Using the data in [`config.json`](config.md), this operation MUST create a new container.
This means that all of the resources associated with the container MUST be created, however, the user-specified process MUST NOT be run at this time.
This means that all of the resources associated with the container MUST be created, however, the user-specified code (from [`process`][process]) MUST NOT be run at this time.
Copy link
Contributor

Choose a reason for hiding this comment

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

user-specified code seems a bit abstract to me, and I don't see the problem with the original one.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok I just found we are using use-specified code in Start operation, I think they need to be consistent, my preference would be use-specified process but I don't have strong opinion on this, I'll leave it to others.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

On Fri, May 27, 2016 at 06:50:14PM -0700, Qiang Huang wrote:

-This means that all of the resources associated with the container MUST be created, however, the user-specified process MUST NOT be run at this time.
+This means that all of the resources associated with the container MUST be created, however, the user-specified code (from [process][process]) MUST NOT be run at this time.

user-specified code seems a bit abstract to me, and I don't see
the problem with the original one.

The distinction between “process” and “code” is that fork(2) and
similar create a new process running the caller's code, while
execve(2) and similar execute new code in the caller's existing
process. In this spec, the container process is created at
create-time (with fork(2) or similar) and then that process is
switched to user-specified code at start-time (with execve(2) or
similar). I'm dropping the “process MUST NOT be run” phrasing here,
because we do want to create the container process here. We just
don't want it running the user's code yet.

@crosbymichael
Copy link
Member

I think its a mistake to call it users code. we are not running any "code". we are not a jit'd runtime that takes code and runs it. it is all processes and the key in the spec is called "process" so calling it code is just adding more confusion.

@wking
Copy link
Contributor Author

wking commented Jun 1, 2016

On Wed, Jun 01, 2016 at 02:37:55PM -0700, Michael Crosby wrote:

I think its a mistake to call it users code.

execve(2) says “program” and unpacks that to “a binary executable, or
a script starting with a [shebang]” 1. proc(5) documents
/proc/[pid]/exe by talking about “the executed command” 2. Create
is about creating a new container process, and start is just
about changing what that process is doing.

@crosbymichael
Copy link
Member

We are here to make meaningful abstractions for defining a container, not specify how the kernel executes code

@wking
Copy link
Contributor Author

wking commented Jun 1, 2016

On Wed, Jun 01, 2016 at 03:10:06PM -0700, Michael Crosby wrote:

We are here to make meaningful abstractions for defining a
container, not specify how the kernel executes code

To me, “user-specified process MUST NOT be run” sounds like “‘create’
should not create the container process”. But it MUST create that
process. So I'm suggesting we adopt wording that makes it clear that
the container process is created by ‘create’ and that ‘start’ just
tells the pre-existing container process to run the user-specified
code/program/executable/script/command/…. If you prefer a word beside
“code” from that list, let me know. But I think the current spec
wording is needlessly conflating it with “process”.

@tianon
Copy link
Member

tianon commented Nov 4, 2016

Rejected

I'm with @crosbymichael on this one -- I think process is a more appropriate word here than code.

Rejected with PullApprove

@wking
Copy link
Contributor Author

wking commented Nov 4, 2016

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

I'm with @crosbymichael on this one -- I think process is a more
appropriate word here than code.

In that case, we may need to revisit the current master and add a
longer description of the distinction. Because:

$ git grep 'user-specified code' origin/master -- runtime.md
origin/master:runtime.md: * created: the container has been created but the user-specified code has not yet been executed
origin/master:runtime.md: * running: the container has been created and the user-specified code is running
origin/master:runtime.md: * stopped: the container has been created and the user-specified code has been executed but is no longer running
origin/master:runtime.md: While the resources requested in the config.json MUST be created, the user-specified code (from process) MUST NOT be run at this time.
origin/master:runtime.md: The runtime MUST run the user-specified code, as specified by process.
origin/master:runtime.md:This means that all of the resources associated with the container MUST be created, however, the user-specified code MUST NOT be run at this time.
origin/master:runtime.md:This operation MUST run the user-specified code as specified by process.
$ git grep 'user-specified process' origin/master -- runtime.md
…no hits…

It looks like the changes I was proposing here have made it in through
other PRs:

Does someone in the ‘process’ camp want to open an issue about adding
clarifying language?

wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Nov 18, 2016
In [1], I'd proposed replacing our old "user-specified process" with
"user-specified code" to help distinguish between 'create' (cloning
the container process) and 'start' (signaling the container process to
execve (or similar) the user-specified $STUFF_FROM_THE_process_CONFIG
rejected, although the renaming proposed there had already landed via
dd0cd21 (Add a 'status' field to our state struct, 2016-05-26, opencontainers#462).

This PR attempts to find a common ground between "process" (preferred
by maintainers in opencontainers#466 [2,3,4], but which I consider incorrect [5])
and "code" (which maintainers found confusing [3,4,6]).  The Linux
execve(2) has says "program" and unpacks that to "a binary executable,
or a script starting with a [shebang]" [7].  proc(5) documents
/proc/[pid]/exe by talking about "the executed command" [8].  The
POSIX exec docs call this the "process image" and talk about loading
it from the "new process image file" [9].

POSIX formally defines "command" [11], "executable file" [12], and
"program" [13].  The only reference to "process image" in the
definitions is in the "executable file" entry.  The "command"
definition is focused on the shell, the "executable file" definition
is focused on files, and the "program" definition talks about a
"prepared sequence of instructions to the system", so "program" seems
like the best fit.

[1]: opencontainers#466
     Subject: runtime: Replace "user-specified process" with "user-specified code" in 'create'
[2]: opencontainers#466 (comment)
[3]: opencontainers#466 (comment)
[4]: opencontainers#466 (comment)
[5]: opencontainers#466 (comment)
[6]: opencontainers#466 (comment)
[7]: http://man7.org/linux/man-pages/man2/execve.2.html
[8]: http://man7.org/linux/man-pages/man5/proc.5.html
[9]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
[10]: https://git.kernel.org/cgit/docs/man-pages/man-pages.git/
[11]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_104
[12]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_154
[13]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_306

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Nov 18, 2016
In [1], I'd proposed replacing our old "user-specified process" with
"user-specified code" to help distinguish between 'create' (cloning
the container process) and 'start' (signaling the container process to
execve or similar the user-specified $STUFF_FROM_THE_process_CONFIG).
That PR was rejected, although the renaming proposed there had already
landed via dd0cd21 (Add a 'status' field to our state struct,
2016-05-26, opencontainers#462).

This PR attempts to find a common ground between "process" (preferred
by maintainers in opencontainers#466 [2,3,4], but which I consider incorrect [5])
and "code" (which maintainers found confusing [3,4,6]).  The Linux
execve(2) has says "program" and unpacks that to "a binary executable,
or a script starting with a [shebang]" [7].  proc(5) documents
/proc/[pid]/exe by talking about "the executed command" [8].  The
POSIX exec docs call this the "process image" and talk about loading
it from the "new process image file" [9].

POSIX formally defines "command" [11], "executable file" [12], and
"program" [13].  The only reference to "process image" in the
definitions is in the "executable file" entry.  The "command"
definition is focused on the shell, the "executable file" definition
is focused on files, and the "program" definition talks about a
"prepared sequence of instructions to the system", so "program" seems
like the best fit.

[1]: opencontainers#466
     Subject: runtime: Replace "user-specified process" with "user-specified code" in 'create'
[2]: opencontainers#466 (comment)
[3]: opencontainers#466 (comment)
[4]: opencontainers#466 (comment)
[5]: opencontainers#466 (comment)
[6]: opencontainers#466 (comment)
[7]: http://man7.org/linux/man-pages/man2/execve.2.html
[8]: http://man7.org/linux/man-pages/man5/proc.5.html
[9]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
[10]: https://git.kernel.org/cgit/docs/man-pages/man-pages.git/
[11]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_104
[12]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_154
[13]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_306

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Nov 18, 2016
In [1], I'd proposed replacing our old "user-specified process" with
"user-specified code" to help distinguish between 'create' (cloning
the container process) and 'start' (signaling the container process to
execve or similar the user-specified $STUFF_FROM_THE_process_CONFIG).
That PR was rejected, although the renaming proposed there had already
landed via dd0cd21 (Add a 'status' field to our state struct,
2016-05-26, opencontainers#462).

This PR attempts to find a common ground between "process" (preferred
by maintainers in opencontainers#466 [2,3,4], but which I consider incorrect [5])
and "code" (which maintainers found confusing [3,4,6]).  The Linux
execve(2) says "program" and unpacks that to "a binary executable, or
a script starting with a [shebang]" [7].  proc(5) documents
/proc/[pid]/exe by talking about "the executed command" [8].  The
POSIX exec docs call this the "process image" and talk about loading
it from the "new process image file" [9].

POSIX formally defines "command" [11], "executable file" [12], and
"program" [13].  The only reference to "process image" in the
definitions is in the "executable file" entry.  The "command"
definition is focused on the shell, the "executable file" definition
is focused on files, and the "program" definition talks about a
"prepared sequence of instructions to the system", so "program" seems
like the best fit.

[1]: opencontainers#466
     Subject: runtime: Replace "user-specified process" with "user-specified code" in 'create'
[2]: opencontainers#466 (comment)
[3]: opencontainers#466 (comment)
[4]: opencontainers#466 (comment)
[5]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_295
[6]: opencontainers#466 (comment)
[7]: http://man7.org/linux/man-pages/man2/execve.2.html
[8]: http://man7.org/linux/man-pages/man5/proc.5.html
[9]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
[10]: https://git.kernel.org/cgit/docs/man-pages/man-pages.git/
[11]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_104
[12]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_154
[13]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_306

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Nov 18, 2016
In [1], I'd proposed replacing our old "user-specified process" with
"user-specified code" to help distinguish between 'create' (cloning
the container process) and 'start' (signaling the container process to
execve or similar the user-specified $STUFF_FROM_THE_process_CONFIG).
That PR was rejected, although the renaming proposed there had already
landed via dd0cd21 (Add a 'status' field to our state struct,
2016-05-26, opencontainers#462).

This PR attempts to find a common ground between "process" (preferred
by maintainers in opencontainers#466 [2,3,4], but which I consider incorrect [5])
and "code" (which maintainers found confusing [3,4,6]).  The Linux
execve(2) says "program" and unpacks that to "a binary executable, or
a script starting with a [shebang]" [7].  proc(5) documents
/proc/[pid]/exe by talking about "the executed command" [8].  The
POSIX exec docs call this the "process image" and talk about loading
it from the "new process image file" (although they also sprinkle in a
number of “program” references, apparently interchangeably with
“process image”) [9].

POSIX formally defines "command" [11], "executable file" [12], and
"program" [13].  The only reference to "process image" in the
definitions is in the "executable file" entry.  The "command"
definition is focused on the shell, the "executable file" definition
is focused on files, and the "program" definition talks about a
"prepared sequence of instructions to the system", so "program" seems
like the best fit.

[1]: opencontainers#466
     Subject: runtime: Replace "user-specified process" with "user-specified code" in 'create'
[2]: opencontainers#466 (comment)
[3]: opencontainers#466 (comment)
[4]: opencontainers#466 (comment)
[5]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_295
[6]: opencontainers#466 (comment)
[7]: http://man7.org/linux/man-pages/man2/execve.2.html
[8]: http://man7.org/linux/man-pages/man5/proc.5.html
[9]: http://pubs.opengroup.org/onlinepubs/9699919799/functions/exec.html
[10]: https://git.kernel.org/cgit/docs/man-pages/man-pages.git/
[11]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_104
[12]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_154
[13]: http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap03.html#tag_03_306

Signed-off-by: W. Trevor King <wking@tremily.us>
@wking wking deleted the start-process-to-code branch May 11, 2017 09:10
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.

4 participants