Add support for Windows based containers

[lowenna]

Signed-off-by: John Howard jhoward@microsoft.com

This adds Windows support to the OCI runtime-spec: code, JSON schema and markdown documentation. Note that the support has been added using 'aggressive namespacing', the matching changes of which for Linux and Solaris being currently out for review in #567.

This is a formal follow-on the the previous proof-of-concept PR to add support for Windows, #504

[wking]

Maybe:

specifies the minimum size of the system drive in bytes.

[lowenna]

Agreed. Far more succinct. Will be in next push.

[wking]

Also needs the README bits from #554.

[wking]

Trailing newline.

[lowenna]

Fixed in next push.

[wking]

solaris -> windows.

[lowenna]

Embarrassing copy/paste error! Yes, that one is definitely fixed in the upcoming push.

[lowenna]

@wking - comments addressed. And updated README from #554.

[wking]

The existing spec is not very consistent about this, but I'd like to see an OPTIONAL dropped into this sentence. Maybe:

memory is an OPTIONAL configuration for the container's memory usage.

[lowenna]

No objection at all. Modified memory, CPU, storage, and networking to all match this pattern.

[wking]

You don't seem to define this in the Markdown. If you really want:

{
  …
  "windows": {
    "resources": {
      "memory": {…}
    }
  }
}

instead of:

{
  …
  "windows": {
    "memory": {…}
  }
}

You should drop a resources note in the Markdown.

[lowenna]

Yes, done.

[wking]

On Sat, Sep 17, 2016 at 09:21:03PM -0700, John Howard wrote:

+#### Memory
+
+memory is used to set limits on the container's memory usage.

No objection at all. Modified memory, CPU, storage, and networking
to all match this pattern.

I think we want all-caps OPTIONAL to trigger the RFC 2119 semantics
1. I'll file a separate PR to update the existing instances.

[wking]

Linux has resource limits with soft and hard settings:

The soft limit is the value that the kernel enforces for the corresponding resource. The hard limit acts as a ceiling for the soft limit: an unprivileged process may set only its soft limit to a value in the range from 0 up to the hard limit, and (irreversibly) lower its hard limit. A privileged process (under Linux: one with the CAP_SYS_RESOURCE capability) may make arbitrary changes to either limit value.

So when you say “soft limit”, that's what I'm thinking. However, “reservation” sounds more like “I need at least this much”. Is this setting a minimum amount required by the container? Or an initial limit on the amount that the container can consume?

[lowenna]

Memory management is very different between Linux and Windows. Here, it means a guaranteed minimum amount of memory. I used the existing terminology though. Perhaps incorrectly?

[wking]

I didn't realize we already had that language, and think it needs to be clarified for the Linux case too. Grepping through runC shows that reservation is backed by memory.soft_limit_in_bytes. Kernel docs for that have the following to say about soft limits:

.7. Soft limits

Soft limits allow for greater sharing of memory. The idea behind soft limits
is to allow control groups to use as much of the memory as needed, provided

a. There is no memory contention
b. They do not exceed their hard limit

When the system detects memory contention or low memory, control groups
are pushed back to their soft limits. If the soft limit of each control
group is very high, they are pushed back as much as possible to make
sure that one control group does not starve the others of memory.

Please note that soft limits is a best-effort feature; it comes with
no guarantees, but it does its best to make sure that when memory is
heavily contended for, memory is allocated based on the soft limit
hints/setup. Currently soft limit based reclaim is set up such that
it gets invoked from balance_pgdat (kswapd).

So in your case I think reservation makes a lot of sense for the property name, but you should reroll the description to say something like “guaranteed minimum amount of memory”.

[lowenna]

@wking And final update for now - change optional to OPTIONAL per your latest comment. Didn't realise you meant the capitalisation the previous time.

[wking]

Missing a colon.

[lowenna]

Fixed (all 4 occurances)

[wking]

Looks like an extra level of indents for these two closing braces.

[lowenna]

Fixed. Tab rather than 4 spaces. Blame editor 😇

[wking]

This last line seems overly strict. How about dropping it and using “…the OPTIONAL resources…” in the preceding line?

[wking]

And probably worth a ## Resources before this paragraph so you can link to it specifically after the Windows-specific file grows other types of configurations (which is presumably why you have a resources level at all ;).

[lowenna]

Re wording - I pulled that wording from the Linux spec, or so I thought, so I must have been a little over-keen modifying for the Windows version. So yes, have toned down the verbiage accordingly. Will be in next push.

[lowenna]

Updated to include ## Resources too. Update pushed.

[wking]

With ## Resources above, this one should be an h3.

[lowenna]

Indeed. Have fixed the h1..h4s to be consistent throughout.

[wking]

This is looking pretty good to me. Still needs a JSON Schema, but it may be better to wait on that until the Markdown/Go settle down.

[lowenna]

Now with JSON schema 😄

[wking]

On Mon, Sep 19, 2016 at 01:36:21PM -0700, John Howard wrote:

+You can configure a container's resource limits via the resources field of the Windows configuration.
+Do not specify resources unless required.

Re wording - I pulled that wording from the Linux spec…

Looks like you did ;). I've filed #576 to adjust this language on
Linux.

[wking]

Maybe we want to use defs.json#/definitions/percentPointer backed by something like:

"percent": {
  "type": "integer",
  "minimum": 0,
  "maximum": 100
},
"percentPointer": {
  "oneOf": [
    {
      "$ref": "#/definitions/percent"
    },
    {
      "type": "null"
    }
  ]
},

[lowenna]

Good catch - didn't think of that. Will update.

[lowenna]

Updated in 40bf0b7

[wking]

I'm fine with uint64 here, but am enjoying a mental image of a container that needs 4.3 billion CPUs ;).

[lowenna]

Future proofing :)

[wking]

If we know the cap is 10000, we can use a uint16.

[lowenna]

Fixed to cpuShares type.

[wking]

This probably needs defs-windows.json for CPUShares so we can check the 1–10k range.

[lowenna]

Yup, will update.

[lowenna]

@wking - Updated in d73cf3e

[vishh]

Is limit being over-committed?

[lowenna]

Yes. More information on overall memory management of containers on Windows (which is different between Windows Server and Hyper-V containers) will eventually make it into the documentation on microsoft.com, but for now there's nothing published as such.

[vishh]

I assume the CPU object is essentially a union across three parameters?

[lowenna]

@vishh Well yes, but bearing in mind any or all of them are optional.

[wking]

I don't have much of a preference between cpuShares and CPUShares, but we should pick one and stick to it ;).

[lowenna]

I'll stick with camelCase. Mea culpa

[wking]

The Markdown docs say the minimum is 1.

[lowenna]

And they're right, this is incorrect. Thanks for catching. Updated in 6c66d16

[wking]

d57e63f looks good to me :).

[mrunalp]

optional -> OPTIONAL

[mrunalp]

Can you make similar changes throughout this PR?

[lowenna]

@mrunalp Yes, of course. Done and pushed.

[mrunalp]

Iops --> IO operations or iops?

[lowenna]

@mrunalp Updated to IO operations per seconds.

[mrunalp]

LGTM

[RobDolinMS]

@vbatts Would you please give this a look?

[vbatts]

Good stuff. Thanks @jhowardmsft

LGTM