config: Link platform:"…" JSON tags with ~~protocol~~ platform slugs #570
Conversation
|
@wking Please break-out into separate PRs |
wking
force-pushed
the
go-protocol-definition
branch
from
eb82bad to
c987b8f
Compare
|
I pushed the header normalization out into #650 and rebased the remaining two commits onto master with eb82bad → c987b8f, so this is now focused exclusively on slugging protocols and defining the Go binding's |
wking
force-pushed
the
go-protocol-definition
branch
from
c987b8f to
a78a538
Compare
config.md
Outdated
| @@ -2,6 +2,7 @@ | |||
|
|
|||
| The container's top-level directory MUST contain a configuration file called `config.json`. | |||
| The canonical schema is defined in this document, but there is a JSON Schema in [`schema/config-schema.json`](schema/config-schema.json) and Go bindings in [`specs-go/config.go`](specs-go/config.go). | |||
| For properties that are only defined for some [protocols](spec.md#protocols), the Go property has a `platform` tag listing those protocols (e.g. `platform:"linux,solaris"`). | |||
There was a problem hiding this comment.
I'd call them platforms not protocols, because that is what they actually are and why the tag is named that way.
There was a problem hiding this comment.
I'd call them platforms not protocols, because that is what they actually are and why the tag is named that way.
That's what they are now, but protocols leaves the door open for future entries that are not platforms. Are we comfortable closing that door (I'd rather leave it open, but I'm not a maintainer)?
If maintainers are comfortable closing the door, how thoroughly should I replace ‘protocol’ with ‘platform’? Do you want me to change these too:
$ git grep -i protocol origin/master
origin/master:spec.md:Protocols defined by this specification are:
origin/master:spec.md:An implementation is not compliant for a given CPU architecture if it fails to satisfy one or more of the MUST, REQUIRED, or SHALL requirements for the protocols it implements.
origin/master:spec.md:An implementation is compliant for a given CPU architecture if it satisfies all the MUST, REQUIRED, and SHALL requirements for the protocols it implements.
For example,
An implementation is compliant for a given CPU architecture if it satisfies all the MUST, REQUIRED, and SHALL requirements for the platforms it implements.
reads a bit strangely to me, because we are currently including the CPU architecture in platform. Maybe the protocol-less version of that line would be:
An implementation is compliant for a given CPU architecture if it satisfies all the MUST, REQUIRED, and SHALL requirements for the operating systems it implements.
There was a problem hiding this comment.
I'd vote for platform as well, and I don't see why our spec would define such non-platform protocol such as linux-cgroup protocol.
There was a problem hiding this comment.
If we use platform, it should be consistent through all spec, I think your first replacement is fine, an implementation for a specific platform sound reasonable to me .
There was a problem hiding this comment.
I don't see why our spec would define such non-platform protocol such as
linux-cgroupprotocol.
I'd floated making hooks a protocol to accommodate a Windows runtime that chose not to support then despite their being possible on Windows (although I'm not sure if hooks are possible or not on Windows). And I think we want a base protocol to cover stuff that all compliant runtimes will need (e.g. ociVersion). But having a maintainer reply to my counter-argument with a clear rejection of "protocol" is enough for a rebase here. I'll get it pushed sometime today, and then file a follow-up PR adding base, which you can pick up if it looks useful.
|
@wking Could your update and rebase since this PR has a |
We'll be referring to these in code, and using a slug everywhere avoids having to define both a slug form (linux) and an English form (Linux containers). Signed-off-by: W. Trevor King <wking@tremily.us>
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>
wking
force-pushed
the
go-protocol-definition
branch
from
a78a538 to
4af0c72
Compare
wking
changed the title
|
On Wed, Feb 08, 2017 at 12:37:06AM -0800, Qiang Huang wrote:
@wking Could your update and rebase since this PR has a `v1.0.0`
milestone, thanks.
Rebased with “protocol” → “platform” (although I still think that's
unnecessarily restricting future platform/protocol choices [1]) in
a78a538 → 4af0c72. Despite that change, I think 4af0c72 is still a
step mostly-forward because it defines the semantics of the JSON tags.
[1]: #570 (comment)
|
|
LGTM |
|
LGTM |
Spun off from here.
More details on the individual pivots in the commit messages. The
meat is the one-liner in the last commit, and the bit I'm not sure
about (also discussed in the commit message ;) is whether we want to
rename the JSON tag from
platform:"…"toprotocol:"…"toconsolidate around the “protocol” term used in #527 to decouple
compliance from platforms. For example, if we want to add
optional extensions (e.g. allowing Linux containers that use
namespacing but not cgroups), we could have a ‘linux’ protocol (with
the base stuff) and a ‘linux-cgroups’ protocol (with the cgroups stuff
and the base stuff). Runtimes that implemented both would be
compliant with both ‘linux’ and ‘linux-cgroups’.