From a5a3b778c05f462b94b0f0e85e12176ac72fe2bd Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 22 Sep 2017 11:39:48 +0100 Subject: [PATCH] More granular merge of Runtime from labels & yaml Previously any Runtime specified in yml would completely override anything from the image label, even if they set distinct fields. This pushes the merging down to the next layer, and in the case of BindNS down two layers. Most of the fields involved needed to become pointers to support this, which required a smattering of other changes to cope. As well as the local test suite this has been put through the linuxkit test suite (as of cc200d296a94). I also tested in the scenario which caused me to file #152. Fixes #152. Signed-off-by: Ian Campbell --- src/moby/config.go | 72 +++++++++++++++++++++++++++++++++++----------- src/moby/image.go | 14 +++++---- 2 files changed, 65 insertions(+), 21 deletions(-) diff --git a/src/moby/config.go b/src/moby/config.go index 7eed658f5..33ab46fa3 100644 --- a/src/moby/config.go +++ b/src/moby/config.go @@ -93,21 +93,21 @@ type Image struct { // Runtime is the type of config processed at runtime, not used to build the OCI spec type Runtime struct { - Mounts []specs.Mount `yaml:"mounts" json:"mounts,omitempty"` - Mkdir []string `yaml:"mkdir" json:"mkdir,omitempty"` - Interfaces []Interface `yaml:"interfaces" json:"interfaces,omitempty"` - BindNS Namespaces `yaml:"bindNS" json:"bindNS,omitempty"` + Mounts *[]specs.Mount `yaml:"mounts" json:"mounts,omitempty"` + Mkdir *[]string `yaml:"mkdir" json:"mkdir,omitempty"` + Interfaces *[]Interface `yaml:"interfaces" json:"interfaces,omitempty"` + BindNS Namespaces `yaml:"bindNS" json:"bindNS,omitempty"` } // Namespaces is the type for configuring paths to bind namespaces type Namespaces struct { - Cgroup string `yaml:"cgroup" json:"cgroup,omitempty"` - Ipc string `yaml:"ipc" json:"ipc,omitempty"` - Mnt string `yaml:"mnt" json:"mnt,omitempty"` - Net string `yaml:"net" json:"net,omitempty"` - Pid string `yaml:"pid" json:"pid,omitempty"` - User string `yaml:"user" json:"user,omitempty"` - Uts string `yaml:"uts" json:"uts,omitempty"` + Cgroup *string `yaml:"cgroup" json:"cgroup,omitempty"` + Ipc *string `yaml:"ipc" json:"ipc,omitempty"` + Mnt *string `yaml:"mnt" json:"mnt,omitempty"` + Net *string `yaml:"net" json:"net,omitempty"` + Pid *string `yaml:"pid" json:"pid,omitempty"` + User *string `yaml:"user" json:"user,omitempty"` + Uts *string `yaml:"uts" json:"uts,omitempty"` } // Interface is the runtime config for network interfaces @@ -422,6 +422,17 @@ func assignInterfaceArray(v1, v2 *[]interface{}) []interface{} { return []interface{}{} } +// assignRuntimeInterfaceArray does ordered overrides from arrays of Interface structs +func assignRuntimeInterfaceArray(v1, v2 *[]Interface) []Interface { + if v2 != nil { + return *v2 + } + if v1 != nil { + return *v1 + } + return []Interface{} +} + // assignStrings does ordered overrides from JSON string array pointers func assignStrings(v1, v2 *[]string) []string { if v2 != nil { @@ -477,6 +488,18 @@ func assignString(v1, v2 *string) string { return "" } +// assignString does ordered overrides from JSON string pointers +func assignStringPtr(v1, v2 *string) *string { + if v2 != nil { + return v2 + } + if v1 != nil { + return v1 + } + s := "" + return &s +} + // assignMappings does ordered overrides from UID, GID maps func assignMappings(v1, v2 *[]specs.LinuxIDMapping) []specs.LinuxIDMapping { if v2 != nil { @@ -501,13 +524,30 @@ func assignResources(v1, v2 *specs.LinuxResources) specs.LinuxResources { // assignRuntime does ordered overrides from Runtime func assignRuntime(v1, v2 *Runtime) Runtime { - if v2 != nil { - return *v2 + if v1 == nil { + v1 = &Runtime{} } - if v1 != nil { - return *v1 + if v2 == nil { + v2 = &Runtime{} } - return Runtime{} + runtimeMounts := assignBinds(v1.Mounts, v2.Mounts) + runtimeMkdir := assignStrings(v1.Mkdir, v2.Mkdir) + runtimeInterfaces := assignRuntimeInterfaceArray(v1.Interfaces, v2.Interfaces) + runtime := Runtime{ + Mounts: &runtimeMounts, + Mkdir: &runtimeMkdir, + Interfaces: &runtimeInterfaces, + BindNS: Namespaces{ + Cgroup: assignStringPtr(v1.BindNS.Cgroup, v2.BindNS.Cgroup), + Ipc: assignStringPtr(v1.BindNS.Ipc, v2.BindNS.Ipc), + Mnt: assignStringPtr(v1.BindNS.Mnt, v2.BindNS.Mnt), + Net: assignStringPtr(v1.BindNS.Net, v2.BindNS.Net), + Pid: assignStringPtr(v1.BindNS.Pid, v2.BindNS.Pid), + User: assignStringPtr(v1.BindNS.User, v2.BindNS.User), + Uts: assignStringPtr(v1.BindNS.Uts, v2.BindNS.Uts), + }, + } + return runtime } // assignStringEmpty does ordered overrides if strings are empty, for diff --git a/src/moby/image.go b/src/moby/image.go index d6af09d69..1b7451625 100644 --- a/src/moby/image.go +++ b/src/moby/image.go @@ -246,11 +246,14 @@ func ImageBundle(prefix string, image string, config []byte, runtime Runtime, tw if err := tw.WriteHeader(hdr); err != nil { return err } - runtime.Mounts = append(runtime.Mounts, specs.Mount{Source: "tmpfs", Type: "tmpfs", Destination: "/" + tmp}) - // remount private as nothing else should see the temporary layers - runtime.Mounts = append(runtime.Mounts, specs.Mount{Destination: "/" + tmp, Options: []string{"remount", "private"}}) overlayOptions := []string{"lowerdir=/" + root, "upperdir=/" + path.Join(tmp, "upper"), "workdir=/" + path.Join(tmp, "work")} - runtime.Mounts = append(runtime.Mounts, specs.Mount{Source: "overlay", Type: "overlay", Destination: "/" + path.Join(prefix, "rootfs"), Options: overlayOptions}) + runtimeMounts := append(*runtime.Mounts, + specs.Mount{Source: "tmpfs", Type: "tmpfs", Destination: "/" + tmp}, + // remount private as nothing else should see the temporary layers + specs.Mount{Destination: "/" + tmp, Options: []string{"remount", "private"}}, + specs.Mount{Source: "overlay", Type: "overlay", Destination: "/" + path.Join(prefix, "rootfs"), Options: overlayOptions}, + ) + runtime.Mounts = &runtimeMounts } else { if foundElsewhere { // we need to make the mountpoint at rootfs @@ -264,7 +267,8 @@ func ImageBundle(prefix string, image string, config []byte, runtime Runtime, tw } } // either bind from another location, or bind from self to make sure it is a mountpoint as runc prefers this - runtime.Mounts = append(runtime.Mounts, specs.Mount{Source: "/" + root, Destination: "/" + path.Join(prefix, "rootfs"), Options: []string{"bind"}}) + runtimeMounts := append(*runtime.Mounts, specs.Mount{Source: "/" + root, Destination: "/" + path.Join(prefix, "rootfs"), Options: []string{"bind"}}) + runtime.Mounts = &runtimeMounts } // write the runtime config