Move User struct to spec.go #96
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Otherwise User struct will not be found on other platform Signed-off-by: Gao feng <[email protected]>
|
Why don't you add spec_xxx.go to support other platforms? I don't think this User struct is applicable for Windows. |
|
@lizf-os Process in spec.go references to User. I'm working on running runv |
|
I think it's better to have different user definition for different platforms. I think it's enough to only have |
|
@gao-feng Thanks for your PR. However, this was discussed earlier and it made sense to have a separate per platform User struct as it doesn't seem very portable right now. When we have more implementations, we will consider abstracting it and moving it into platform independent section if it makes more sense. |
|
Closing this for now. We will reopen it in the future. Thanks! |
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this pull request
Jun 5, 2016
Since be59415 (Split create and start, 2016-04-01, opencontainers#384), it's possible for a container process to never execute user-specified code (e.g. you can call 'create', 'kill', 'delete' without calling 'start'). For folks who expect to do that, there's no reason to define process.args. The only other process property required for all platforms is 'cwd', but the runtime's idler code isn't specified in sufficient detail for the configuration author to have an opinion about what its working directory should be. On Linux and Solaris, uid and gid are also required. My preferred approach here is to make those optional and define defaults [1,2]: If unset, the runtime will not attempt to manipulate the user ID (e.g. not calling setuid(2) or similar). But the maintainer consensus is that they want those to be explicitly required properties [3,4,5]. With the current spec, one option could be to make process optional (with the idler's working directory unspecified) for OSes besides Linux and Solaris, but the main reason that Windows doesn't have a user property is that we don't know how to specify it [6]. I expect all platforms will have some sort of required user field, which means they'll all have to define 'process'. While I'm indenting the sub-properties, also wrap them to one line per sentence (style.md). [1]: opencontainers#417 (comment) [2]: https://groups.google.com/a/opencontainers.org/forum/#!topic/dev/DWdystx5X3A Subject: Exposing platform defaults Date: Thu, 14 Jan 2016 15:36:26 -0800 Message-ID: <[email protected]> [3]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-04-17.00.log.html#l-44 [4]: opencontainers#417 (comment) [5]: opencontainers#417 (comment) [6]: opencontainers#96 (comment) Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this pull request
Jun 5, 2016
Since be59415 (Split create and start, 2016-04-01, opencontainers#384), it's possible for a container process to never execute user-specified code (e.g. you can call 'create', 'kill', 'delete' without calling 'start'). For folks who expect to do that, there's no reason to define process.args. The only other process property required for all platforms is 'cwd', but the runtime's idler code isn't specified in sufficient detail for the configuration author to have an opinion about what its working directory should be. On Linux and Solaris, 'user' is also required for 'uid' and 'gid'. My preferred approach here is to make those optional and define defaults [1,2]: If unset, the runtime will not attempt to manipulate the user ID (e.g. not calling setuid(2) or similar). But the maintainer consensus is that they want those to be explicitly required properties [3,4,5]. With the current spec, one option could be to make process optional (with the idler's working directory unspecified) for OSes besides Linux and Solaris, but the main reason that Windows doesn't have a user property is that we don't know how to specify it [6]. I expect all platforms will have some sort of required user field, which means they'll all have to define 'process'. While I'm indenting the sub-properties, also wrap them to one line per sentence (style.md). [1]: opencontainers#417 (comment) [2]: https://groups.google.com/a/opencontainers.org/forum/#!topic/dev/DWdystx5X3A Subject: Exposing platform defaults Date: Thu, 14 Jan 2016 15:36:26 -0800 Message-ID: <[email protected]> [3]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-04-17.00.log.html#l-44 [4]: opencontainers#417 (comment) [5]: opencontainers#417 (comment) [6]: opencontainers#96 (comment) Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this pull request
Jun 5, 2016
Linux and Solaris both use the same POSIX-based structure (which I've moved to defs-linux.json). Windows likely needs a string-based structure, but we're punting on that until we have more feedback from the Windows folks [1]. Regardless of whether we have a Windows user structure yet, the maintainer consensus is that the property is required [2,3,4]. [1]: opencontainers#96 (comment) [2]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-04-17.00.log.html#l-44 [3]: opencontainers#417 (comment) [4]: opencontainers#417 (comment) Signed-off-by: W. Trevor King <[email protected]>
wking
added a commit
to wking/opencontainer-runtime-spec
that referenced
this pull request
Nov 11, 2016
Since be59415 (Split create and start, 2016-04-01, opencontainers#384), it's possible for a container process to never execute user-specified code (e.g. you can call 'create', 'kill', 'delete' without calling 'start'). For folks who expect to do that, there's no reason to define process.args. The only other process property required for all platforms is 'cwd', but the runtime's idler code isn't specified in sufficient detail for the configuration author to have an opinion about what its working directory should be. On Linux and Solaris, 'user' is also required for 'uid' and 'gid'. My preferred approach here is to make those optional and define defaults [1,2]: If unset, the runtime will not attempt to manipulate the user ID (e.g. not calling setuid(2) or similar). But the maintainer consensus is that they want those to be explicitly required properties [3,4,5]. With the current spec, one option could be to make process optional (with the idler's working directory unspecified) for OSes besides Linux and Solaris, but the main reason that Windows doesn't have a user property is that we don't know how to specify it [6]. I expect all platforms will have some sort of required user field, which means they'll all have to define 'process'. While I'm indenting the sub-properties, also wrap them to one line per sentence (style.md). [1]: opencontainers#417 (comment) [2]: https://groups.google.com/a/opencontainers.org/forum/#!topic/dev/DWdystx5X3A Subject: Exposing platform defaults Date: Thu, 14 Jan 2016 15:36:26 -0800 Message-ID: <[email protected]> [3]: http://ircbot.wl.linuxfoundation.org/meetings/opencontainers/2016/opencontainers.2016-05-04-17.00.log.html#l-44 [4]: opencontainers#417 (comment) [5]: opencontainers#417 (comment) [6]: opencontainers#96 (comment) Signed-off-by: W. Trevor King <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Otherwise User struct will not be found on other platform
Signed-off-by: Gao feng [email protected]