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.

Sign up for GitHub

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

config: Make 'process.args' optional #620

Closed
wants to merge 1 commit into from
Closed

config: Make 'process.args' optional #620

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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 config.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,9 @@ See links for details about [mountvol](http://ss64.com/nt/mountvol.html) and [Se
* **`cwd`** (string, REQUIRED) is the working directory that will be set for the executable.
This value MUST be an absolute path.
* **`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].
* **`args`** (array of strings, OPTIONAL) 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*.
This property is REQUIRED when [`start`](runtime.md#start) is called.

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

Expand Down
1 change: 1 addition & 0 deletions runtime.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ This operation MUST generate an error if it is not provided the container ID.
Attempting to start a container that does not exist MUST generate an error.
Attempting to start an already started container MUST have no effect on the container and MUST generate an error.
This operation MUST run the user-specified program as specified by [`process`](config.md#process).
This operation MUST generate an error if `process.args` was not set.
Copy link
Member

@mikebrow mikebrow Jan 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

The process.args array is currently "OPTIONAL", not REQUIRED. So if it's optional we need to explain why/when its REQUIRED. I know you know why, but it does not explicitly say why / when. The MUST requirement being in the START command paragraph "implies" that the process.args array (first element) is REQUIRED only when START is used but is not explicit or explained. The word "set" does not express explicitly what you mean. I only mention this because I believe MUST language should only have one interpretation.

Copy link
Contributor Author

@wking wking Jan 11, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

After some discussion on IRC, @mikebrow recommended the following addition to the process.args docs:

When start is used process.args is no longer OPTIONAL and MUST be set.

There's also a similar conditional requirement in the device spec, but “REQUIRED unless you won't call start” and “OPTIONAL unless you'll call start in which case it's REQUIRED” both feel too awkward.

With 7071706 → e45ddba I've added:

When start will be called, this property is REQUIRED.

which gets the cross-reference to the start command. It replaces @mikebrow's “is no longer OPTIONAL and MUST be set” with “is REQUIRED”, because I like “REQUIRED” better than “MUST be set”. I think “is no longer OPTIONAL” is fairly clear from the context, but I'm ok adding that back in if folks feel like it's necessary. And I've replaced “is used” with “will be called” because you need to set this field at create time in order to use it when start is called.


Upon successful completion of this operation the `status` property of this container MUST be `running`.

Expand Down
3 changes: 1 addition & 2 deletions schema/config-schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,8 +74,7 @@
"id": "https://opencontainers.org/schema/bundle/process",
"type": "object",
"required": [
"cwd",
"args"
"cwd"
],
"properties": {
"args": {
Expand Down
2 changes: 1 addition & 1 deletion specs-go/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Process struct {
// User specifies user information for the process.
User User `json:"user"`
// Args specifies the binary and arguments for the application to execute.
Args []string `json:"args"`
Args []string `json:"args,omitempty"`
// Env populates the process environment for the process.
Env []string `json:"env,omitempty"`
// Cwd is the current working directory for the process and must be
Expand Down