Merge pull request #16779 from Microsoft/10662-execdrivercommand

Windows: Refactor execdriver.Command

daemon/container_unix.go

@@ -329,30 +329,32 @@ func populateCommand(c *Container, env []string) error {
 	uidMap, gidMap := c.daemon.GetUIDGIDMaps()
 
 	c.command = &execdriver.Command{
-		ID:                 c.ID,
-		Rootfs:             c.rootfsPath(),
-		ReadonlyRootfs:     c.hostConfig.ReadonlyRootfs,
-		InitPath:           "/.dockerinit",
-		WorkingDir:         c.Config.WorkingDir,
-		Network:            en,
-		Ipc:                ipc,
-		UIDMapping:         uidMap,
-		GIDMapping:         gidMap,
-		RemappedRoot:       remappedRoot,
-		Pid:                pid,
-		UTS:                uts,
-		Resources:          resources,
+		CommonCommand: execdriver.CommonCommand{
+			ID:            c.ID,
+			InitPath:      "/.dockerinit",
+			MountLabel:    c.getMountLabel(),
+			Network:       en,
+			ProcessConfig: processConfig,
+			ProcessLabel:  c.getProcessLabel(),
+			Rootfs:        c.rootfsPath(),
+			Resources:     resources,
+			WorkingDir:    c.Config.WorkingDir,
+		},
 		AllowedDevices:     allowedDevices,
+		AppArmorProfile:    c.AppArmorProfile,
 		AutoCreatedDevices: autoCreatedDevices,
 		CapAdd:             c.hostConfig.CapAdd.Slice(),
 		CapDrop:            c.hostConfig.CapDrop.Slice(),
+		CgroupParent:       c.hostConfig.CgroupParent,
+		GIDMapping:         gidMap,
 		GroupAdd:           c.hostConfig.GroupAdd,
-		ProcessConfig:      processConfig,
-		ProcessLabel:       c.getProcessLabel(),
-		MountLabel:         c.getMountLabel(),
+		Ipc:                ipc,
 		LxcConfig:          lxcConfig,
-		AppArmorProfile:    c.AppArmorProfile,
-		CgroupParent:       c.hostConfig.CgroupParent,
+		Pid:                pid,
+		ReadonlyRootfs:     c.hostConfig.ReadonlyRootfs,
+		RemappedRoot:       remappedRoot,
+		UIDMapping:         uidMap,
+		UTS:                uts,
 	}
 
 	return nil

daemon/container_windows.go

@@ -80,11 +80,6 @@ func populateCommand(c *Container, env []string) error {
 		return derr.ErrorCodeInvalidNetworkMode.WithArgs(c.hostConfig.NetworkMode)
 	}
 
-	pid := &execdriver.Pid{}
-
-	// TODO Windows. This can probably be factored out.
-	pid.HostPid = c.hostConfig.PidMode.IsHost()
-
 	// TODO Windows. More resource controls to be implemented later.
 	resources := &execdriver.Resources{
 		CommonResources: execdriver.CommonResources{
@@ -126,26 +121,23 @@ func populateCommand(c *Container, env []string) error {
 	}
 	layerFolder := m["dir"]
 
-	// TODO Windows: Factor out remainder of unused fields.
 	c.command = &execdriver.Command{
-		ID:             c.ID,
-		Rootfs:         c.rootfsPath(),
-		ReadonlyRootfs: c.hostConfig.ReadonlyRootfs,
-		InitPath:       "/.dockerinit",
-		WorkingDir:     c.Config.WorkingDir,
-		Network:        en,
-		Pid:            pid,
-		Resources:      resources,
-		CapAdd:         c.hostConfig.CapAdd.Slice(),
-		CapDrop:        c.hostConfig.CapDrop.Slice(),
-		ProcessConfig:  processConfig,
-		ProcessLabel:   c.getProcessLabel(),
-		MountLabel:     c.getMountLabel(),
-		FirstStart:     !c.HasBeenStartedBefore,
-		LayerFolder:    layerFolder,
-		LayerPaths:     layerPaths,
-		Hostname:       c.Config.Hostname,
-		Isolated:       c.hostConfig.Isolation.IsHyperV(),
+		CommonCommand: execdriver.CommonCommand{
+			ID:            c.ID,
+			Rootfs:        c.rootfsPath(),
+			InitPath:      "/.dockerinit",
+			WorkingDir:    c.Config.WorkingDir,
+			Network:       en,
+			MountLabel:    c.getMountLabel(),
+			Resources:     resources,
+			ProcessConfig: processConfig,
+			ProcessLabel:  c.getProcessLabel(),
+		},
+		FirstStart:  !c.HasBeenStartedBefore,
+		LayerFolder: layerFolder,
+		LayerPaths:  layerPaths,
+		Hostname:    c.Config.Hostname,
+		Isolated:    c.hostConfig.Isolation.IsHyperV(),
 	}
 
 	return nil

daemon/daemon.go

@@ -220,7 +220,9 @@ func (daemon *Daemon) Register(container *Container) error {
 		container.setStoppedLocking(&execdriver.ExitStatus{ExitCode: 137})
 		// use the current driver and ensure that the container is dead x.x
 		cmd := &execdriver.Command{
-			ID: container.ID,
+			CommonCommand: execdriver.CommonCommand{
+				ID: container.ID,
+			},
 		}
 		daemon.execDriver.Terminate(cmd)
 

daemon/execdriver/driver.go

@@ -6,9 +6,7 @@ import (
 	"os/exec"
 	"time"
 
-	"github.com/docker/docker/pkg/idtools"
 	"github.com/opencontainers/runc/libcontainer"
-	"github.com/opencontainers/runc/libcontainer/configs"
 )
 
 // Context is a generic key value pair that allows
@@ -106,36 +104,6 @@ type Driver interface {
 	SupportsHooks() bool
 }
 
-// Ipc settings of the container
-// It is for IPC namespace setting. Usually different containers
-// have their own IPC namespace, however this specifies to use
-// an existing IPC namespace.
-// You can join the host's or a container's IPC namespace.
-type Ipc struct {
-	ContainerID string `json:"container_id"` // id of the container to join ipc.
-	HostIpc     bool   `json:"host_ipc"`
-}
-
-// Pid settings of the container
-// It is for PID namespace setting. Usually different containers
-// have their own PID namespace, however this specifies to use
-// an existing PID namespace.
-// Joining the host's PID namespace is currently the only supported
-// option.
-type Pid struct {
-	HostPid bool `json:"host_pid"`
-}
-
-// UTS settings of the container
-// It is for UTS namespace setting. Usually different containers
-// have their own UTS namespace, however this specifies to use
-// an existing UTS namespace.
-// Joining the host's UTS namespace is currently the only supported
-// option.
-type UTS struct {
-	HostUTS bool `json:"host_uts"`
-}
-
 // CommonResources contains the resource configs for a driver that are
 // common across platforms.
 type CommonResources struct {
@@ -169,46 +137,23 @@ type ProcessConfig struct {
 	Tty         bool     `json:"tty"`
 	Entrypoint  string   `json:"entrypoint"`
 	Arguments   []string `json:"arguments"`
-	Terminal    Terminal `json:"-"` // standard or tty terminal
-	Console     string   `json:"-"` // dev/console path
-	ConsoleSize [2]int   `json:"-"` // h,w of initial console size
+	Terminal    Terminal `json:"-"` // standard or tty terminal (Unix)
+	Console     string   `json:"-"` // dev/console path (Unix)
+	ConsoleSize [2]int   `json:"-"` // h,w of initial console size (Windows)
 }
 
-// Command wraps an os/exec.Cmd to add more metadata
-//
-// TODO Windows: Factor out unused fields such as LxcConfig, AppArmorProfile,
-// and CgroupParent.
-type Command struct {
-	ID                 string            `json:"id"`
-	Rootfs             string            `json:"rootfs"` // root fs of the container
-	ReadonlyRootfs     bool              `json:"readonly_rootfs"`
-	InitPath           string            `json:"initpath"` // dockerinit
-	WorkingDir         string            `json:"working_dir"`
-	ConfigPath         string            `json:"config_path"` // this should be able to be removed when the lxc template is moved into the driver
-	Network            *Network          `json:"network"`
-	Ipc                *Ipc              `json:"ipc"`
-	Pid                *Pid              `json:"pid"`
-	UTS                *UTS              `json:"uts"`
-	RemappedRoot       *User             `json:"remap_root"`
-	UIDMapping         []idtools.IDMap   `json:"uidmapping"`
-	GIDMapping         []idtools.IDMap   `json:"gidmapping"`
-	Resources          *Resources        `json:"resources"`
-	Mounts             []Mount           `json:"mounts"`
-	AllowedDevices     []*configs.Device `json:"allowed_devices"`
-	AutoCreatedDevices []*configs.Device `json:"autocreated_devices"`
-	CapAdd             []string          `json:"cap_add"`
-	CapDrop            []string          `json:"cap_drop"`
-	GroupAdd           []string          `json:"group_add"`
-	ContainerPid       int               `json:"container_pid"`  // the pid for the process inside a container
-	ProcessConfig      ProcessConfig     `json:"process_config"` // Describes the init process of the container.
-	ProcessLabel       string            `json:"process_label"`
-	MountLabel         string            `json:"mount_label"`
-	LxcConfig          []string          `json:"lxc_config"`
-	AppArmorProfile    string            `json:"apparmor_profile"`
-	CgroupParent       string            `json:"cgroup_parent"` // The parent cgroup for this command.
-	FirstStart         bool              `json:"first_start"`
-	LayerPaths         []string          `json:"layer_paths"` // Windows needs to know the layer paths and folder for a command
-	LayerFolder        string            `json:"layer_folder"`
-	Hostname           string            `json:"hostname"` // Windows sets the hostname in the execdriver
-	Isolated           bool              `json:"isolated"` // Windows: Isolated is a Hyper-V container rather than Windows Server Container
+// CommonCommand is the common platform agnostic part of the Command structure
+// which wraps an os/exec.Cmd to add more metadata
+type CommonCommand struct {
+	ContainerPid  int           `json:"container_pid"` // the pid for the process inside a container
+	ID            string        `json:"id"`
+	InitPath      string        `json:"initpath"`    // dockerinit
+	MountLabel    string        `json:"mount_label"` // TODO Windows. More involved, but can be factored out
+	Mounts        []Mount       `json:"mounts"`
+	Network       *Network      `json:"network"`
+	ProcessConfig ProcessConfig `json:"process_config"` // Describes the init process of the container.
+	ProcessLabel  string        `json:"process_label"`  // TODO Windows. More involved, but can be factored out
+	Resources     *Resources    `json:"resources"`
+	Rootfs        string        `json:"rootfs"` // root fs of the container
+	WorkingDir    string        `json:"working_dir"`
 }

daemon/execdriver/driver_unix.go

@@ -12,6 +12,7 @@ import (
 	"time"
 
 	"github.com/docker/docker/daemon/execdriver/native/template"
+	"github.com/docker/docker/pkg/idtools"
 	"github.com/docker/docker/pkg/mount"
 	"github.com/docker/docker/pkg/ulimit"
 	"github.com/opencontainers/runc/libcontainer"
@@ -46,6 +47,36 @@ type Resources struct {
 	MemorySwappiness int64            `json:"memory_swappiness"`
 }
 
+// Ipc settings of the container
+// It is for IPC namespace setting. Usually different containers
+// have their own IPC namespace, however this specifies to use
+// an existing IPC namespace.
+// You can join the host's or a container's IPC namespace.
+type Ipc struct {
+	ContainerID string `json:"container_id"` // id of the container to join ipc.
+	HostIpc     bool   `json:"host_ipc"`
+}
+
+// Pid settings of the container
+// It is for PID namespace setting. Usually different containers
+// have their own PID namespace, however this specifies to use
+// an existing PID namespace.
+// Joining the host's PID namespace is currently the only supported
+// option.
+type Pid struct {
+	HostPid bool `json:"host_pid"`
+}
+
+// UTS settings of the container
+// It is for UTS namespace setting. Usually different containers
+// have their own UTS namespace, however this specifies to use
+// an existing UTS namespace.
+// Joining the host's UTS namespace is currently the only supported
+// option.
+type UTS struct {
+	HostUTS bool `json:"host_uts"`
+}
+
 // Network settings of the container
 type Network struct {
 	Mtu            int    `json:"mtu"`
@@ -54,6 +85,29 @@ type Network struct {
 	HostNetworking bool   `json:"host_networking"`
 }
 
+// Command wraps an os/exec.Cmd to add more metadata
+type Command struct {
+	CommonCommand
+
+	// Fields below here are platform specific
+
+	AllowedDevices     []*configs.Device `json:"allowed_devices"`
+	AppArmorProfile    string            `json:"apparmor_profile"`
+	AutoCreatedDevices []*configs.Device `json:"autocreated_devices"`
+	CapAdd             []string          `json:"cap_add"`
+	CapDrop            []string          `json:"cap_drop"`
+	CgroupParent       string            `json:"cgroup_parent"` // The parent cgroup for this command.
+	GIDMapping         []idtools.IDMap   `json:"gidmapping"`
+	GroupAdd           []string          `json:"group_add"`
+	Ipc                *Ipc              `json:"ipc"`
+	LxcConfig          []string          `json:"lxc_config"`
+	Pid                *Pid              `json:"pid"`
+	ReadonlyRootfs     bool              `json:"readonly_rootfs"`
+	RemappedRoot       *User             `json:"remap_root"`
+	UIDMapping         []idtools.IDMap   `json:"uidmapping"`
+	UTS                *UTS              `json:"uts"`
+}
+
 // InitContainer is the initialization of a container config.
 // It returns the initial configs for a container. It's mostly
 // defined by the default template.

daemon/execdriver/driver_windows.go

@@ -33,3 +33,16 @@ type NetworkInterface struct {
 	// container and the port on the host.
 	PortBindings nat.PortMap `json:"port_bindings"`
 }
+
+// Command wraps an os/exec.Cmd to add more metadata
+type Command struct {
+	CommonCommand
+
+	// Fields below here are platform specific
+
+	FirstStart  bool     `json:"first_start"`  // Optimisation for first boot of Windows
+	Hostname    string   `json:"hostname"`     // Windows sets the hostname in the execdriver
+	LayerFolder string   `json:"layer_folder"` // Layer folder for a command
+	LayerPaths  []string `json:"layer_paths"`  // Layer paths for a command
+	Isolated    bool     `json:"isolated"`     // True if a Hyper-V container
+}

daemon/execdriver/lxc/lxc_template_unit_test.go

@@ -45,19 +45,21 @@ func TestLXCConfig(t *testing.T) {
 		t.Fatal(err)
 	}
 	command := &execdriver.Command{
-		ID: "1",
-		Resources: &execdriver.Resources{
-			MemorySwap: int64(swap),
-			CommonResources: execdriver.CommonResources{
-				Memory:    int64(mem),
-				CPUShares: int64(cpu),
+		CommonCommand: execdriver.CommonCommand{
+			ID: "1",
+			Network: &execdriver.Network{
+				Mtu: 1500,
+			},
+			ProcessConfig: execdriver.ProcessConfig{},
+			Resources: &execdriver.Resources{
+				MemorySwap: int64(swap),
+				CommonResources: execdriver.CommonResources{
+					Memory:    int64(mem),
+					CPUShares: int64(cpu),
+				},
 			},
-		},
-		Network: &execdriver.Network{
-			Mtu: 1500,
 		},
 		AllowedDevices: make([]*configs.Device, 0),
-		ProcessConfig:  execdriver.ProcessConfig{},
 	}
 	p, err := driver.generateLXCConfig(command)
 	if err != nil {
@@ -87,15 +89,17 @@ func TestCustomLxcConfig(t *testing.T) {
 		Privileged: false,
 	}
 	command := &execdriver.Command{
-		ID: "1",
+		CommonCommand: execdriver.CommonCommand{
+			ID: "1",
+			Network: &execdriver.Network{
+				Mtu: 1500,
+			},
+			ProcessConfig: processConfig,
+		},
 		LxcConfig: []string{
 			"lxc.utsname = docker",
 			"lxc.cgroup.cpuset.cpus = 0,1",
 		},
-		Network: &execdriver.Network{
-			Mtu: 1500,
-		},
-		ProcessConfig: processConfig,
 	}
 
 	p, err := driver.generateLXCConfig(command)
@@ -218,16 +222,18 @@ func TestCustomLxcConfigMounts(t *testing.T) {
 		},
 	}
 	command := &execdriver.Command{
-		ID: "1",
+		CommonCommand: execdriver.CommonCommand{
+			ID: "1",
+			Network: &execdriver.Network{
+				Mtu: 1500,
+			},
+			Mounts:        mounts,
+			ProcessConfig: processConfig,
+		},
 		LxcConfig: []string{
 			"lxc.utsname = docker",
 			"lxc.cgroup.cpuset.cpus = 0,1",
 		},
-		Network: &execdriver.Network{
-			Mtu: 1500,
-		},
-		Mounts:        mounts,
-		ProcessConfig: processConfig,
 	}
 
 	p, err := driver.generateLXCConfig(command)
@@ -260,14 +266,16 @@ func TestCustomLxcConfigMisc(t *testing.T) {
 
 	processConfig.Env = []string{"HOSTNAME=testhost"}
 	command := &execdriver.Command{
-		ID: "1",
+		CommonCommand: execdriver.CommonCommand{
+			ID: "1",
+			Network: &execdriver.Network{
+				Mtu: 1500,
+			},
+			ProcessConfig: processConfig,
+		},
 		LxcConfig: []string{
 			"lxc.cgroup.cpuset.cpus = 0,1",
 		},
-		Network: &execdriver.Network{
-			Mtu: 1500,
-		},
-		ProcessConfig:   processConfig,
 		CapAdd:          []string{"net_admin", "syslog"},
 		CapDrop:         []string{"kill", "mknod"},
 		AppArmorProfile: "lxc-container-default-with-nesting",
@@ -311,17 +319,19 @@ func TestCustomLxcConfigMiscOverride(t *testing.T) {
 
 	processConfig.Env = []string{"HOSTNAME=testhost"}
 	command := &execdriver.Command{
-		ID: "1",
+		CommonCommand: execdriver.CommonCommand{
+			ID: "1",
+			Network: &execdriver.Network{
+				Mtu: 1500,
+			},
+			ProcessConfig: processConfig,
+		},
 		LxcConfig: []string{
 			"lxc.cgroup.cpuset.cpus = 0,1",
 			"lxc.network.ipv4 = 172.0.0.1",
 		},
-		Network: &execdriver.Network{
-			Mtu: 1500,
-		},
-		ProcessConfig: processConfig,
-		CapAdd:        []string{"NET_ADMIN", "SYSLOG"},
-		CapDrop:       []string{"KILL", "MKNOD"},
+		CapAdd:  []string{"NET_ADMIN", "SYSLOG"},
+		CapDrop: []string{"KILL", "MKNOD"},
 	}
 
 	p, err := driver.generateLXCConfig(command)

daemon/execdriver/windows/checkoptions.go

@@ -9,21 +9,11 @@ import (
 )
 
 func checkSupportedOptions(c *execdriver.Command) error {
-	// Windows doesn't support read-only root filesystem
-	if c.ReadonlyRootfs {
-		return errors.New("Windows does not support the read-only root filesystem option")
-	}
-
 	// Windows doesn't support username
 	if c.ProcessConfig.User != "" {
 		return errors.New("Windows does not support the username option")
 	}
 
-	// Windows doesn't support custom lxc options
-	if c.LxcConfig != nil {
-		return errors.New("Windows does not support lxc options")
-	}
-
 	// TODO Windows: Validate other fields which Windows doesn't support, factor
 	// out where applicable per platform.
 

runconfig/hostconfig.go

@@ -250,15 +250,15 @@ type HostConfig struct {
 	VolumesFrom       []string              // List of volumes to take from other container
 	Devices           []DeviceMapping       // List of devices to map inside the container
 	NetworkMode       NetworkMode           // Network namespace to use for the container
-	IpcMode           IpcMode               // IPC namespace to use for the container
-	PidMode           PidMode               // PID namespace to use for the container
-	UTSMode           UTSMode               // UTS namespace to use for the container
+	IpcMode           IpcMode               // IPC namespace to use for the container	// Unix specific
+	PidMode           PidMode               // PID namespace to use for the container	// Unix specific
+	UTSMode           UTSMode               // UTS namespace to use for the container	// Unix specific
 	CapAdd            *stringutils.StrSlice // List of kernel capabilities to add to the container
 	CapDrop           *stringutils.StrSlice // List of kernel capabilities to remove from the container
 	GroupAdd          []string              // List of additional groups that the container process will run as
 	RestartPolicy     RestartPolicy         // Restart policy to be used for the container
 	SecurityOpt       []string              // List of string values to customize labels for MLS systems, such as SELinux.
-	ReadonlyRootfs    bool                  // Is the container root filesystem in read-only
+	ReadonlyRootfs    bool                  // Is the container root filesystem in read-only	// Unix specific
 	Ulimits           []*ulimit.Ulimit      // List of ulimits to be set in the container
 	LogConfig         LogConfig             // Configuration of the logs for this container
 	CgroupParent      string                // Parent cgroup.