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.md: only required properties are MUST applied #935

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion runtime.md
Original file line number Diff line number Diff line change
Expand Up @@ -100,9 +100,10 @@ This operation MUST [generate an error](#errors) if it is not provided a path to
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](#errors) and a new container MUST NOT be created.
This operation MUST create a new container.

All of the properties configured in [`config.json`](config.md) except for [`process`](config.md#process) MUST be applied.
All of the properties configured in [`config.json`](config.md) except for [`process`](config.md#process) and [Platform](spec.md#platforms)-specific configuration beyond the target platform MUST be applied.
Copy link
Contributor

@wking wking Nov 9, 2017

Choose a reason for hiding this comment

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

I feel like this is the idea that the extensibility section was trying to convey,and which I was trying to tighten up in #680. Instead of attempting to repeat the condition here, I would rather update that section to cover the non-ignore-ability of properties defined by this spec for that target platform (even ones like process.rlimits, which lie outside of linux). Text here that references that section would be great, though. Maybe something like:

All of the [recognized properties](config.md#extensibility) configured in [`config.json`](config.md) except for [`process`](config.md#process) MUST be applied.

with wording in the extensibility section and/or glossary to more formally define "recognized properties"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wking I don't think the extensibility section conveys this. In my understanding, unknown property means the property is not defined in spec, not the property that runtime doesn't support (if we have different understanding, maybe glossary for unknown property is needed?).

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think the extensibility section conveys this.

Not yet (#680 is me trying to tighten part of it down). I'm staying that the extensibility section is where we already attempt to cover this sort of thing, not that it already covers the hole you've identified here.

I'd rather tighten up the extensibility section to cover all of the “which properties can the runtime ignore?” conditions, because if it covers a subset, and runtime.md adds a few more, then config authors might read the extensibility section, think they understand what the runtime can ignore, and then be surprised when it also ignores something that was added in runtime.md. It's easier to understand if we collect all the conditions in one place.

[`process.args`](config.md#process) MUST NOT be applied until triggered by the [`start`](#start) operation.
The remaining `process` properties MAY be applied by this operation.
[Platform](spec.md#platforms)-specific configuration MAY be ignored if it's not the runtime's target platform.
If the runtime cannot apply a property as specified in the [configuration](config.md), it MUST [generate an error](#errors) and a new container MUST NOT be created.

The runtime MAY validate `config.json` against this spec, either generically or with respect to the local system capabilities, before creating the container ([step 2](#lifecycle)).
Expand Down