Introducing Solaris in OCI #411

anuthan · 2016-04-29T04:29:43Z

This PR introduces Solaris application container configuration parameters to OCI.

Signed-off-by: Abhijeeth Nuthan abhijeeth.nuthan@oracle.com

vbatts · 2016-04-29T13:53:01Z

nicely. On the whole, LGTM

wking · 2016-04-29T17:49:30Z

config-solaris.md

@@ -0,0 +1,88 @@
+# Solaris Application Container Configuration
+
+Solaris application containers can be configured using the following properties, all of the below properties are the same as given in zonecfg(1M) man page, except milestone. The Solaris specification is entirely optional.


I'm not clear on “entirely optional”. If you're aiming that at runtime authors (e.g. “authors of Linux runtimes can ignore this”), I don't think we need to call it out explicitly (all platform-specific specification can be ignored by runtime targeting other platforms). If it's targeting bundle authors (e.g. “all settings in this section are optional, so even if you're writing a bundle targeting Solaris, you can leave this section out”), then I'd rather that get spelled out in more detail. For example:

solaris (object, optional) Solaris-specific configuration. Even configurations targeting Solaris MAY leave this field unset.

"entirely optional" means as you said, "solaris (object, optional) Solaris-specific configuration. Even configurations targeting Solaris MAY leave this field unset."
Having said that, would it be more appropriate to the the above text in config.md as opposed to config-solaris.md and have a entry for each platform to say if the platform-specific section is required by the said platform.

On Fri, Apr 29, 2016 at 11:06:00AM -0700, Abhijeeth Nuthan wrote:

Having said that, would it be more appropriate to the the above text
in config.md as opposed to config-solaris.md and have a entry for
each platform to say if the platform-specific section is required by
the said platform.

Yeah, putting something more structured in 1 would be good. It
looks like you're softening the existing SHOULD to a MAY. I think
that's also the case for Linux, but I'm not sure, and having each
field listed with our usual:

{key} ({type}, {when-needed}) {description}

would make it more obvious.

Sounds like we need a separate PR to address the restructuring, if required. :)

On Fri, Apr 29, 2016 at 11:28:12AM -0700, Abhijeeth Nuthan wrote:

Sounds like we need a separate PR to address the restructuring, if required. :)

I just filed #414.

vishh · 2016-04-29T19:22:55Z

@anuthan Overall LGTM. Is there an implementation of this Spec that users can play with today?

anuthan · 2016-04-29T20:14:28Z

@vishh : The implementation of this spec is actively being worked on. I cannot comment on the timeline when this would be available. All I can say is coming soon to a Solaris store near you. :)

anuthan · 2016-04-29T20:18:03Z

@vbatts and @vishh : Thanks for the LGTMs

crosbymichael · 2016-05-02T18:02:55Z

specs-go/config.go

+	// Specify a name for the automatically created VNIC datalink.
+	Linkname string `json:"linkname,omitempty"`
+	// Specify the link over which the VNIC will be created.
+	Lowerlink string `json:"lower-link,omitempty"`


These need to be camel case and not include - to be consistent with the rest of the spec.

@crosbymichael : As stated previously, the names of the properties which appear in the config.json , such as capped-cpu, capped-memory, lower-link, etc., have been part of Solaris virtualization for a long time.
We prefer to keep the same name and structure to how it is defined in https://docs.oracle.com/cd/E36784_01/html/E36871/zonecfg-1m.html .
Sorry if that ruins consistency.

crosbymichael · 2016-05-02T18:04:11Z

Looks good overall but all your json fields need to be camel case and not include - because it makes it inconsistent with the spec as a whole.

max-shm-memory to maxShmMemory

mrunalp · 2016-05-02T18:28:51Z

config-solaris.md

+## Network
+
+### Automatic Network (anet)
+anet is specified as an array that is used to setup networking for Solaris application containers. The anet resource represents the automatic creation of a network resource for an application container. When such a container is started, a temporary VNIC(Virtual NIC) is automatically created for the container. The VNIC is deleted when the container is torn down.


One sentence per line.

@mrunalp : done.

philips · 2016-05-04T17:15:34Z

LGTM, I would like @anuthan to add a link or some explanatory text about the lifecycle of networking in Solaris is tied to a particular process as this is quite different than Linux where a majority of the expertise is coming from.

Signed-off-by: Abhijeeth Nuthan <abhijeeth.nuthan@oracle.com>

anuthan · 2016-05-04T19:23:02Z

@philips : I have added text for life-cycle of networking and a pointer to our documentation, which is a man page of our administrative daemon.

@crosbymichael : camel case'd everything.

crosbymichael · 2016-05-04T19:42:20Z

LGTM

vbatts · 2016-05-04T20:17:34Z

LGTM

anuthan · 2016-05-04T20:23:27Z

@vbatts Thanks.

Also thanks to all who reviewed.

Fixup for 7c9daeb (Introducing Solaris in OCI, 2016-04-25, opencontainers#411) along the lines of b373a15 (config: Split platform-specific configuration into its own section, 2016-05-02, opencontainers#414). Signed-off-by: W. Trevor King <wking@tremily.us>

This should have been part of 759ee79 (config: Add platform-specific entry for 'solaris', 2016-05-06, opencontainers#431), since the example has playform.os set to 'linux'. There was some (brief) discussion of this point before the 'solaris' section landed [1], but the "should only be set if" wording landed in parallel via b373a15 (config: Split platform-specific configuration into its own section, 2016-05-02, opencontainers#414), and I'd forgotten to go back and apply that logic to opencontainers#411. Having a full Solaris example would be useful, but I think it should be a separate, Solaris-only example. [1]: opencontainers#411 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

This should have been part of 759ee79 (config: Add platform-specific entry for 'solaris', 2016-05-06, opencontainers#431), since the example has platform.os set to 'linux'. There was some (brief) discussion of this point before the 'solaris' section landed [1], but the "should only be set if" wording landed in parallel via b373a15 (config: Split platform-specific configuration into its own section, 2016-05-02, opencontainers#414), and I'd forgotten to go back and apply that logic to opencontainers#411. Having a full Solaris example would be useful, but I think it should be a separate, Solaris-only example. [1]: opencontainers#411 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

Fixup for 7c9daeb (Introducing Solaris in OCI, 2016-04-25, opencontainers#411) along the lines of b373a15 (config: Split platform-specific configuration into its own section, 2016-05-02, opencontainers#414). Signed-off-by: W. Trevor King <wking@tremily.us>

This should have been part of 759ee79 (config: Add platform-specific entry for 'solaris', 2016-05-06, opencontainers#431), since the example has platform.os set to 'linux'. There was some (brief) discussion of this point before the 'solaris' section landed [1], but the "should only be set if" wording landed in parallel via b373a15 (config: Split platform-specific configuration into its own section, 2016-05-02, opencontainers#414), and I'd forgotten to go back and apply that logic to opencontainers#411. Having a full Solaris example would be useful, but I think it should be a separate, Solaris-only example. [1]: opencontainers#411 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

The note is from 7c9daeb (Introducing Solaris in OCI, 2016-04-25, opencontainers#411), but as I pointed out there [1], this is also true for Linux. 08908d6 (config: Explicit container namespace for uid, gid, and additionalGids, 2016-04-29, opencontainers#412) landed in parallel with more explicit namepacing for these fields, so we no longer need the overly-specific Solaris note. [1]: opencontainers#411 (comment) Signed-off-by: W. Trevor King <wking@tremily.us>

wking reviewed Apr 29, 2016
View reviewed changes

crosbymichael reviewed May 2, 2016
View reviewed changes

mrunalp reviewed May 2, 2016
View reviewed changes

wking mentioned this pull request May 3, 2016

artifacts must be present in a single directory opencontainers/runtime-tools#55

Closed

mrunalp added this to the v0.6.0 milestone May 4, 2016

Introducing Solaris in OCI

7c9daeb

Signed-off-by: Abhijeeth Nuthan <abhijeeth.nuthan@oracle.com>

vbatts merged commit bf58a8f into opencontainers:master May 4, 2016

anuthan mentioned this pull request May 5, 2016

Adding link in README.md to Solaris configuration. #424

Merged

wking mentioned this pull request May 16, 2016

config.md: Remove 'solaris' from full example #436

Merged

wking mentioned this pull request May 24, 2016

*: Remove "bundle" and go straight to config.json #423

Closed

wking mentioned this pull request Jun 27, 2016

Add text about extensions #510

Merged

wking mentioned this pull request Nov 14, 2016

config: Drop Solaris UID/GID note #623

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introducing Solaris in OCI #411

Introducing Solaris in OCI #411

anuthan commented Apr 29, 2016

vbatts commented Apr 29, 2016

wking Apr 29, 2016

anuthan Apr 29, 2016

wking Apr 29, 2016

anuthan Apr 29, 2016

wking Apr 29, 2016

vishh commented Apr 29, 2016

anuthan commented Apr 29, 2016

anuthan commented Apr 29, 2016

crosbymichael May 2, 2016

anuthan May 2, 2016

crosbymichael commented May 2, 2016

mrunalp May 2, 2016

anuthan May 2, 2016 •

edited

Loading

philips commented May 4, 2016

anuthan commented May 4, 2016 •

edited

Loading

crosbymichael commented May 4, 2016

vbatts commented May 4, 2016

anuthan commented May 4, 2016

		@@ -0,0 +1,88 @@
		# Solaris Application Container Configuration

		Solaris application containers can be configured using the following properties, all of the below properties are the same as given in zonecfg(1M) man page, except milestone. The Solaris specification is entirely optional.

Introducing Solaris in OCI #411

Introducing Solaris in OCI #411

Conversation

anuthan commented Apr 29, 2016

vbatts commented Apr 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vishh commented Apr 29, 2016

anuthan commented Apr 29, 2016

anuthan commented Apr 29, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crosbymichael commented May 2, 2016

Choose a reason for hiding this comment

anuthan May 2, 2016 • edited Loading

Choose a reason for hiding this comment

philips commented May 4, 2016

anuthan commented May 4, 2016 • edited Loading

crosbymichael commented May 4, 2016

vbatts commented May 4, 2016

anuthan commented May 4, 2016

anuthan May 2, 2016 •

edited

Loading

anuthan commented May 4, 2016 •

edited

Loading