config.md: minor changes for process #809

Mashimiao · 2017-05-12T03:26:56Z

Signed-off-by: Ma Shimiao [email protected]

wking · 2017-05-12T18:37:12Z

config.md

@@ -132,14 +132,14 @@ For Windows, see [mountvol][mountvol] and [SetVolumeMountPoint][set-volume-mount
 * **`env`** (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2001's `environ`][ieee-1003.1-2001-xbd-c8.1].
 * **`args`** (array of strings, REQUIRED) with similar semantics to [IEEE Std 1003.1-2001 `execvp`'s *argv*][ieee-1003.1-2001-xsh-exec].
  This specification extends the IEEE standard in that at least one entry is REQUIRED, and that entry is used with the same semantics as `execvp`'s *file*.
-* **`capabilities`** (object, OPTIONAL) is an object containing arrays that specifies the sets of capabilities for the process(es) inside the container. Valid values are platform-specific. For example, valid values for Linux are defined in the [capabilities(7)][capabilities.7] man page, such as `CAP_CHOWN`. Any value which cannot be mapped to a relevant kernel interface MUST cause an error.
+* **`capabilities`** (object, OPTIONAL) is an object containing arrays that specifies the sets of capabilities for the processes in the container. Valid values are platform-specific. For example, valid values for Linux are defined in the [capabilities(7)][capabilities.7] man page, such as `CAP_CHOWN`. Any value which cannot be mapped to a relevant kernel interface MUST cause an error.


They may not all have these caps anyway (e.g. you could inject in a new process into the container with more caps, or drop the caps in the main process or one of its children). This is just setting the caps for the process configured by process, so we should probably follow the earlier properties in this section and replace “the process(es) inside the container” with “the process”.

fixed, thanks

wking · 2017-05-12T18:37:19Z

config.md

-* **`terminal`** (bool, OPTIONAL) specifies whether a terminal is attached to that process, defaults to false.
-  As an example, if set to true on Linux a pseudoterminal pair is allocated for the container process and the pseudoterminal slave is duplicated on the container process's [standard streams][stdin.3].
+* **`terminal`** (bool, OPTIONAL) specifies whether a terminal is attached to the process, defaults to false.
+  As an example, if set to true on Linux a pseudoterminal pair is allocated for the process and the pseudoterminal slave is duplicated on the process's [standard streams][stdin.3].


While we touch this line, can we shift to four-space indents? Or we can punt that to a PR that just does whitespace adjustment for process (like #724 is doing for hooks).

Let's punt that to a new PR. There are too many lines starting with two-space indents

wking · 2017-05-13T06:06:02Z

config.md

@@ -132,14 +132,14 @@ For Windows, see [mountvol][mountvol] and [SetVolumeMountPoint][set-volume-mount
 * **`env`** (array of strings, OPTIONAL) with the same semantics as [IEEE Std 1003.1-2001's `environ`][ieee-1003.1-2001-xbd-c8.1].
 * **`args`** (array of strings, REQUIRED) with similar semantics to [IEEE Std 1003.1-2001 `execvp`'s *argv*][ieee-1003.1-2001-xsh-exec].
  This specification extends the IEEE standard in that at least one entry is REQUIRED, and that entry is used with the same semantics as `execvp`'s *file*.
-* **`capabilities`** (object, OPTIONAL) is an object containing arrays that specifies the sets of capabilities for the process(es) inside the container. Valid values are platform-specific. For example, valid values for Linux are defined in the [capabilities(7)][capabilities.7] man page, such as `CAP_CHOWN`. Any value which cannot be mapped to a relevant kernel interface MUST cause an error.
+* **`capabilities`** (object, OPTIONAL) is an object containing arrays that specifies the sets of capabilities for the process in the container. Valid values are platform-specific. For example, valid values for Linux are defined in the [capabilities(7)][capabilities.7] man page, such as `CAP_CHOWN`. Any value which cannot be mapped to a relevant kernel interface MUST cause an error.


I still think we want to drop “in(side) the container” here, like you're doing up above for terminal. And we should do that down in rlimits too. The initial process line establishes that the process is inside the container, and all these properties under process can just use “the process”.

OK, updated.

wking · 2017-05-15T17:47:30Z

config.md

  As an example, the ['no_new_privs'][no-new-privs] article in the kernel documentation has information on how this is achieved using a prctl system call on Linux.

 For Linux-based systems the process structure supports the following process-specific fields.

-* **`apparmorProfile`** (string, OPTIONAL) specifies the name of the AppArmor profile to be applied to processes in the container.
+* **`apparmorProfile`** (string, OPTIONAL) specifies the name of the AppArmor profile to be applied to process.


“to process” → “to the process”.

fixed, thanks

fixed, thanks

Still not fixed in 04864b7.

And maybe "to be applied to" -> "for", to get the same substance in fewer words.

sorry, may left it behind when doing rebase. updated.

…gain) Roll back the genericization from 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673). Lifting the restriction there seems to have been motivated by "Solaris supports capabilities", but that was before the split into a capabilities object which happened in eb114f0 (Add ambient and bounding capability support, 2017-02-02, opencontainers#675). It's not clear if Solaris supports ambient caps, or what Solaris API rlimits or noNewPrivileges were punting to [1]. And John Howard has recently confirmed that Windows does not support capabilities and is unlikely to do so in the future [2]. John's statement didn't directly address rlimits or noNewPrivileges, but we can always restore any of these properties to the Solaris/Windows platforms if/when we get docs about which API we're punting to on those platforms. Also add some backticks, remove the hyphens in "OPTIONAL) - the", standardize lines I touch to use "the process" [3], and use four-space indents here to keep Pandoc happy (see 7795661 (runtime.md: Fix sub-bullet indentation, 2016-06-08, opencontainers#495). [1]: opencontainers#673 (comment) [2]: opencontainers#810 (comment) [3]: opencontainers#809 (comment) Signed-off-by: W. Trevor King <[email protected]>

Mashimiao · 2017-05-23T03:07:17Z

@opencontainers/runtime-spec-maintainers PTAL

tianon · 2017-05-23T06:29:02Z

config.md

@@ -119,11 +119,11 @@ For Windows, see [mountvol][mountvol] and [SetVolumeMountPoint][set-volume-mount

 ## <a name="configProcess" />Process

-**`process`** (object, OPTIONAL) specifies the container process.
+**`process`** (object, OPTIONAL) specifies a process to run inside the container.


IMO "the container process" has a very different connotation here than simply "a process to run", especially in the context of a PID namespace (which can only exist as long as "the" container process does), so this change doesn't make a lot of sense to me.

OK, updated

Roll back the genericization from 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673). Lifting the restriction there seems to have been motivated by "Solaris supports capabilities", but that was before the split into a capabilities object which happened in eb114f0 (Add ambient and bounding capability support, 2017-02-02, opencontainers#675). It's not clear if Solaris supports ambient caps, or what Solaris API noNewPrivileges were punting to [1]. And John Howard has recently confirmed that Windows does not support capabilities and is unlikely to do so in the future [2]. He also confirmed that Windows does not support rlimits [3]. John's statement didn't directly address noNewPrivileges, but we can always restore any of these properties to the Solaris/Windows platforms if/when we get docs about which API we're punting to on those platforms. Also add some backticks, remove the hyphens in "OPTIONAL) - the", standardize lines I touch to use "the process" [4], and use four-space indents here to keep Pandoc happy (see 7795661 (runtime.md: Fix sub-bullet indentation, 2016-06-08, opencontainers#495). [1]: opencontainers#673 (comment) [2]: opencontainers#810 (comment) [3]: opencontainers#835 (comment) [4]: opencontainers#809 (comment) Signed-off-by: W. Trevor King <[email protected]>

crosbymichael · 2017-06-01T18:53:09Z

@Mashimiao can you rebase?

Mashimiao · 2017-06-02T01:21:17Z

@opencontainers/runtime-spec-maintainers rebased. PTAL

Signed-off-by: Ma Shimiao <[email protected]>

tianon · 2017-06-07T15:14:15Z

LGTM

hqhq · 2017-06-14T14:22:47Z

LGTM

Roll back the genericization from 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673). Lifting the restriction there seems to have been motivated by "Solaris supports capabilities", but that was before the split into a capabilities object which happened in eb114f0 (Add ambient and bounding capability support, 2017-02-02, opencontainers#675). It's not clear if Solaris supports ambient caps, or what Solaris API noNewPrivileges were punting to [1]. And John Howard has recently confirmed that Windows does not support capabilities and is unlikely to do so in the future [2]. He also confirmed that Windows does not support rlimits [3]. John's statement didn't directly address noNewPrivileges, but we can always restore any of these properties to the Solaris/Windows platforms if/when we get docs about which API we're punting to on those platforms. Also add some backticks, remove the hyphens in "OPTIONAL) - the", standardize lines I touch to use "the process" [4], and use four-space indents here to keep Pandoc happy (see 7795661 (runtime.md: Fix sub-bullet indentation, 2016-06-08, opencontainers#495). [1]: opencontainers#673 (comment) [2]: opencontainers#810 (comment) [3]: opencontainers#835 (comment) [4]: opencontainers#809 (comment) Signed-off-by: W. Trevor King <[email protected]>

With [1], which landed on 2016-05-26 [2], 'kill' is signalling "the process in the container". And since [3] (2017-06-04), the only remaining "processes" references have to do with namespaces, where the namespace in question is fairly clear. So I don't think there's much uncertainty left about what "container processes" means, or any implication that it's a single set of processes. [1]: https://github.com/duglin/runtime-spec/blob/be594153b522f52bebd0500cef6fe0f1d77f6a59/runtime.md#kill [2]: opencontainers/runtime-spec#384 (comment) [3]: opencontainers/runtime-spec#809 (comment)

wking reviewed May 12, 2017

View reviewed changes

Mashimiao force-pushed the config-minor-process-changes branch from 3e8ee95 to 641c704 Compare May 13, 2017 01:28

wking reviewed May 13, 2017

View reviewed changes

Mashimiao force-pushed the config-minor-process-changes branch from 641c704 to 9086f1d Compare May 13, 2017 10:12

wking reviewed May 15, 2017

View reviewed changes

Mashimiao force-pushed the config-minor-process-changes branch from 9086f1d to 2b59a13 Compare May 16, 2017 07:21

wking approved these changes May 16, 2017

View reviewed changes

wking mentioned this pull request May 18, 2017

config: Make capabilities, noNewPrivileges, and rlimits Linux-only (again) #835

Closed

tianon reviewed May 23, 2017

View reviewed changes

Mashimiao force-pushed the config-minor-process-changes branch from 2b59a13 to d3f0b51 Compare May 23, 2017 07:46

crosbymichael added this to the 1.0.0 milestone Jun 1, 2017

Mashimiao force-pushed the config-minor-process-changes branch from d3f0b51 to 04864b7 Compare June 2, 2017 01:20

config.md: minor changes for process

cbae6d0

Signed-off-by: Ma Shimiao <[email protected]>

Mashimiao force-pushed the config-minor-process-changes branch from 04864b7 to cbae6d0 Compare June 2, 2017 01:48

hqhq merged commit b5017e2 into opencontainers:master Jun 14, 2017

vbatts mentioned this pull request Jul 5, 2017

bump to v1.0.0-rc6 #882

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

config.md: minor changes for process #809

config.md: minor changes for process #809

Mashimiao commented May 12, 2017

wking May 12, 2017

Mashimiao May 13, 2017

wking May 12, 2017

Mashimiao May 13, 2017

wking May 13, 2017

Mashimiao May 13, 2017

wking May 15, 2017

Mashimiao May 16, 2017

wking Jun 2, 2017

wking Jun 2, 2017

Mashimiao Jun 2, 2017

Mashimiao commented May 23, 2017

tianon May 23, 2017

Mashimiao May 23, 2017

crosbymichael commented Jun 1, 2017

Mashimiao commented Jun 2, 2017

tianon commented Jun 7, 2017 •

edited by caniszczyk

Loading

hqhq commented Jun 14, 2017 •

edited by caniszczyk

Loading

config.md: minor changes for process #809

config.md: minor changes for process #809

Conversation

Mashimiao commented May 12, 2017

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

Mashimiao commented May 23, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crosbymichael commented Jun 1, 2017

Mashimiao commented Jun 2, 2017

tianon commented Jun 7, 2017 • edited by caniszczyk Loading

hqhq commented Jun 14, 2017 • edited by caniszczyk Loading

tianon commented Jun 7, 2017 •

edited by caniszczyk

Loading

hqhq commented Jun 14, 2017 •

edited by caniszczyk

Loading