source and schema: differentiate with examples by vbatts · Pull Request #276 · opencontainers/runtime-spec

vbatts · 2015-12-18T21:50:29Z

The standard is on the JSON schema (not yet IETF spec JSON-schema), such
that it is not implemenations specific. Thus far, the reference has been
in how golang source renders the JSON documents.

Having the JSON source and the markdown documents in sync has been an
ongoing step to keep in sync.

Separating these two allows the golang source to continue being a
reference, but the JSON schema in the documentation to be the
reference.

As validation tooling is refined, then it will facilitate ensuring
the available golang source conforms to the reference JSON.

wking · 2015-12-18T22:08:20Z

nit: “exmample” → “example”, and I'd drop “contents” and “(generated with runc spec)”. The generation note may be useful for a commit message, but doesn't seem useful in the spec itself.

wking · 2015-12-18T22:28:06Z

On Fri, Dec 18, 2015 at 01:50:31PM -0800, Vincent Batts wrote:

Separating these two allows the golang source to continue being a
reference, but the JSON schema in the documentation to be the
reference.

I'm not sure these examples have much normative weight, but they
certainly are useful for showing how the normative sections (e.g. 1)
fit together. And it would let runC and similar drop the examples
they currently embed 2 by providing a centrally-managed example.
So +1 on this going in (although I've left a few inline comments about
the example content).

vishh · 2015-12-18T22:30:04Z

+1 for the overall intent of separating schema from the golang structures.

crosbymichael · 2015-12-18T22:52:57Z

The only downside is that it makes the Go import path ugly.

wking · 2015-12-18T23:00:52Z

On Fri, Dec 18, 2015 at 02:52:59PM -0800, Michael Crosby wrote:

The only downside is that it makes the Go import path ugly.

We expect Go import paths to get more interesting soon anyway, no?
@mrunalp's platform-directory revival 1 seemed to go over well in
the 2015-12-16 meeting. I don't remember hearing any pushback other
than @duglin's concern that it was adding platform-dependent
compilation instead of removing it 2, and once we cleared that up I
think everyone was in favor of it 3.

vishh · 2015-12-19T01:46:18Z

@vbatts: How do we differentiate between optional and required fields? And how will this address extensible fields?

wking · 2015-12-19T04:10:30Z

On Fri, Dec 18, 2015 at 05:46:21PM -0800, Vish Kannan wrote:

How do we differentiate between optional and required fields? And
how will this address extensible fields?

The normative specification will still be things like 1. This is
just a global example to help tie all of that together.

vbatts · 2015-12-21T20:22:13Z

@vishh that has to be done per field and value. Like the comments that describe them.

***`path`** (string, required) ...
***`readonly`** (bool, optional) ...

It's not very parsable, but is legible.

vbatts · 2015-12-21T20:23:36Z

@crosbymichael you thinking those paths would be ugly was the first thing that came to mind. What do you recommend besides github.com/opencontainers/specs/src/specs ?

vbatts · 2015-12-21T20:24:24Z

otherwise, I've rebased and updated the structs a little based on comments.
PTAL

wking · 2015-12-21T20:39:12Z

On Mon, Dec 21, 2015 at 12:24:27PM -0800, Vincent Batts wrote:

otherwise, I've rebased and updated the structs a little based on comments.

All the ecccada → 99d1df7 changes look good to me, but this is still
duplicating fields required by #164 1. And there are still a number
of nulls in the resources that can be left out now that #233 has
landed a pointer-based approach to “don't touch this”.

vbatts · 2016-01-13T21:22:18Z

rebased. PTAL

wking · 2016-01-13T21:37:12Z

On Wed, Jan 13, 2016 at 01:22:20PM -0800, Vincent Batts wrote:

rebased. PTAL

$ diff -u <(git show -M50 99d1df7) <(git show -M50 998344)
--- /dev/fd/63 2016-01-13 13:33:17.048080367 -0800
+++ /dev/fd/62 2016-01-13 13:33:17.048080367 -0800
@@ -1,4 +1,4 @@
-commit 99d1df7
+commit 9983445
Author: Vincent Batts vbatts@hashbangbash.com
Date: Fri Dec 18 15:47:21 2015 -0500

So that is just “rebased onto the new master to pickup changes in the
renamed files”. I still see the nulls I suggest we remove 1.

mrunalp · 2016-01-13T22:43:19Z

We can do this using ocitools to generate and a Makefile target to update this.

wking · 2016-01-13T22:51:53Z

On Wed, Jan 13, 2016 at 02:43:21PM -0800, Mrunal Patel wrote:

We can do this using ocitools to generate and a Makefile target to
update this.

I'm concerned that:

Write normative Markdown (this repo)
Update informative Go types (this repo)
Update ocitools to pull in new Go types
Generate new example JSON with ocitools
Update these informative examples (this repo)

is too long a loop, involving a number of non-normative moving parts.
It also means that without some ocitools hoop-jumping, you'd have to
land a single semantic update in two PRs (one PR for (1) and (2), and
another PR for (5)).

But if the idea is that PR submitters will use whatever tooling they
want (including a Make-spawned ocitools or hand-editing) to update
this section, then +1 to anything folks think makes that easier.

vbatts · 2016-01-19T16:14:58Z

rebased

vbatts · 2016-01-20T18:31:41Z

brainstorming: would the sample json be better served as an config.example.json file sitting in the directory, rather than appended to the *.md?

wking · 2016-03-07T22:04:03Z

+ "CAP_NET_BIND_SERVICE"
+ ],
+ "apparmorProfile": "",
+ "selinuxProcessLabel": ""


Before these moved @mrunalp and I were suggesting we drop the empty values. Since #329 they're omitempty, so I see no reason to keep them in this example.

Its fine to have these in an example because how else would ppl know that they are there without them being in an EXAMPLE

On Tue, Mar 08, 2016 at 10:43:21AM -0800, Michael Crosby wrote:
"apparmorProfile": "",
"selinuxProcessLabel": ""
Its fine to have these in an example because how else would ppl know
that they are there without them being in an EXAMPLE
I'm fine listing them here with meaningful content. And regardless of
whether they're listed here, I think people should be able to discover
them via the process documentation [1](which seems to use
‘selinuxLabel’ and not ‘selinuxProcessLabel’ anyway).

vbatts · 2016-03-09T16:27:27Z

rebased

mrunalp · 2016-03-09T17:34:36Z

Should we format the json using jq to get a consistent output?

crosbymichael · 2016-03-09T19:02:18Z

import"github.com/opencontainers/specs/spec-go"

vbatts · 2016-03-09T19:04:12Z

@crosbymichael done

The standard is on the JSON schema (not yet IETF spec JSON-schema), such that it is not implemenations specific. Thus far, the reference has been in how golang source renders the JSON documents. Having the JSON source and the markdown documents in sync has been an ongoing step to keep in sync. Separating these two allows the golang source to continue being _a_ reference, but the JSON schema in the documentation to be _the_ reference. As validation tooling is refined, then it will facilitate ensuring the available golang source conforms to the reference JSON. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts · 2016-03-09T19:18:36Z

rebased again

crosbymichael · 2016-03-09T19:20:23Z

LGTM

mrunalp · 2016-03-09T19:28:04Z

+ "hooks": {
+ "prestart": [
+ {
+ "path": "/",


This should be /usr/bin/uptime
args[0] could match that or be something else or just empty.

Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

mrunalp · 2016-03-09T19:52:52Z

LGTM

source and schema: differentiate with examples

The label changed in 5a8a779 (Move process specific settings to process, 2016-03-02, opencontainers#329) and 7bf06d5 (source and schema: differentiate with examples, 2015-12-18, opencontainers#276) missed this instance when rebasing around opencontainers#329. Signed-off-by: W. Trevor King <wking@tremily.us>

Also: * Update the link to Go bindings after 7bf06d5 (source and schema: differentiate with examples, 2015-12-18, opencontainers#276). * Add a reference to the JSON Schema after cdcabde (schema: JSON Schema and validator for `config.json`, 2016-01-19, opencontainers#313). It's pretty clear that the Go bindings cannot be canonical on their own, because they do not define limits (e.g. the 0 through 512 range for FileMode). The JSON Schema is closer, but still does not cover everything (e.g. "a directory must exist at root.path"). Both the Go bindings and the JSON Schema could grow to cover the full spec by adding that sort of thing to comments and descriptions, but that's not how things seem to be working now. Signed-off-by: W. Trevor King <wking@tremily.us>

Catch up with 7bf06d5 (source and schema: differentiate with examples, 2015-12-18, opencontainers#276). Signed-off-by: W. Trevor King <wking@tremily.us>

Also: * Update the link to Go bindings after 7bf06d5 (source and schema: differentiate with examples, 2015-12-18, opencontainers#276). * Add a reference to the JSON Schema after cdcabde (schema: JSON Schema and validator for `config.json`, 2016-01-19, opencontainers#313). It's pretty clear that the Go bindings cannot be canonical on their own, because they do not define limits (e.g. the 0 through 512 range for FileMode). The JSON Schema is closer, but still does not cover everything (e.g. "a directory must exist at root.path"). Both the Go bindings and the JSON Schema could grow to cover the full spec by adding that sort of thing to comments and descriptions, but that's not how things seem to be working now. Signed-off-by: W. Trevor King <wking@tremily.us>

Catch up with 7bf06d5 (source and schema: differentiate with examples, 2015-12-18, opencontainers#276). Signed-off-by: W. Trevor King <wking@tremily.us>

The label changed in 5a8a779 (Move process specific settings to process, 2016-03-02, opencontainers#329) and 7bf06d5 (source and schema: differentiate with examples, 2015-12-18, opencontainers#276) missed this instance when rebasing around opencontainers#329. Signed-off-by: W. Trevor King <wking@tremily.us>

Also: * Update the link to Go bindings after 7bf06d5 (source and schema: differentiate with examples, 2015-12-18, opencontainers#276). * Add a reference to the JSON Schema after cdcabde (schema: JSON Schema and validator for `config.json`, 2016-01-19, opencontainers#313). It's pretty clear that the Go bindings cannot be canonical on their own, because they do not define limits (e.g. the 0 through 512 range for FileMode). The JSON Schema is closer, but still does not cover everything (e.g. "a directory must exist at root.path"). Both the Go bindings and the JSON Schema could grow to cover the full spec by adding that sort of thing to comments and descriptions, but that's not how things seem to be working now. Signed-off-by: W. Trevor King <wking@tremily.us>

Catch up with 7bf06d5 (source and schema: differentiate with examples, 2015-12-18, opencontainers#276). Signed-off-by: W. Trevor King <wking@tremily.us>

wking reviewed Dec 18, 2015
View reviewed changes

vbatts force-pushed the schema branch from ecccada to 99d1df7Compare December 21, 2015 20:20

This was referenced Jan 12, 2016
Aims to clarification, adjusts the directory structure for specs. #297
Closed
Update ROADMAP.md #300
Merged

vbatts added this to the v0.3.0 milestone Jan 13, 2016

vbatts mentioned this pull request Jan 13, 2016
proto: initial protobuf for config #185
Closed

vbatts force-pushed the schema branch from 99d1df7 to 9983445Compare January 13, 2016 21:22

vbatts mentioned this pull request Jan 13, 2016
Is it necessary to provide a minimum config in specs? #105
Closed

vbatts force-pushed the schema branch from 9983445 to 9a1a481Compare January 19, 2016 16:14

vbatts mentioned this pull request Jan 19, 2016
json-schema: initial pass at schema and validator #313
Merged
5 tasks

wking reviewed Mar 7, 2016
View reviewed changes

vbatts force-pushed the schema branch from cd6cff9 to 603e077Compare March 9, 2016 16:27

vbatts force-pushed the schema branch from 603e077 to 4dcb06fCompare March 9, 2016 16:29

vbatts force-pushed the schema branch from 4dcb06f to 2b37f45Compare March 9, 2016 19:04

vbatts force-pushed the schema branch from 2b37f45 to 7bf06d5Compare March 9, 2016 19:18

mrunalp reviewed Mar 9, 2016
View reviewed changes

config: formatted example json
cfbf70c
Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts force-pushed the schema branch from 59db3e4 to cfbf70cCompare March 9, 2016 19:40

mrunalp pushed a commit that referenced this pull request Mar 9, 2016
Merge pull request#276from vbatts/schema
fae9a3e
source and schema: differentiate with examples

mrunalp merged commit fae9a3e into opencontainers:masterMar 9, 2016

This was referenced Mar 9, 2016
specs-go/config: Fix 'SelinuxProcessLabel' -> 'SelinuxLabel' #339
Merged
add omitempty to 'Devices #340
Merged

wking mentioned this pull request Mar 10, 2016
config: Make Markdown canonical #342
Merged

vbatts deleted the schema branch April 12, 2016 03:51

Uh oh!

Conversation

vbatts commented Dec 18, 2015

Uh oh!

wkingDec 18, 2015

Choose a reason for hiding this comment

Uh oh!

wking commented Dec 18, 2015

Uh oh!

vishh commented Dec 18, 2015

Uh oh!

crosbymichael commented Dec 18, 2015

Uh oh!

wking commented Dec 18, 2015

Uh oh!

vishh commented Dec 19, 2015

Uh oh!

wking commented Dec 19, 2015

Uh oh!

vbatts commented Dec 21, 2015

Uh oh!

vbatts commented Dec 21, 2015

Uh oh!

vbatts commented Dec 21, 2015

Uh oh!

wking commented Dec 21, 2015

Uh oh!

vbatts commented Jan 13, 2016

Uh oh!

wking commented Jan 13, 2016

Uh oh!

mrunalp commented Jan 13, 2016

Uh oh!

wking commented Jan 13, 2016

Uh oh!

vbatts commented Jan 19, 2016

Uh oh!

vbatts commented Jan 20, 2016

Uh oh!

wkingMar 7, 2016

Choose a reason for hiding this comment

Uh oh!

crosbymichaelMar 8, 2016

Choose a reason for hiding this comment

Uh oh!

wkingMar 8, 2016

Choose a reason for hiding this comment

Uh oh!

vbatts commented Mar 9, 2016

Uh oh!

mrunalp commented Mar 9, 2016

Uh oh!

crosbymichael commented Mar 9, 2016

Uh oh!

vbatts commented Mar 9, 2016

Uh oh!

vbatts commented Mar 9, 2016

Uh oh!

crosbymichael commented Mar 9, 2016

Uh oh!

mrunalpMar 9, 2016

Choose a reason for hiding this comment

Uh oh!

vbattsMar 9, 2016

Choose a reason for hiding this comment

Uh oh!

mrunalp commented Mar 9, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants