question about valid values runtime choose to support #813
Comments
wking
added a commit
to wking/ocitools-v2
that referenced
this issue
Sep 21, 2017
The spec isn't particuarly clear on this, saying [1]:
* Linux: valid values are defined in the
[`getrlimit(2)`][getrlimit.2] man page, such as `RLIMIT_MSGQUEUE`.
* Solaris: valid values are defined in the
[`getrlimit(3)`][getrlimit.3] man page, such as `RLIMIT_CORE`.
and [2]:
For each entry in `rlimits`, a [`getrlimit(3)`][getrlimit.3] on
`type` MUST succeed.
It doesn't say:
Linux: The value MUST be listed in the getrlimit(2) man page...
and it doesn't require the runtime to support the values listed in the
man page [3,4]. So there are three sets:
* Values listed in the man page
* Values supported by the host kernel
* Values supported by the runtime
And as the spec stands, these sets are only weakly coupled, and any of
them could be a sub- or superset of any other. In practice, I expect
the sets to all coincide, with the kernel occasionally adding or
removing values, and the man page and runtimes trailing along behind.
To address that, this commit weakens the previous hard error to a
SHOULD-level error. The PosixProcRlimitsValueError constant is new to
this commit, because the spec contains neither a MUST nor a SHOULD for
this condition, although I expect a SHOULD-level suggestion was
implied by [1]. The posixProcRef constant is cherry-picked from
27503c5 (complete spec errors of config.md, 2017-09-05, opencontainers#458).
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[3]: opencontainers/runtime-spec#813
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L463
Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/ocitools-v2
that referenced
this issue
Sep 21, 2017
The spec isn't particuarly clear on this, saying [1]:
* Linux: valid values are defined in the
[`getrlimit(2)`][getrlimit.2] man page, such as `RLIMIT_MSGQUEUE`.
* Solaris: valid values are defined in the
[`getrlimit(3)`][getrlimit.3] man page, such as `RLIMIT_CORE`.
and [2]:
For each entry in `rlimits`, a [`getrlimit(3)`][getrlimit.3] on
`type` MUST succeed.
It doesn't say:
Linux: The value MUST be listed in the getrlimit(2) man page...
and it doesn't require the runtime to support the values listed in the
man page [3,4]. So there are three sets:
* Values listed in the man page
* Values supported by the host kernel
* Values supported by the runtime
And as the spec stands, these sets are only weakly coupled, and any of
them could be a sub- or superset of any other. In practice, I expect
the sets to all coincide, with the kernel occasionally adding or
removing values, and the man page and runtimes trailing along behind.
To address that, this commit weakens the previous hard error to a
SHOULD-level error. The PosixProcRlimitsValueError constant is new to
this commit, because the spec contains neither a MUST nor a SHOULD for
this condition, although I expect a SHOULD-level suggestion was
implied by [1]. The posixProcRef constant is cherry-picked from
27503c5 (complete spec errors of config.md, 2017-09-05, opencontainers#458).
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[3]: opencontainers/runtime-spec#813
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L463
Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/ocitools-v2
that referenced
this issue
Sep 21, 2017
The spec isn't particuarly clear on this, saying [1]:
* Linux: valid values are defined in the
[`getrlimit(2)`][getrlimit.2] man page, such as `RLIMIT_MSGQUEUE`.
* Solaris: valid values are defined in the
[`getrlimit(3)`][getrlimit.3] man page, such as `RLIMIT_CORE`.
and [2]:
For each entry in `rlimits`, a [`getrlimit(3)`][getrlimit.3] on
`type` MUST succeed.
It doesn't say:
Linux: The value MUST be listed in the getrlimit(2) man page...
and it doesn't require the runtime to support the values listed in the
man page [3,4]. So there are three sets:
* Values listed in the man page
* Values supported by the host kernel
* Values supported by the runtime
And as the spec stands, these sets are only weakly coupled, and any of
them could be a sub- or superset of any other. In practice, I expect
the sets to all coincide, with the kernel occasionally adding or
removing values, and the man page and runtimes trailing along behind.
To address that, this commit weakens the previous hard error to a
SHOULD-level error. The PosixProcRlimitsValueError constant is new to
this commit, because the spec contains neither a MUST nor a SHOULD for
this condition, although I expect a SHOULD-level suggestion was
implied by [1]. The posixProcRef constant is cherry-picked from
27503c5 (complete spec errors of config.md, 2017-09-05, opencontainers#458).
Also make CheckRlimits a no-op on Windows, because the spec does not
define process.rlimits for that OS [5].
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[3]: opencontainers/runtime-spec#813
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L463
[5]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#posix-process
Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/ocitools-v2
that referenced
this issue
Sep 22, 2017
The spec isn't particuarly clear on this, saying [1]:
* Linux: valid values are defined in the
[`getrlimit(2)`][getrlimit.2] man page, such as `RLIMIT_MSGQUEUE`.
* Solaris: valid values are defined in the
[`getrlimit(3)`][getrlimit.3] man page, such as `RLIMIT_CORE`.
and [2]:
For each entry in `rlimits`, a [`getrlimit(3)`][getrlimit.3] on
`type` MUST succeed.
It doesn't say:
Linux: The value MUST be listed in the getrlimit(2) man page...
and it doesn't require the runtime to support the values listed in the
man page [3,4]. So there are three sets:
* Values listed in the man page
* Values supported by the host kernel
* Values supported by the runtime
And as the spec stands, these sets are only weakly coupled, and any of
them could be a sub- or superset of any other. In practice, I expect
the sets to all coincide, with the kernel occasionally adding or
removing values, and the man page and runtimes trailing along behind.
To address that, this commit weakens the previous hard error to a
SHOULD-level error. The PosixProcRlimitsValueError constant is new to
this commit, because the spec contains neither a MUST nor a SHOULD for
this condition, although I expect a SHOULD-level suggestion was
implied by [1]. The posixProcRef constant is cherry-picked from
27503c5 (complete spec errors of config.md, 2017-09-05, opencontainers#458).
Also make CheckRlimits a no-op on Windows, because the spec does not
define process.rlimits for that OS [5].
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[3]: opencontainers/runtime-spec#813
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L463
[5]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#posix-process
Signed-off-by: W. Trevor King <[email protected]>
This was referenced Sep 22, 2017
|
I think the idea was to avoid failing a runtime as non-compliant if
the host system didn't support a feature. But with the current
wording, you can have compliant runtimes that use that language to not
support most values described in the spec (everything except the ‘ro’
mount option on Windows [1] and ‘l3CacheSchema’ on Linux [2]?) even
when the host system supports the values.
I'd rather require runtimes to support at least the values the spec
lists as valid (with runtimes having the option to support additional
values as they see fit). I believe the “kernel doesn't support the
feature you've asked for case” is covered by [3]:
If the runtime cannot apply a property as specified in the
configuration, it MUST generate an error and a new container MUST
NOT be created.
Dropping the “runtimes MAY choose which subset” line would make using
compliant runtimes more reliable, although we'd probably want to
either inline our valid-values list (like we do for ‘ro’ in Windows
mounts [1]) or link to a specific version of an external doc (like we
do for Solaris rlimits [4,5]). Linking to a floating external doc
(like we do for Linux rlimits [6,7]) will lead to issues as values are
added to and removed from the external doc.
There have also been concerns about linking to man pages for lists of
valid values, because man pages may be an incomplete or incorrect
reflection of what the kernel actually supports. I'd address those
cases by either:
a. Submitting upstream patches to fix the man pages, and then linking
the spec to the patched version, or
b. Inlining the valid values in the spec so we don't need a normative
link to the buggy man page.
With both of those being fairly straightforward, I don't think “man
pages are unreliable” is a sufficient reason to avoid requiring
runtime support for values that the spec lists as valid.
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L84
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config-linux.md#L459
[3]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/runtime.md#L106
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L169
[5]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L857
[6]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168
[7]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L856
|
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this issue
Oct 4, 2017
In 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673) we got: Implementations MUST error out when invalid values are encountered and MUST generate an error message and error out when encountering valid values it chooses to not support In c763e64 (config: Move valid-value rules to their own section, 2017-02-07, opencontainers#681), I'd moved that out into the current "Valid values" section with the line I'm removing in this commit. However, giving runtimes a blanket clause to ignore valid values makes it harder to use runtimes, because you can't be sure an OCI-compliant runtime supports the spec-defined value you need [1]. There have been concerns about requiring runtimes to support values which are not supported by the host system [2]. But since 766abd6 (runtime.md: Require 'create' to fail if config.json asks for the impossible, 2016-09-08, opencontainers#559) we've had runtime.md wording that gives the runtime the ability to compliantly die in those cases. That text had a wording tweak in 72e8062 (runtime: Explicitly make process.* timing implementation-defined, 2017-02-27, opencontainers#700), and is now: If the runtime cannot apply a property as specified in the configuration, it MUST generate an error and a new container MUST NOT be created. With this line removed, consumers will be able to rely on valid-value support in compliant runtimes, although many properties could use clearer runtimes-MUST-support wording for those values. However, we can sort those out on a per-property basis. And runtimes are still allowed to support extention values not defined in the spec (e.g. new capability types, or mount options, or whatever). Like all extentions, it is up to the runtime and runtime-caller to negotiate support in those cases. [1]: opencontainers#813 (comment) [2]: opencontainers#673 (comment) Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this issue
Oct 4, 2017
In 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673) we got: Implementations MUST error out when invalid values are encountered and MUST generate an error message and error out when encountering valid values it chooses to not support In c763e64 (config: Move valid-value rules to their own section, 2017-02-07, opencontainers#681), I'd moved that out into the current "Valid values" section with the line I'm removing in this commit. However, giving runtimes a blanket clause to ignore valid values makes it harder to use runtimes, because you can't be sure an OCI-compliant runtime supports the spec-defined value you need [1]. There have been concerns about requiring runtimes to support values which are not supported by the host system [2]. But since 766abd6 (runtime.md: Require 'create' to fail if config.json asks for the impossible, 2016-09-08, opencontainers#559) we've had runtime.md wording that gives the runtime the ability to compliantly die in those cases. That text had a wording tweak in 72e8062 (runtime: Explicitly make process.* timing implementation-defined, 2017-02-27, opencontainers#700), and is now: If the runtime cannot apply a property as specified in the configuration, it MUST generate an error and a new container MUST NOT be created. With this commit, consumers will be able to rely on valid-value support in compliant runtimes. Many properties could use clearer runtimes-MUST-support wording for those values, but we can sort those out on a per-property basis in follow-up work. And runtimes are still allowed to support extention values not defined in the spec (e.g. new capability types, or mount options, or whatever). Like all extentions, it is up to the runtime and runtime-caller to negotiate support in those cases. [1]: opencontainers#813 (comment) [2]: opencontainers#673 (comment) Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this issue
Oct 4, 2017
In 718f9f3 (minor narrative cleanup regarding config compatibility, 2017-01-30, opencontainers#673) we got: Implementations MUST error out when invalid values are encountered and MUST generate an error message and error out when encountering valid values it chooses to not support In c763e64 (config: Move valid-value rules to their own section, 2017-02-07, opencontainers#681), I'd moved that out into the current "Valid values" section with the line I'm removing in this commit. However, giving runtimes a blanket clause to ignore valid values makes it harder to use runtimes, because you can't be sure an OCI-compliant runtime supports the spec-defined value you need [1]. There have been concerns about requiring runtimes to support values which are not supported by the host system [2]. But since 766abd6 (runtime.md: Require 'create' to fail if config.json asks for the impossible, 2016-09-08, opencontainers#559) we've had runtime.md wording that gives the runtime the ability to compliantly die in those cases. That text had a wording tweak in 72e8062 (runtime: Explicitly make process.* timing implementation-defined, 2017-02-27, opencontainers#700), and is now: If the runtime cannot apply a property as specified in the configuration, it MUST generate an error and a new container MUST NOT be created. As it stands both before and after this commit, a runtime can *still* die in 'create' because it cannot apply values supported by the host. This commit is just a step towards requiring runtimes to support as many values as the host supports; it doesn't get us all the way there. Many properties could use clearer runtimes-MUST-support wording for those values, but we can sort those out on a per-property basis in follow-up work. And runtimes are still allowed to support extention values not defined in the spec (e.g. new capability types, or mount options, or whatever). Like all extentions, it is up to the runtime and runtime-caller to negotiate support in those cases. [1]: opencontainers#813 (comment) [2]: opencontainers#673 (comment) Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/ocitools-v2
that referenced
this issue
Oct 23, 2017
The spec isn't particuarly clear on this, saying [1]:
* Linux: valid values are defined in the
[`getrlimit(2)`][getrlimit.2] man page, such as `RLIMIT_MSGQUEUE`.
* Solaris: valid values are defined in the
[`getrlimit(3)`][getrlimit.3] man page, such as `RLIMIT_CORE`.
and [2]:
For each entry in `rlimits`, a [`getrlimit(3)`][getrlimit.3] on
`type` MUST succeed.
It doesn't say:
Linux: The value MUST be listed in the getrlimit(2) man page...
and it doesn't require the runtime to support the values listed in the
man page [3,4]. So there are three sets:
* Values listed in the man page
* Values supported by the host kernel
* Values supported by the runtime
And as the spec stands, these sets are only weakly coupled, and any of
them could be a sub- or superset of any other. In practice, I expect
the sets to all coincide, with the kernel occasionally adding or
removing values, and the man page and runtimes trailing along behind.
To address that, this commit weakens the previous hard error to a
SHOULD-level error. The PosixProcRlimitsTypeValueError constant is
new to this commit, because the spec contains neither a MUST nor a
SHOULD for this condition, although I expect a SHOULD-level suggestion
was implied by [1].
Also make CheckRlimits a no-op on Windows, because the spec does not
define process.rlimits for that OS [5].
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L168-L169
[3]: opencontainers/runtime-spec#813
[4]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0/config.md#L463
[5]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0/config.md#posix-process
Signed-off-by: W. Trevor King <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
In config.md, spec says
Unless support for a valid value is explicitly required, runtimes MAY choose which subset of the valid values it will support.I think there may will be a problem, if different runtime support differen subset. If so, we can't guarantee the portability.
I think this is against our goal of standardization.
The text was updated successfully, but these errors were encountered: