Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Move User struct to spec.go #96

Closed
wants to merge 1 commit into from
Closed

Move User struct to spec.go #96

wants to merge 1 commit into from

Conversation

gao-feng
Copy link
Contributor

@gao-feng gao-feng commented Aug 6, 2015

Otherwise User struct will not be found on other platform

Signed-off-by: Gao feng feng@hyper.sh

Move User struct to spec.go
7357ab1 
Otherwise User struct will not be found on other platform

Signed-off-by: Gao feng <feng@hyper.sh>
@lizf-os
Copy link
Contributor

lizf-os commented Aug 6, 2015

Why don't you add spec_xxx.go to support other platforms? I don't think this User struct is applicable for Windows.

@gao-feng
Copy link
Contributor Author

gao-feng commented Aug 6, 2015

@lizf-os Process in spec.go references to User.

I'm working on running runv https://github.com/hyperhq/runv on osx, runv almostly only uses the Spec struct, I don't know the definition of User struct should be on other platforms and if there should be other structures. Does opencontainers have plan to define OCF Spec on other platforms?

@dqminh
Copy link
Contributor

dqminh commented Aug 6, 2015

I think it's better to have different user definition for different platforms. I think it's enough to only have user_unix and user_windows, but conflating the unix default to spec.go is not a good idea

@mrunalp
Copy link
Contributor

mrunalp commented Aug 6, 2015

@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.

@mrunalp
Copy link
Contributor

mrunalp commented Aug 13, 2015

Closing this for now. We will reopen it in the future. Thanks!

@mrunalp mrunalp closed this Aug 13, 2015
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jun 5, 2016
@wking
config: Make 'process.args' optional
bf042d9 
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: <20160114233625.GN6362@odin.tremily.us>
[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 <wking@tremily.us>
@wking wking mentioned this pull request Jun 5, 2016
Closed
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jun 5, 2016
@wking
config: Make 'process.args' optional
5ca74df 
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: <20160114233625.GN6362@odin.tremily.us>
[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 <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Jun 5, 2016
@wking
config: Explicitly list process.user as required
f025239 
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 <wking@tremily.us>
wking added a commit to wking/opencontainer-runtime-spec that referenced this pull request Nov 11, 2016
@wking
config: Make 'process.args' optional
64bbabc 
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: <20160114233625.GN6362@odin.tremily.us>
[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 <wking@tremily.us>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@gao-feng @lizf-os @dqminh @mrunalp