config: reformat and add optional/required #333

philips · 2016-09-21T02:35:39Z

Reformat this file to look more like manifest.md and add
OPTIONAL/REQUIRED tags on all fields.

Fixes #251

Reformat this file to look more like manifest.md and add OPTIONAL/REQUIRED tags on all fields. Fixes opencontainers#251 Signed-off-by: Brandon Philips <[email protected]>

Caught by @coolljt072 and rebased here in markdown format. Signed-off-by: Brandon Philips <[email protected]>

coolljt0725 · 2016-09-21T03:36:58Z

config.md

+     A set of directories which should be created as data volumes in a container running this image. This field MAY be "null".
+     If a file or folder exists within the image with the same path as a data volume, that file or folder is replaced with the data volume and is never merged. **NOTE:** This JSON structure value is unusual because it is a direct JSON serialization of the Go type <code>map[string]struct{}</code> and is represented in JSON as an object mapping its keys to an empty object.
+
+   - **WorkingDir** *string*, REQUIRED


why the config is OPTIONAL, but WorkingDir is REQUIRED? I think all the filed in config is OPTIONAL :)

Does this mean if config is not null, then the WorkingDir is REQUIRED?

coolljt0725 · 2016-09-21T03:41:26Z

config.md

+
+     A custom message set when creating the layer.
+
+  - **empty_layer** *string*


I think this is a Boolean, not a string

wking · 2016-09-21T05:19:34Z

On Tue, Sep 20, 2016 at 08:36:59PM -0700, Lei Jitang wrote:

WorkingDir string, REQUIRED

why the config is OPTIONAL, but WorkingDir is REQUIRED? I think
all the filed in config is OPTIONAL :)

This means that if you set ‘config’ you MUST set ‘config.WorkingDir’.
And it's probably required because the runtime-spec requires
process.cwd 1. I've filed opencontainers/runtime-spec#489 to relax
the process.args requirement, and if that ever gets legs I may file a
follow-up relaxing process.cwd. But until the runtime-spec relaxes
the requirement (possibly never), I think image-spec is best off
reinventing as little of the wheel as possible ;).

wking · 2016-09-21T05:19:59Z

config.md

-</dl>
+- **created** *string*, OPTIONAL
+
+  A ISO-8601 formatted combined date and time at which the image was created.


“A ISO” → “An ISO”.

wking · 2016-09-21T05:22:07Z

config.md

+- **config** *object*, OPTIONAL
+
+  The execution parameters which should be used as a base when running a container using the image.
+  This field can be <code>null</code>, in which case any execution parameters should be specified at creation of the container.


<code> → backticks.

wking · 2016-09-21T05:24:34Z

config.md

+  The execution parameters which should be used as a base when running a container using the image.
+  This field can be <code>null</code>, in which case any execution parameters should be specified at creation of the container.
+
+   - **user** *string*, OPTIONAL


This was previously title-case, and as of 403a1af specs-go/v1/config.go still has:

User string `json:"User"`

wking · 2016-09-21T05:25:28Z

config.md

+
+     The username or UID which the process in the container should run as.
+     This acts as a default value to use when the value is not specified when creating a container.
+     All of the following are valid: `user`, `uid`, `user:group`, `uid:gid`, `uid:group`, `uiser:gid`


uiser:gid → user:gid.

wking · 2016-09-21T05:26:20Z

config.md

+A layer DiffID is a SHA256 digest over the layer's uncompressed tar archive and serialized in the descriptor digest format, e.g., `sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9`.
+Layers must be packed and unpacked reproducibly to avoid changing the layer DiffID, for example by using tar-split to save the tar headers.
+
+NOTE: the DiffID is different than the digest in the manifest list because the manifest digest is taken over the gzipped layer for <code>application/vnd.oci.image.layer.tar+gzip</code> types.


<code> → backticks.

wking · 2016-09-21T05:26:38Z

config.md

+
+   - **MemorySwap** *integer*, OPTIONAL
+
+     Total memory usage (memory + swap); set to <code>-1</code> to disable swap.


<code> → backticks.

wking · 2016-09-21T05:27:53Z

config.md

+     Its keys can be in the format of:
+`port/tcp`, `port/udp`, `port` with the default protocol being `tcp` if not specified.
+     These values act as defaults and are merged with any specified when creating a container.
+     **NOTE:** This JSON structure value is unusual because it is a direct JSON serialization of the Go type <code>map[string]struct{}</code> and is represented in JSON as an object mapping its keys to an empty object.


<code> → backticks.

wking · 2016-09-21T05:28:10Z

config.md

+
+   - **Env** *array of strings*, OPTIONAL
+
+     Entries are in the format of <code>VARNAME="var value"</code>.


<code> → backticks.

wking · 2016-09-21T05:28:33Z

config.md

+   - **Entrypoint** *array of strings*
+
+     A list of arguments to use as the command to execute when the container starts.
+     This value acts as a  default and is replaced by an entrypoint specified when creating a container. This field MAY be "null".


"null" → null?

Also remove the doubled space: “as a default“ → “as a default”. And move “This field MAY be null” to its own line.

wking · 2016-09-21T05:30:31Z

config.md

+
+   - **Volumes** *object*, OPTIONAL
+     A set of directories which should be created as data volumes in a container running this image. This field MAY be "null".
+     If a file or folder exists within the image with the same path as a data volume, that file or folder is replaced with the data volume and is never merged. **NOTE:** This JSON structure value is unusual because it is a direct JSON serialization of the Go type <code>map[string]struct{}</code> and is represented in JSON as an object mapping its keys to an empty object.


<code> → backticks.

wking · 2016-09-21T05:31:17Z

config.md

+
+ Describes the history of each layer.
+ The array is ordered from bottom-most layer to top-most layer.
+ The object has the following fields:


Missing indents?

wking · 2016-09-21T05:32:24Z

config.md

+ The array is ordered from bottom-most layer to top-most layer.
+ The object has the following fields:
+
+  - **created** *string*, OPTIONAL


Four-space indents for sub-lists (opencontainers/runtime-spec#495).

wking · 2016-09-21T05:34:19Z

config.md

+
+  - **empty_layer** *string*
+
+     This field is used to mark if the history item created a filesystem diff, OPTIONAL


“OPTIONAL“ should go on the line with empty_layer, like you do with all the other properties.

coolljt0725 · 2016-09-21T06:22:56Z

config.md

+     Sets the current working directory of the entrypoint process in the container.
+     This value acts as a default and is replaced by a working directory specified when creating a container.
+
+- **rootfs** *object, REQUIRED


*object -> *object*

still lack of a * at the end of object. It looks like
rootfs *object, REQUIRED

coolljt0725 · 2016-09-21T06:29:17Z

config.md

+     If an `Entrypoint` value is not specified, then the first entry of the `Cmd` array should be interpreted as the executable to run.
+
+   - **Volumes** *object*, OPTIONAL
+     A set of directories which should be created as data volumes in a container running this image. This field MAY be "null".


should have a blang line after **Volumes** *object*, OPTIONAL

wking · 2016-09-21T07:00:37Z

config.md

+
+Each image has an associated JSON structure which describes some basic information about the image such as date created, author, and the ID of its parent image as well as execution/runtime configuration like its entrypoint, default arguments, CPU/memory shares, networking, and volumes.
+The JSON structure also references a cryptographic hash of each layer used by the image, and provides history information for those layers.
+This JSON is considered to be immutable, because changing it would change the computed ImageID.


Can you add a new commit carrying #301, which changes ImageID → [ImageID](#imageid)?

wking · 2016-09-21T07:03:12Z

config.md

+
+Each image's ID is given by the SHA256 hash of its configuration JSON.
+It is represented as a hexadecimal encoding of 256 bits, e.g., <code>sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9</code>.
+Since the configuration JSON that gets hashed references hashes of each layer in the image, this formulation of the ImageID makes images content-addresable.


Can you add a new commit carrying #301, which changes configuration JSON → configuration JSON, both on this line and the “Each image's ID…” line two above it?

@coolljt0725

- use correct indentation for sub lists - address some missing OPTIONAL tags - random formatting issues. Thanks @coolljt0725 and @wking for the review. Signed-off-by: Brandon Philips <[email protected]>

philips · 2016-09-21T22:31:40Z

Addressed @coolljt0725 and @wking

wking · 2016-09-22T04:52:33Z

config.md

+
+       An array of layer content hashes (`DiffIDs`), in order from bottom-most to top-most.
+
+- **history** *array of object*, REQUIRED


“array of object” → “array of objects”

Is history OPTIONAL? since all the filed in history is optional

wking · 2016-09-22T04:53:41Z

config.md

+   - **Volumes** *object*, OPTIONAL
+
+     A set of directories which should be created as data volumes in a container running this image. This field MAY be null.
+     If a file or folder exists within the image with the same path as a data volume, that file or folder is replaced with the data volume and is never merged. **NOTE:** This JSON structure value is unusual because it is a direct JSON serialization of the Go type `map[string]struct{}` and is represented in JSON as an object mapping its keys to an empty object.


Split this into separate lines for each sentence.

wking · 2016-09-22T04:56:15Z

config.md

+
+Each image's ID is given by the SHA256 hash of its configuration JSON.
+It is represented as a hexadecimal encoding of 256 bits, e.g., `sha256:a9561eb1b190625c9adb5a9513e72c4dedafc1cb2d4c5236c9a6957ec7dfd5a9`.
+Since the configuration JSON that gets hashed references hashes of each layer in the image, this formulation of the ImageID makes images content-addresable.


This still needs “configuration JSON” links carried from #301.

wking · 2016-09-22T04:57:30Z

config.md

+
+    - **empty_layer** *boolean*, OPTIONAL
+
+       This field is used to mark if the history item created a filesystem diff, OPTIONAL


“, OPTIONAL” → “.” (you say empty_layer is optional two lines above)

wking · 2016-09-22T05:00:04Z

config.md

+
+    - **created** *string*, OPTIONAL
+
+       Creation time, expressed as a ISO-8601 formatted combined date and time


“a ISO” → “an ISO”.

wking · 2016-09-22T05:01:17Z

config.md

+   - **Entrypoint** *array of strings*, OPTIONAL
+
+     A list of arguments to use as the command to execute when the container starts.
+     This value acts as a  default and is replaced by an entrypoint specified when creating a container. This field MAY be null.


Still has a doubled space in “as a default”.

Some duplicate spaces and duplicate words. Signed-off-by: Brandon Philips <[email protected]>

philips · 2016-09-22T19:01:30Z

I think this is g2g @opencontainers/image-spec-maintainers

vbatts · 2016-09-22T19:08:51Z

Pretty huge overhaul.
/me 👀

wking · 2016-09-22T19:13:57Z

On Thu, Sep 22, 2016 at 12:01:30PM -0700, Brandon Philips wrote:

I think this is g2g @opencontainers/image-spec-maintainers

As of 85138c9 there are still a number of unaddressed comments on the
current diff:

Carrying some links from Add some links in config.md #301 [1,2].
Splitting to one sentence per line 3.
Fixing a dangling ‘*object’ 4.
Fixing a missing plural 5.
Making ‘history’ OPTIONAL 6, since everything inside it is OPTIONAL.
Fixing “a ISO” 7.

vbatts · 2016-09-22T19:26:16Z

There are a couple of questions on the choices made for OPTIONAL/REQUIRED

philips · 2016-09-22T21:12:46Z

@vbatts what are the questions?

stevvooe · 2016-09-22T21:51:29Z

config.md

-
-        Here is an example rootfs section:
-
-<pre>"rootfs": {


Are we dropping the examples?

There's a big example at the end which gets verified vs. the JSON Schema. I think that's sufficient.

@stevvooe Right, I came to the same conclusion as @wking. The examples in the middle of the spec weren't being verified and the sub-objects aren't really stand alone anyways.

The other downside of adding them back in is it makes formatting into markdown impossible and I need to go back to HTML (or find some hack with a mix).

Various markdown, and grammar issues summarized here: opencontainers#333 (comment) Thank you to wking and coolljt0725 for the detailed review.

Various markdown, and grammar issues summarized here: opencontainers#333 (comment) Thank you to wking and coolljt0725 for the detailed review. Signed-off-by: Brandon Philips <[email protected]>

philips · 2016-09-27T05:37:45Z

@wking I addressed all but the link from #301 in 974ac79. We can punt that discussion about adding readability links once this material change to the spec lands.

jonboulle · 2016-09-27T16:15:28Z

LGTM. Read over this and seems fine. It'll be much easier to land subsequent changes too.

vbatts · 2016-09-27T16:21:37Z

Thanks for updating it.
LGTM

As suggested [1,2] for opencontainers#333. [1]: https://github.com/opencontainers/image-spec/pull/333/files#r79975924 [2]: opencontainers#333 (comment) Signed-off-by: W. Trevor King <[email protected]>

Originally proposed by Lei Jitang in 28743f6 (Add some links in config.md, 2016-09-18, opencontainers#301) and not carried by opencontainers#333 [1]. [1]: opencontainers#333 (comment) Signed-off-by: W. Trevor King <[email protected]>

Covering all cases turned up by: $ git grep '\*array' runtime-spec does this pretty consistently, and the first such usage appeared in image-spec with f3d0f78 (config: reformat and add optional/required, 2016-09-20, opencontainers#333). Signed-off-by: W. Trevor King <[email protected]>

Covering all cases turned up by: $ git grep '\*array' runtime-spec does this pretty consistently, and the first such usage appeared in image-spec with f3d0f78 (config: reformat and add optional/required, 2016-09-20, opencontainers#333). And before that, c22ca79 (serialization: docker v1 image format media type, 2016-04-04, opencontainers#6) had landed some: <code>array of strings</code> Signed-off-by: W. Trevor King <[email protected]>

Various markdown, and grammar issues summarized here: opencontainers/image-spec#333 (comment) Thank you to wking and coolljt0725 for the detailed review. Signed-off-by: Brandon Philips <[email protected]>