Add field to specify whether an isolator is required for execution#225
Add field to specify whether an isolator is required for execution#225b wants to merge 1 commit intoappc:masterfrom
Conversation
|
I think having this is reasonable. Letting the image say what it expects seems reasonable for a number of the isolators. What required means for the resource isolators is more nebulous however. /cc @cdaylward @jonboulle |
|
For things that have a minimum requested allocation, like resource/cpu and resource/memory, it seems that either required is implicit or should be available. For the other currently specified resources, I agree required may not be applicable. |
|
I can't say I'm convinced by this change. "Letting the image say what it expects" is different from the spec enforcing that implementations MUST fail execution if isolators can't be satisfied. When are isolators not a runtime-controlled characteristic? In #197 you wrote:
Do you have examples here? Are you thinking of capabilities? Since right now all the other isolators are just resource, and I do not see why these should be necessarily prescribed in an image. (However I have been meaning to revisit the idea of introducing language regarding "reservations" and "limits" to the resource isolators which might clarify this a bit). |
|
Oops, missed this:
My previous suggestion re: reservations/limit would help here. |
|
gaaah, I see language to this effect has been added to the spec. sorry, still catching up on things while I was out... |
|
Correct, this is a backwards-compatible change. If you leave out the "required" field everything works as it does today. |
|
@cdaylward how are those isolator thoughts coming along? ;-) |
An isolator MAY include a field named "required". If the "required" field is present and has value "true" then an executor MUST either enforce the isolator or refuse to execute the container and return an error. A missing "required" field is treated as a value of "false".