-
Notifications
You must be signed in to change notification settings - Fork 142
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
validate: fix platform check #347
validate: fix platform check #347
Conversation
currently, only platform linux, windows and solaris are valid. Signed-off-by: Ma Shimiao <[email protected]>
On Tue, Mar 14, 2017 at 01:03:53AM -0700, Ma Shimiao wrote:
currently, only platform linux, windows and solaris are valid.
I don't read the spec this way. I think all of these OSes are valid
(based on [1]). ‘linux’, ‘windows’, and ‘solaris’ are only special
because they have additional, platform-dependent settings [2]. An
‘android’ config, for example, could still take advantage of all the
cross-platform settings (‘root’, ‘mounts’, most of ‘process’, …).
On the other hand, I doubt there will be any Android (or whatever)
implementations that don't want access to some platform-specific
fields. So maybe the spec should be reworded to limit platform.os to
the three values you list (in which case this PR would become
appropriate) and the current SHOULD around $GOOS should be dropped?
I'm fine with both the restricted and permissive approaches, but would
rather not have this restricted runtime-tools check while runtime-spec
remains permissive.
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0-rc5/config.md#L301
[2]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0-rc5/config.md#L323-L328
|
Refer to https://github.com/opencontainers/runtime-spec/blob/master/spec.md#platforms
The spec just defines three platforms, others should be undefined and should be restricted.
|
On Tue, Mar 14, 2017 at 01:22:27AM -0700, Ma Shimiao wrote:
Refer to https://github.com/opencontainers/runtime-spec/blob/master/spec.md#platforms
The spec just defines three platforms, others should be undefined and should be restricted.
I think that section is outlining platforms for compliance testing
(i.e. “runtime-tools will have the following certification tracks…”).
So there would be no certification track for Android, etc.
The current wording in [1], on the other hand, clearly *does* apply to
valid platform.os values, and is very permissive. It allows *any*
string in a valid config, and only SHOULDs values from $GOOS.
If the intention is to keep platform.os permissive, I think the spec
might want more wording in [2] to make the compliance-testing
distinction more clear. My preferred approach is to call the
compliance-testing tracks “protocols”, but my previous attempts to
land that language were rejected [3]. I'm sure there are plenty of
other ways to clarify that distinction though.
[1]: https://github.com/opencontainers/runtime-spec/blame/v1.0.0-rc5/config.md#L301
[2]: https://github.com/opencontainers/runtime-spec/blob/v1.0.0-rc5/spec.md#platforms
[3]: opencontainers/runtime-spec#570 (review)
|
If according to https://github.com/opencontainers/runtime-spec/blame/v1.0.0-rc5/config.md#L301, platform.os is really very permissive and there is no need to change the code. |
On Tue, Mar 14, 2017 at 02:21:10AM -0700, Ma Shimiao wrote:
But I doubt if support all OS is correct.
I'm fine either way [1]. It's up to the runtime-spec maintainers to
make this call, but I can file an issue about scoping the compliance
testing platform language if you think that would help.
By the way, there is always one question in mind, the spec support
windows OS, but how windows make processs isolated without
namespace?
@RobDolinMS may be able to shed more light on this, but it looks like
Windows does have a namespace-based container technology (“Windows
Server Containers” [2]). I'm not sure which namespaces Windows Server
Containers support. I'm also not sure if any WIP Windows runtime uses
those containers or if it uses Hyper-V containers. And if they do use
Windows Server Containers, I'm not sure how you write a runtime config
to tune which namespaces get used. All interesting things that will
probably need to get sorted out before runtime-tools can
compliance-test Windows runtimes.
[1]: #347 (comment)
[2]: https://docs.microsoft.com/en-us/virtualization/windowscontainers/quick-start/
|
On 03/14/2017 05:31 PM, W. Trevor King wrote:
'm fine either way [1]. It's up to the runtime-spec maintainers to
make this call, but I can file an issue about scoping the compliance
testing platform language if you think that would help.
Thanks, I think it would be a big help.
> By the way, there is always one question in mind, the spec support
> windows OS, but how windows make processs isolated without
> namespace?
@RobDolinMS may be able to shed more light on this, but it looks like
Windows does have a namespace-based container technology (“Windows
Thanks for your information
|
With opencontainers/runtime-spec#850 landed, I expect we can close this PR and, once runtime-spec cuts an rc6, file a new one to remove all of |
As runtime-spec removed platform entry, closing this PR. |
currently, only platform linux, windows and solaris are valid.
Signed-off-by: Ma Shimiao [email protected]