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

docs/command-line-interface: Require complete runtime coverage #615

Merged

Conversation

wking
Copy link
Contributor

@wking wking commented Apr 4, 2018

I didn't see wording in the previous version of this spec to require runtimes to support the full API, so I've added language for that here. This ensures that callers who only need the specified API will be able to run with an any runtime that is compliant with this API. You could get some way towards this requirement before by leaning on
the spec's:

An implementation is compliant if it satisfies all the applicable MUST, REQUIRED, and SHALL requirements.

but there are a number of requirements that currently lack lower-level MUSTs. For example, without the full-API requirement, the only delete requirement would be:

The runtime MUST NOT attempt to read from its stdin.

You could have argued that a runtime which did not implement delete satisfied that condition, and so was “compliant” even in the absence of delete support. However, such a runtime would likely break all compliant callers. This commit ensures that such a runtime would clearly be “not compliant”.

The portability SHOULD NOT is just a hint to callers. If you require extentions beyond this API spec, you'll want to ensure you are comfortable with the potential reduction in compatible runtimes.

I've made a patch-level version bump for this commit. If you consider the new requirement to be an extention from the previous spec, there was no way to be a caller rely on any 1.0.0 API calls working. I think the new explicit requirement was an implicit requirement of the 1.0.0, and that sort of typo fix deserves a patch-level bump. For example, we've been requiring runtimes to support create in validation/create.go since those tests landed. This commit just gives us a MUST we can cite for those (and other similar) failures.

I didn't see wording in the previous version of this spec to require
runtimes to support the full API, so I've added language for that
here.  This ensures that callers who only need the specified API will
be able to run with an any runtime that is compliant with this API.
You could get some way towards this requirement before by leaning on
the spec's:

> An implementation is compliant if it satisfies all the applicable
> MUST, REQUIRED, and SHALL requirements.

but there are a number of requirements that currently lack lower-level
MUSTs.  For example, without the full-API requirement, the only
'delete' requirement would be:

> The runtime MUST NOT attempt to read from its stdin.

You could have argued that a runtime which did not implement 'delete'
satisfied that condition, and so was "compliant" even in the absence
of 'delete' support.  However, such a runtime would likely break all
compliant callers.  This commit ensures that such a runtime would
clearly be "not compliant".

The portability SHOULD NOT is just a hint to callers.  If you require
extentions beyond this API spec, you'll want to ensure you are
comfortable with the potential reduction in compatible runtimes.

I've made a patch-level version bump for this commit.  If you consider
the new requirement to be an extention from the previous spec, there
was no way to be a caller rely on any 1.0.0 API calls working.  I
think the new explicit requirement was an implicit requirement of the
1.0.0, and that sort of typo fix deserves a patch-level bump.  For
example, we've been requiring runtimes to support 'create' in
validation/create.go since those tests landed.  This commit just gives
us a MUST we can cite for those (and other similar) failures.

Signed-off-by: W. Trevor King <wking@tremily.us>
@alban
Copy link
Contributor

alban commented Apr 5, 2018

LGTM (but I'm not a maintainer)

@zhouhao3
Copy link

zhouhao3 commented Apr 6, 2018

LGTM

Approved with PullApprove

@zhouhao3 zhouhao3 merged commit fcd5b23 into opencontainers:master Apr 6, 2018
@wking wking deleted the require-runtime-support-for-full-api branch April 6, 2018 16:08
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.

3 participants