Json schema and examples by vbatts · Pull Request #370 · opencontainers/runtime-spec

vbatts · 2016-04-06T21:12:22Z

Fixes #344

mrunalp · 2016-04-06T21:13:58Z

config.md

+            {
+                "hostID": 1000,
+                "containerID": 0,
+                "size": 10


32k will be a better example for the mappings.

Several fields needed the correct typing, and updates for recent changes. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts · 2016-04-07T14:14:14Z

updated. PTAL.

wking · 2016-04-07T18:51:01Z

On Thu, Apr 07, 2016 at 07:14:17AM -0700, Vincent Batts wrote:

updated. PTAL.

It looks like fc6670d → 1e2a72d addresses all the points raised so
far, except for the version issue which we aren't sure how to handle
1.

There are still some zero values that don't make sense (e.g. in
linux.resources.cpu.period, but 2 has “The minimum quota allowed for
the quota or period is 1ms.”).

vbatts · 2016-04-07T19:48:06Z

@wking fair. Though it also says that a -1 to quotas indicates no restriction, though we have it as a uint64. That does not line up.

I've updated the example.

mrunalp · 2016-04-08T17:06:08Z

config.md

+            "memory": {
+                "limit": 536870912,
+                "reservation": 0,
+                "swap": 0,


Perhaps make the swap same as the limit?

done

On Fri, Apr 8, 2016 at 1:06 PM, Mrunal Patel notifications@github.com
wrote:

In config.md
#370 (comment)
:

]

},

"pids": {

"limit": 32771

},

"hugepageLimits": [

{

"pageSize": "2MB",

"limit": 9223372036854772000

}

],

"oomScoreAdj": 100,

"memory": {

"limit": 536870912,

"reservation": 0,

"swap": 0,

Perhaps make the swap same as the limit?

—
You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/runtime-spec/pull/370/files/e8ba1d05e1347edc6771a8133a4c729db6df3a50#r59057195

wking · 2016-04-08T18:16:49Z

config.md

+                "swappiness": 0
+            },
+            "cpu": {
+                "shares": 0,


To pick another suspicious example, it looks like 0 here is “never run this if anyone else a share value” (docs here). Maybe follow the kernel docs and use 1024?

mrunalp · 2016-04-11T21:36:15Z

@vbatts Could you address the latest comments from @wking ?

vbatts · 2016-04-11T22:22:15Z

@mrunalp done.
PTAL.

wking · 2016-04-11T22:36:58Z

config-linux.md

    "memory": {
-        "limit": 0,
+        "limit": 536870912,
        "reservation": 0,


This is probably not a good example of a soft-limit ;).

http://vignette3.wikia.nocookie.net/starwars/images/2/21/Stahp.jpg/revision/latest?cb=20140401024542

now I have a question. @mrunalp or @crosbymichael, does this reservation correspond to memory.soft_limit_in_bytes? if so, that can take a string like 1G or 256M. So is this uint64 value just the number of bytes for a soft limit?

AFAIK, it is supposed to be in bytes.

If we were to be pedantic, I'd rename all the fields to include the units. For example, reservation becomes reservationBytes

It is in bytes.

fixed. and add in bytes to the field docs

On Mon, Apr 11, 2016 at 03:52:52PM -0700, Vish Kannan wrote:

@@ -267,9 +267,9 @@ The following parameters can be specified to setup the controller:

"memory": { - "limit": 0, + "limit": 536870912, "reservation": 0,

If we were to be pedantic, I'd rename all the fields to include the
units. For example, reservation becomes reservationBytes

Ugh :p. I think that's what docs are for ;).

I disagree. API should be user-friendly and readable. Having to keep
referring to docs while programming against an API is not a good user
experience. Simple changes like including units in the field names will
save a lot of time in the long run.

On Mon, Apr 11, 2016 at 4:01 PM, W. Trevor King notifications@github.com
wrote:

In config-linux.md
#370 (comment)
:

@@ -267,9 +267,9 @@ The following parameters can be specified to setup the controller:

"memory": { - "limit": 0, + "limit": 536870912, "reservation": 0,

On Mon, Apr 11, 2016 at 03:52:52PM -0700, Vish Kannan wrote: > @@ -267,9
+267,9 @@ The following parameters can be specified to setup the
controller: > > ```json > "memory": { > - "limit": 0, > + "limit":
536870912, > "reservation": 0, If we were to be pedantic, I'd rename all
the fields to include the units. For example,reservationbecomes
`reservationBytes`
Ugh :p. I think that's what docs are for ;).

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
https://github.com/opencontainers/runtime-spec/pull/370/files/c1612af9b80dd828f8fccea3bfca5300281d37b9#r59296588

On Mon, Apr 11, 2016 at 04:08:14PM -0700, Vish Kannan wrote:

@@ -267,9 +267,9 @@ The following parameters can be specified to setup the controller:

"memory": { - "limit": 0, + "limit": 536870912, "reservation": 0,

I disagree. API should be user-friendly and readable. Having to keep
referring to docs while programming against an API is not a good
user experience.

Some ideas don't pack down nicely into a JSON key, so I think not
referring to docs while programming against an API is a bad idea. If
the only interesting thing to say is “this is in bytes”, then reading
the docs won't take very long ;). That said, the kernel devs are with
you on this one (since they chose limit_in_bytes),

wking · 2016-04-11T22:45:09Z

a341686 → c1612af look good to me, although it's still pretty easy to
find apparently-meaningless example values 1. I thought part of the
purpose of this PR was fixing these meaningless examples (see also
#348), but we can obviously push some of that to follow-up PRs /
issues if we don't want to check them now.

mrunalp · 2016-04-11T22:45:32Z

I think we can do cleanups in follow on PRs. This LGTM overall.

mrunalp · 2016-04-11T22:45:41Z

@crosbymichael @vishh PTAL

vishh · 2016-04-11T22:51:34Z

LGTM

* "complete" JSON example * fix a couple of values * fix a missing comma Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

Through 6734c7a (Merge pull request opencontainers#370 from vbatts/json_schema_and_examples, 2016-04-11). The only unlisted changes to master were a brief run with ffjson (opencontainers#343, opencontainers#351), but that was pulled out due to gccgo issues in opencontainers#363. Signed-off-by: W. Trevor King <wking@tremily.us>

To match where they're defined in the JSON Schema [1]. The old location is from d4e7326 (config: JSON examples, 2016-04-06, opencontainers#370), and seems to have been accidental. [1]: https://github.com/opencontainers/runtime-spec/blob/0982071b288ddddc1ae84d21c4bd682c96942f5c/schema/schema-linux.json#L21-L48

To match where they're defined in the JSON Schema [1]. The old location is from d4e7326 (config: JSON examples, 2016-04-06, opencontainers#370), and seems to have been accidental. [1]: https://github.com/opencontainers/runtime-spec/blob/0982071b288ddddc1ae84d21c4bd682c96942f5c/schema/schema-linux.json#L21-L48 Signed-off-by: W. Trevor King <wking@tremily.us>

Through 6734c7a (Merge pull request opencontainers#370 from vbatts/json_schema_and_examples, 2016-04-11). The only unlisted changes to master were a brief run with ffjson (opencontainers#343, opencontainers#351), but that was pulled out due to gccgo issues in opencontainers#363. Signed-off-by: W. Trevor King <wking@tremily.us>

To match where they're defined in the JSON Schema [1]. The old location is from d4e7326 (config: JSON examples, 2016-04-06, opencontainers#370), and seems to have been accidental. [1]: https://github.com/opencontainers/runtime-spec/blob/0982071b288ddddc1ae84d21c4bd682c96942f5c/schema/schema-linux.json#L21-L48 Signed-off-by: W. Trevor King <wking@tremily.us>

mrunalp reviewed Apr 6, 2016
View reviewed changes

config.md Outdated

{

"hostID": 1000,

"containerID": 0,

"size": 10

Copy link

Contributor

mrunalp Apr 6, 2016

Choose a reason for hiding this comment

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

32k will be a better example for the mappings.

vbatts force-pushed the json_schema_and_examples branch from fc6670d to 737f4ff Compare April 7, 2016 02:51

schema: updates and fixes

d118a8f

Several fields needed the correct typing, and updates for recent changes. Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts force-pushed the json_schema_and_examples branch from 737f4ff to 1e2a72d Compare April 7, 2016 14:13

vbatts force-pushed the json_schema_and_examples branch from 1e2a72d to e8ba1d0 Compare April 7, 2016 19:47

mrunalp reviewed Apr 8, 2016
View reviewed changes

vbatts force-pushed the json_schema_and_examples branch from e8ba1d0 to a341686 Compare April 8, 2016 17:42

wking reviewed Apr 8, 2016
View reviewed changes

vbatts force-pushed the json_schema_and_examples branch from a341686 to c1612af Compare April 11, 2016 22:21

wking reviewed Apr 11, 2016
View reviewed changes

config: JSON examples

d4e7326

* "complete" JSON example * fix a couple of values * fix a missing comma Signed-off-by: Vincent Batts <vbatts@hashbangbash.com>

vbatts force-pushed the json_schema_and_examples branch from c1612af to d4e7326 Compare April 11, 2016 22:56

wking mentioned this pull request Apr 11, 2016

cgroup: Add support for memory.kmem.tcp.limit_in_bytes #235

Merged

mrunalp merged commit 6734c7a into opencontainers:master Apr 12, 2016

vbatts deleted the json_schema_and_examples branch April 12, 2016 02:09

wking mentioned this pull request Apr 12, 2016

Bump version for v0.5.0 release #379

Merged

wking mentioned this pull request Apr 20, 2016

config: Move (u|g)idMappings from 'process' to 'linux' #390

Merged

wking mentioned this pull request Sep 19, 2016

Inconsistent size values in example #580

Closed

Conversation

vbatts commented Apr 6, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vbatts commented Apr 7, 2016

Uh oh!

wking commented Apr 7, 2016

Uh oh!

vbatts commented Apr 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mrunalp commented Apr 11, 2016

Uh oh!

vbatts commented Apr 11, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wking commented Apr 11, 2016

Uh oh!

mrunalp commented Apr 11, 2016

Uh oh!

mrunalp commented Apr 11, 2016

Uh oh!

vishh commented Apr 11, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants