Enable parsing the OCI linux configuration on non-linux platforms

[laijs]

The first patch changes the user of the Process to a json.RawMessage

The type of user becomes json.RawMessage instead of platform aware
structure, so the Spec becomes a platform-independent structure.

It makes we can parse the config.json as a Spec before knowing
the os, and allows we move the linux related *.go to linux/.

The runtime should parse the user with the corresponding platform
specific User structure.

The second patch enables parsing the OCI linux configuration on non-linux platforms

What does this patch do?

• Make the LinuxSpec be visible by golang on non-linux platforms when “go build”

What did this patch change?

• Add linux/
• Rename config_linux.go to linux/config.go
• Rename config-linux.md to linux/config.md
• Rename runtime_config_linux.go to linux/runtime-config.go
• Rename runtime-config-linux.md to linux/runtime-config.md
• Rename runtime-linux.md -> linux/runtime.md
• Remove the “// +build linux” from linux/config.go
• The package name of linux/*.go is changed to “linux”
• Add import of the “specs” in the linux/*.go

The type LinuxSpec (or say the OCI linux configuration) is a structure
describing some information, there is no executable code bound to the linux,
so the LinuxSpec should be naturally usable by any platform when needed.

But the file names (ended with “_linux.go”) and the build tag disallow
us to do so. The patch changes it. Removing the “_linux” and the build tag
makes go build accept the LinuxSpec on any platform. The linux related files
are also moved to the linux/ to eliminate the future possible name clashing
(include the file names and the type names).

After making the LinuxSpec be visible on non-linux platforms, anyone can
do anything of the following in any platforms. (They are also the useful usecases)

• 1. parse the OCI linux configuration (config.json & runtime.json)
• 2. verify the OCI linux configuration or the bundle
• 3. check or test the OCI linux configuration or even the bundle
• 4. discover/build/deliver linux OCI bundle
• 5. Run the container via the hypervisor-based runtime on non-linux platforms
• etc.)

The 5) is one thing of what we are focusing on. After this patch applied,
anyone can build the runv[1] and run linux container on darwin. (Also after
we update the opencontainers/spec under Godeps/ in our runv.git[1] project.)

[1]: https://github.com/hyperhq/runv

[totherme]

We would also like to be able to build on darwin, so we're interested in this PR.

What happens if someone tries to add a windows-compatible User type? Will it clash with the User definition in config-linux.go? Should we consider renaming User to LinuxUser to avoid such clashes?

@totherme and @glyn

[laijs]

@totherme good question!

I don't want to add LinuxUser. We should move the files to the linux/ directory
to avoid the naming clashes and the window files to windows/.

We can also move them now. @crosbymichael @philips

[glyn]

@laijs sounds good.

[laijs]

The User in the Spec need to be changed to a reference as user string.
and the User in the config-linux.go needs a name for referencing.

It is important for making the Spec be a platform-independent structure.

[mrunalp]

Needs rebase.

[philips]

I would rather just leave the user field out completely instead of creating a mapping string. Thoughts @crosbymichael . Overall I am fine with this change.

[dqminh]

I'm not sure about this. It seems that it's more reasonable to add additional data structure for different plarforms instead of having linux structure available to linux platform. Hypervisor / darwin / windows should be able to add their own data structure here, or we can add additional build tags to linux-specific file if the platform supports linux structure.

[laijs]

@mrunalp
Filename-renames generally causes conflict quickly, so reviewing the attempts/proposal of the PR (or include the non-rename-part) at first is what I beg for. Anyway I will update the patch soon.

[laijs]

@philips The user field is important for the Process. Leaving the user field out may also lead to leave the main-process out from the Spec structure as consequence which is accepted by me. I hope more maintainers give opinions about leaving the main-process out from the Spec structure or other solutions.

[laijs]

@dqminh IMO, the more reasonable choice would be the LinuxSpec, WindowsSpec and DarwinSpec can all live together in any platforms. We can't achieve it by build-tags, only well structuring of the code helps.

I don't quit got the meaning of "instead of having linux structure available to linux platform", this pr makes the linux structure available for any platforms including linux.

specs/
         config.go
         linux/
         windows/

import (
    "opencontainers/specs"
    "opencontainers/specs/linux"
    "opencontainers/specs/windows"
)

func useSpec() {
    // parse the config as specs.Spec
   if result.Platform.OS == "linux" {
       // re-parse the config as linux.Spec and use it
       // after my pr accepted, the name is still linux.LinuxSpec, but it it just another orthogonal problem
   } else if result.Platform.OS == "windows" {
       // re-parse the config as windows.Spec and use it
   }
}

[julz]

One possibility is to borrow from the Golang approach (in exec.Cmd) and use a common shared struct with a system specific field for system-specific info (SysProcAttr, in the case of exec.Cmd).

This would make Spec the root (instead of Spec being nested in LinuxSpec as today). We would then have a nested PlatformAttr field (for example, there may be better names for this) which would be selected based on the platform. This could be stored as a json.RawMessage and parsed based on the selected platform (or, to state it in terms of json rather than go, the schema of this field would be defined by the selected platform). Something like this:

type Spec struct {
 // ...
 Platform string
 PlatformAttr json.RawMessage
}

type LinuxSpec struct {
   // ...
}

Parsing would then look like this rather than needing to reparse the same file:

 var spec specs.Spec
 json.NewDecoder(file).Decode(&spec)

 select spec.Platform {
   case "Linux":
     var linuxSpec linux.Spec
     json.NewDecoder(spec.PlatformAttr).Decode(&linuxSpec)
  default:
    panic("unsupported platform")
 }

If we wanted to support the case of a container which supports multiple platforms (runC, runV for example), PlatformSpec could be a map[string]json.RawMessage (this avoids having to include the same common spec in multiple files in the case of a multi-platform container).

For the user field, we could have a similar platform-specific json.RawMessage PlatformAttr attribute on the ProcessSpec (whose schema would be based on the relevant Platform), as above.

[laijs]

@julz The Spec being the root or being nested in PlatformSpec are the same case when we face the attempt that we want to allow all the PlatformSpecs visible at the same time at any platform. In Both cases we need to move the LinuxSpec to linux/ or add "Linux" prefix to all possible-name-clashing fields.

Using json.RawMessage for the user field is a good suggestion. It avoids the indirectly mapping the platform-specific User. Thanks.

[julz]

@laijs indeed, to be clear I was suggesting Spec being the root to avoid double-parsing the config file and to provide a simple potential path to supporting multiple platforms in a bundle (and because to my eyes it's clearer than platform-specific configs with nested common specs and mirrors the golang convention for exec); this is probably an orthogonal issue which may make more sense as another PR though. I'm totally in favour of moving the platform-specific parts to their own packages so that they're usable on any platform.

[laijs]

My code in previous comments is just an example, it doesn't mean double-parsing is required in most cases.

In many cases, the platform is known (runc build on linux currently "knows" the config.json is for linux platform), so one parsing is enough. Spec being the root will leads to two-steps parsing.

Yes, it is orthogonal issue.

[laijs]

Rebased.
The User is also changed to json.RawMessage.

[laijs]

Ping

@crosbymichael @philips @shykes @LK4D4 @mrunalp

[mrunalp]

It is a platform-specific structure.

[mrunalp]

Added a minor-nit, otherwise looks good. (Needs another rebase.)

[laijs]

@mrunalp updated as you suggested. Thx!

[laijs]

Rebased again, on the top of newest master (the latest merge of #162). Thx!

[vbatts]

@laijs I think we're going to take this conversation a little more broad than getting into this approach. see #166

[laijs]

This pr will be pending (not close) until the user is changed to platform-agnostic.
I will update this pr properly after the "user" is fixed.
I hope and assume the "user" problem be fixed before v0.2.0

[laijs]

#191

[vishh]

Can User be a string?

[wking]

On Wed, Dec 16, 2015 at 10:25:08AM -0800, Vish Kannan wrote:

@@ -21,8 +25,8 @@ type Spec struct {
type Process struct {
// Terminal creates an interactive terminal for the container.
Terminal bool json:"terminal"

• // User specifies user information for the process.
• User User json:"user"
• // User specifies user information for the process. It is a platform-specific structure.
• User json.RawMessage json:"user"

Can User be a string?

User as a string requires /etc/passwd lookups or some such on Linux,
and I don't think we want to require that (although I'm fine with it
as an option). See my suggestions in #191 for an alternative that
allows both name-based and number-based identification 1, but that's
going to be platform-specific.

[mrunalp]

We will handle this as part of unifying the runtime/config json files.

[wking]

On Wed, Jan 13, 2016 at 02:05:21PM -0800, Mrunal Patel wrote:

We will handle this as part of unifying the runtime/config json
files.

Those seem orthogonal to me (e.g. #284 unifies configs but does not
address cross-platform Go parsing). And #284 is enough of a bear to
rebase and review that I'd rather not have it biting off more than it
needs to.