Windows: Add Hyper-V isolation fields#818
Windows: Add Hyper-V isolation fields#818crosbymichael merged 1 commit intoopencontainers:masterfrom
Conversation
config-windows.md
Outdated
|
|
||
| * **`utilityvmpath`** *(string, OPTIONAL)* - specifies the path to the image used for the utility VM. If not supplied, the runtime will search the container filesystem layers from the bottom-most layer upwards, until it locates "UtilityVM", and default to that path. | ||
|
|
||
| * **`sandboxpath`** *(string)* - specifies the root of the path to the sandbox to be used for the container. |
There was a problem hiding this comment.
@wking Yes, you are correct it is REQUIRED if the Hyper-V structure is supplied. It's not REQUIRED though from a global spec perspective as the Hyper-V structure itself is optional. I took a look at the link, but I'm not entirely sure how I would represent that differently or how you suggest I alter this.
There was a problem hiding this comment.
… but I'm not entirely sure how I would represent that differently or how you suggest I alter this.
I suggest you use:
* **`sandboxpath`** *(string, REQUIRED)* - specifies the root of the path to the sandbox to be used for the container.
The relevant section from the link is:
If an OPTIONAL parent property is an object with a REQUIRED child, the child is only REQUIRED if the parent is set. For example…
and the following example. For an example of this in the current spec, see mounts[].destination, where mounts is OPTIONAL, but any entry in that array MUST include destination.
There was a problem hiding this comment.
Got it, thanks. Updated.
lowenna
force-pushed
the
hyper-v
branch
2 times, most recently
from
9905a6e to
b115ab1
Compare
config-windows.md
Outdated
|
|
||
| ## <a name="configWindowsHyperV" />HyperV | ||
|
|
||
| `hyperv` is an OPTIONAL field of the Windows configuration. If present, the container will be run with Hyper-V isolation. If omitted, the container will be run as a Windows Server container. |
There was a problem hiding this comment.
Do you think it would be appropriate to replace will be with MUST be in these instances?
ala:
`hyperv` is an OPTIONAL field of the Windows configuration.
If present, the container MUST be run with Hyper-V isolation.
If omitted, the container MUST be run as a Windows Server container.(Or another of our RFC-requirement words, if MUST is too strong; https://github.com/opencontainers/runtime-spec/blob/master/spec.md#notational-conventions)
There was a problem hiding this comment.
@tianon Sure, updated. I have to be honest it does feel a little strong, but there's nothing closer.
There was a problem hiding this comment.
Ok, if that feels too strong, maybe relaxing the second half to a SHOULD would be appropriate? ie, if Hyper-V is requested via config, the container either MUST run via Hyper-V, or there must be an error, but otherwise, the behavior can be determined by the runtime if necessary?
There was a problem hiding this comment.
On reflection, MUST is fine.
|
Rebased |
lowenna
force-pushed
the
hyper-v
branch
2 times, most recently
from
c872dc7 to
f362d22
Compare
|
Rebased again. |
|
Also verified this update is correct in the context of the docker engine. |
|
LGTM |
config-windows.md
Outdated
|
|
||
| The following parameters can be specified: | ||
|
|
||
| * **`utilityvmpath`** *(string, OPTIONAL)* - specifies the path to the image used for the utility VM. If not supplied, the runtime will search the container filesystem layers from the bottom-most layer upwards, until it locates "UtilityVM", and default to that path. |
There was a problem hiding this comment.
It would be nice to have an example of this field in the example below.
There was a problem hiding this comment.
@TomSweeneyRedHat Yes, I thought about that, but it was a deliberate omission. The reason is that to give an example of something meaningful (except c:\\path\\to\\utilityvm) is difficult as we don't currently (Windows RS1 and RS2) ship any base image which doesn't include the utility vm image contained within the base image. I expect this to change (in fact will have to) when we support Linux containers on Windows in the RS3 timeframe though, but I still can't at this time provide anything meaningful. Does that make sense?
There was a problem hiding this comment.
@jhowardmsft Thanks for the background info. Your call on this. If I was personally making the decision, I'd put in the sample that you had in your comment, it at least gets the idea into the users mind. I would ask though that once a real world sample can be given here that you loop back and either edit or add it. Cheers!
There was a problem hiding this comment.
How about I update the comment to
...specifies the path to the image used for the utility VM. This would be specified if using a base image which does not contain a utility VM image. If not supplied, the runtime will.... and add the example then? To be fair \\path\\to\\utilityvm is probably about as meaningful an example as I could get regardless 😸
|
Needs rebase now that the other windows PR was just merged 😢 |
|
@dqminh Fully expected, no worries at all. Rebased again. |
Signed-off-by: John Howard <jhoward@microsoft.com>
|
LGTM (redux) |
|
LGTM |
7 participants
Signed-off-by: John Howard jhoward@microsoft.com
Similar to #814, the Windows implementation of the docker daemon still passes a number of fields out of band from the OCI runtime spec. This PR addresses the fields required to support Hyper-V containers by adding them to the runtime spec.