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

Add language for compliance requirements around platforms #527

Merged
merged 2 commits into from
Aug 29, 2016

Conversation

mrunalp
Copy link
Contributor

@mrunalp mrunalp commented Aug 10, 2016

Signed-off-by: Mrunal Patel mrunalp@gmail.com

@wking
Copy link
Contributor

wking commented Aug 10, 2016

I'd rather not use both “one of the platforms” and “for the protocols
it implements”. If we want to split this spec into “protocols” we
should stick to the protocols language with the new addition. If we
want to switch to “platforms” language, we should update the existing
compliance language to use “platforms”.

The “platforms” phrasing is nice and specific for the current spec,
but discussion in #513 included “what if we support ‘at least one of
the following APIs’?” 1, so we may want to stick with the
less-specific “protocols” phrasing to cover both platforms and APIs.
In that case I think it makes sense to make the mapping between
protocols and MUST/SHOULD/… requirements clear. For example:

An implementation is not compliant if it fails to satisfy one or
more of the MUST or REQUIRED requirements for the protocols it
implements. An implementation that satisfies all the MUST or
REQUIRED and all the SHOULD requirements for its protocols is said
to be "unconditionally compliant". Protocols defined by this spec
are:

  • Linux containers: runtime.md, config.md, config-linux.md, and
    runtime-linux.md.
  • Solaris containers: runtime.md, config.md, and config-solaris.md.

And once we land an API spec, it would look something like:

  • Linux containers: runtime.md, config.md, config-linux.md,
    runtime-linux.md, and an API.

  • Solaris containers: runtime.md, config.md and config-solaris.md,
    and an API.

    APIs include at least one of:

  • The command line API

@mrunalp
Copy link
Contributor Author

mrunalp commented Aug 10, 2016

@wking I like your wording but we want to avoid repeating the line for each platform if we can. The only thing that will be different is config-platform.md.

@wking
Copy link
Contributor

wking commented Aug 10, 2016

On Wed, Aug 10, 2016 at 01:40:45PM -0700, Mrunal Patel wrote:

The only thing that will be different is config-platform.md.

That's not true now, although it would be if you can roll
runtime-linux.md into other places. #513 moves the standard-streams
stuff into the command line API and you could move the dev links into
config-linux.md. If we do get a consistent pattern for which files
are part of which “protocols”, I'm fine just explaining the pattern
instead of listing them all out.

@mikebrow
Copy link
Member

mikebrow commented Aug 10, 2016

I like @wking 's wording.

@philips
Copy link
Contributor

philips commented Aug 10, 2016

As discussed on the call @mrunalp please add language around "system architectures (ARM, x86-64, etc)" as well as OSes.

@hqhq
Copy link
Contributor

hqhq commented Aug 11, 2016

OSes are platforms, by architecture, we usually mean x86_64, ARM etc.. I think we need language about architectures.

@mrunalp
Copy link
Contributor Author

mrunalp commented Aug 16, 2016

Updated.

@@ -25,7 +25,10 @@ Table of Contents
In the specifications in the above table of contents, the keywords "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" are to be interpreted as described in [RFC 2119](http://tools.ietf.org/html/rfc2119) (Bradner, S., "Key words for use in RFCs to Indicate Requirement Levels", BCP 14, RFC 2119, March 1997).

An implementation is not compliant if it fails to satisfy one or more of the MUST or REQUIRED requirements for the protocols it implements.
An implementation that satisfies all the MUST or REQUIRED and all the SHOULD requirements for its protocols is said to be "unconditionally compliant".
An implementation that satisfies all the MUST or REQUIRED and all the SHOULD requirements for its protocols is said to be "unconditionally compliant" on one or more CPU architectures.
Copy link
Contributor

Choose a reason for hiding this comment

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

As it stands, your CPU qualification only covers “unconditionally compliant”, and we want to qualify “compliant” as well. How about:

An implementation is not compliant for a given CPU architecture if it fails to satisfy one or more of the MUST or REQUIRED requirements for the protocols it implements.
An implementation is compliant for a given CPU architecture if it satisfies all the MUST and REQUIRED requirements for the protocols it implements.
An implementation that satisfies all the MUST or REQUIRED and all the SHOULD requirements for its protocols on a given CPU architecture is said to be "unconditionally compliant" with those protocols and architectures.

which also covers “compliant” explicitly (it was previously implied to be “all cases that aren't explicitly non-compliant”).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Thanks!

…ctures

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp
Copy link
Contributor Author

mrunalp commented Aug 17, 2016

Pushed update.

@wking
Copy link
Contributor

wking commented Aug 17, 2016

On Wed, Aug 17, 2016 at 10:45:36AM -0700, Mrunal Patel wrote:

Pushed update.

6a5b144 looks good to me.

We probably want to remove wiggles like “The Solaris specification is
entirely optional” 1 now that we have this centralized definition of
what requirements belong to which protocols. I'm fine discussing that
as part of this PR if folks want, or punting it to follow up work.

…pliance language

Signed-off-by: Mrunal Patel <mrunalp@gmail.com>
@mrunalp
Copy link
Contributor Author

mrunalp commented Aug 17, 2016

@opencontainers/runtime-spec-maintainers PTAL

@wking
Copy link
Contributor

wking commented Aug 17, 2016 via email

@wking wking mentioned this pull request Aug 17, 2016
@hqhq
Copy link
Contributor

hqhq commented Aug 19, 2016

LGTM, I thought we might need some CPU architecture specific fields in spec, but we need to figure out what fields and if they exist.

Approved with PullApprove

@wking
Copy link
Contributor

wking commented Aug 19, 2016

On Thu, Aug 18, 2016 at 08:14:54PM -0700, Qiang Huang wrote:

LGTM, I thought we might need some CPU architecture specific fields
in spec, but we need to figure out what fields and if they exist.

The current PR has “for a given CPU architecture” and similar language
and the spec has a platform.arch field. What more are you looking
for? Are you talking about #39 and #69?


Protocols defined by this specification are:
* Linux containers: [runtime.md](runtime.md), [config.md](config.md), [config-linux.md](config-linux.md), and [runtime-linux.md](runtime-linux.md).
* Solaris containers: [runtime.md](runtime.md), [config.md](config.md), and [config-solaris.md](config-solaris.md).
Copy link
Contributor

Choose a reason for hiding this comment

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

Thinking over the tangentially-related #536, we probably want to include bundle.md and glossary.md in both lists.

I think including glossary.md is unfortunate; perhaps we can revisit the decision to put the UTF-8 requirement for configuration JSON there?

@hqhq
Copy link
Contributor

hqhq commented Aug 26, 2016

The current PR has “for a given CPU architecture” and similar language
and the spec has a platform.arch field. What more are you looking
for? Are you talking about #39 and #69?

Yeah, something like that, and I think the problem described in #39 is not resolved yet, GOARCH from golang is not sufficient for all architectures, maybe we should reopen it and discuss more.

@wking
Copy link
Contributor

wking commented Aug 26, 2016

On Fri, Aug 26, 2016 at 08:12:09AM -0700, Qiang Huang wrote:

… I think the problem described in #39 is not resolved yet, GOARCH
from golang is not sufficient for all architectures…

#441 brought over some language from opencontainers/image-spec#60 that
says “If you need something that's not in GOARCH, please let us know”.
I think that allows us to patch holes in GOARCH as they arise.

@hqhq
Copy link
Contributor

hqhq commented Aug 26, 2016

@wking Maybe for architecture itself, GOARCH would be sufficient, but we might need some information like byte order, variant, abi etc.. which might be considered out of architecture name, so argue with GOARCH may not be a wise move.

@wking
Copy link
Contributor

wking commented Aug 26, 2016

On Fri, Aug 26, 2016 at 08:45:34AM -0700, Qiang Huang wrote:

… so argue with GOARCH may not be a wise move.

Maybe not, I don't know. But I still think the “for a given CPU
architecture” covers the current non-OS portion of ‘platform’.
If/when we land language for platform.byte-order (or whatever), we can
amend the architecture-ish scoping here.

@vbatts
Copy link
Member

vbatts commented Aug 29, 2016

LGTM

Approved with PullApprove

@vbatts vbatts merged commit a3d7507 into opencontainers:master Aug 29, 2016
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Sep 15, 2016
So that the semantics are clear.  The platform/protocol disconnect is
unfortunate.  "Protocol" was chosen in de3f1af (Remove language
around Solaris being optional as it is covered in compliance language,
2016-08-17, opencontainers#527) because we may have compliance subsets that aren't
linked to platforms [2].  I'd be open to renaming the JSON tag from
platform:"..." -> protocol:"...", but that's probably more change than
it's worth.

[1]: opencontainers#527 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Sep 15, 2016
So that the semantics are clear.  The platform/protocol disconnect is
unfortunate.  "Protocol" was chosen in de3f1af (Remove language
around Solaris being optional as it is covered in compliance language,
2016-08-17, opencontainers#527) because we may have compliance subsets that aren't
linked to platforms [2].  I'd be open to renaming the JSON tag from
platform:"..." -> protocol:"...", but that's probably more change than
it's worth.

[1]: opencontainers#527 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jan 11, 2017
So that the semantics are clear.  The platform/protocol disconnect is
unfortunate.  "Protocol" was chosen in de3f1af (Remove language
around Solaris being optional as it is covered in compliance language,
2016-08-17, opencontainers#527) because we may have compliance subsets that aren't
linked to platforms [2].  I'd be open to renaming the JSON tag from
platform:"..." -> protocol:"...", but that's probably more change than
it's worth.

[1]: opencontainers#527 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jan 11, 2017
So that the semantics are clear.  The platform/protocol disconnect is
unfortunate.  "Protocol" was chosen in de3f1af (Remove language
around Solaris being optional as it is covered in compliance language,
2016-08-17, opencontainers#527) because we may have compliance subsets that aren't
linked to platforms [2].  I'd be open to renaming the JSON tag from
platform:"..." -> protocol:"...", but that's probably more change than
it's worth.

[1]: opencontainers#527 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Feb 8, 2017
So that the semantics of the tags are clear.

The platform/protocol disconnect is unfortunate.  "Protocol" was
chosen in de3f1af (Remove language around Solaris being optional as
it is covered in compliance language, 2016-08-17, opencontainers#527) because we may
have compliance subsets that aren't linked to platforms [1].  I'd be
open to renaming the JSON tag from platform:"..." -> protocol:"...",
but that's probably more change than it's worth.  The approach taken
in this commit, on the other hand, renames "protocol" to "platform".
I think that unnecessarily limits (or sets up confusing semantics for)
the platform/protocol values you can use, but two maintainers both
prefer "platform" [2,3].

[1]: opencontainers#527 (comment)
[2]: opencontainers#570 (comment)
[3]: opencontainers#570 (comment)

Signed-off-by: W. Trevor King <wking@tremily.us>
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.

6 participants