volume/mounts: remove "containerOS" argument from NewParser (LCOW code)

This changes mounts.NewParser() to create a parser for the current operatingsystem,
instead of one specific to a (possibly non-matching, in case of LCOW) OS.

With the OS-specific handling being removed, the "OS" parameter is also removed
from `daemon.verifyContainerSettings()`, and various other container-related
functions.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>

container/container.go

@@ -474,11 +474,7 @@ func (container *Container) ShouldRestart() bool {
 
 // AddMountPointWithVolume adds a new mount point configured with a volume to the container.
 func (container *Container) AddMountPointWithVolume(destination string, vol volume.Volume, rw bool) {
-	operatingSystem := container.OS
-	if operatingSystem == "" {
-		operatingSystem = runtime.GOOS
-	}
-	volumeParser := volumemounts.NewParser(operatingSystem)
+	volumeParser := volumemounts.NewParser()
 	container.MountPoints[destination] = &volumemounts.MountPoint{
 		Type:        mounttypes.TypeVolume,
 		Name:        vol.Name(),

container/container_unix.go

@@ -64,7 +64,7 @@ func (container *Container) BuildHostnameFile() error {
 func (container *Container) NetworkMounts() []Mount {
 	var mounts []Mount
 	shared := container.HostConfig.NetworkMode.IsContainer()
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 	if container.ResolvConfPath != "" {
 		if _, err := os.Stat(container.ResolvConfPath); err != nil {
 			logrus.Warnf("ResolvConfPath set to %q, but can't stat this filename (err = %v); skipping", container.ResolvConfPath, err)
@@ -199,7 +199,7 @@ func (container *Container) UnmountIpcMount() error {
 // IpcMounts returns the list of IPC mounts
 func (container *Container) IpcMounts() []Mount {
 	var mounts []Mount
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 
 	if container.HasMountFor("/dev/shm") {
 		return mounts
@@ -419,7 +419,6 @@ func copyExistingContents(source, destination string) error {
 
 // TmpfsMounts returns the list of tmpfs mounts
 func (container *Container) TmpfsMounts() ([]Mount, error) {
-	parser := volumemounts.NewParser(container.OS)
 	var mounts []Mount
 	for dest, data := range container.HostConfig.Tmpfs {
 		mounts = append(mounts, Mount{
@@ -428,6 +427,7 @@ func (container *Container) TmpfsMounts() ([]Mount, error) {
 			Data:        data,
 		})
 	}
+	parser := volumemounts.NewParser()
 	for dest, mnt := range container.MountPoints {
 		if mnt.Type == mounttypes.TypeTmpfs {
 			data, err := parser.ConvertTmpfsOptions(mnt.Spec.TmpfsOptions, mnt.Spec.ReadOnly)

daemon/archive_unix.go

@@ -12,7 +12,7 @@ import (
 // cannot be configured with a read-only rootfs.
 func checkIfPathIsInAVolume(container *container.Container, absPath string) (bool, error) {
 	var toVolume bool
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 	for _, mnt := range container.MountPoints {
 		if toVolume = parser.HasResource(mnt, absPath); toVolume {
 			if mnt.RW {

daemon/container.go

@@ -3,10 +3,8 @@ package daemon // import "github.com/docker/docker/daemon"
 import (
 	"fmt"
 	"os"
-	"path"
 	"path/filepath"
 	"runtime"
-	"strings"
 	"time"
 
 	containertypes "github.com/docker/docker/api/types/container"
@@ -231,12 +229,12 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *
 
 // verifyContainerSettings performs validation of the hostconfig and config
 // structures.
-func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
+func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
 	// First perform verification of settings common across all platforms.
-	if err = validateContainerConfig(config, platform); err != nil {
+	if err = validateContainerConfig(config); err != nil {
 		return warnings, err
 	}
-	if err := validateHostConfig(hostConfig, platform); err != nil {
+	if err := validateHostConfig(hostConfig); err != nil {
 		return warnings, err
 	}
 
@@ -248,11 +246,11 @@ func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *conta
 	return warnings, err
 }
 
-func validateContainerConfig(config *containertypes.Config, platform string) error {
+func validateContainerConfig(config *containertypes.Config) error {
 	if config == nil {
 		return nil
 	}
-	if err := translateWorkingDir(config, platform); err != nil {
+	if err := translateWorkingDir(config); err != nil {
 		return err
 	}
 	if len(config.StopSignal) > 0 {
@@ -269,7 +267,7 @@ func validateContainerConfig(config *containertypes.Config, platform string) err
 	return validateHealthCheck(config.Healthcheck)
 }
 
-func validateHostConfig(hostConfig *containertypes.HostConfig, platform string) error {
+func validateHostConfig(hostConfig *containertypes.HostConfig) error {
 	if hostConfig == nil {
 		return nil
 	}
@@ -278,7 +276,7 @@ func validateHostConfig(hostConfig *containertypes.HostConfig, platform string)
 		return errors.Errorf("can't create 'AutoRemove' container with restart policy")
 	}
 	// Validate mounts; check if host directories still exist
-	parser := volumemounts.NewParser(platform)
+	parser := volumemounts.NewParser()
 	for _, c := range hostConfig.Mounts {
 		cfg := c
 		if err := parser.ValidateMountConfig(&cfg); err != nil {
@@ -373,23 +371,13 @@ func validateRestartPolicy(policy containertypes.RestartPolicy) error {
 
 // translateWorkingDir translates the working-dir for the target platform,
 // and returns an error if the given path is not an absolute path.
-func translateWorkingDir(config *containertypes.Config, platform string) error {
+func translateWorkingDir(config *containertypes.Config) error {
 	if config.WorkingDir == "" {
 		return nil
 	}
-	wd := config.WorkingDir
-	switch {
-	case runtime.GOOS != platform:
-		// LCOW. Force Unix semantics
-		wd = strings.Replace(wd, string(os.PathSeparator), "/", -1)
-		if !path.IsAbs(wd) {
-			return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
-		}
-	default:
-		wd = filepath.FromSlash(wd) // Ensure in platform semantics
-		if !system.IsAbs(wd) {
-			return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
-		}
+	wd := filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics
+	if !system.IsAbs(wd) {
+		return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
 	}
 	config.WorkingDir = wd
 	return nil

daemon/container_unix_test.go

@@ -38,7 +38,7 @@ func TestContainerWarningHostAndPublishPorts(t *testing.T) {
 			},
 		}
 		d := &Daemon{configStore: cs}
-		wrns, err := d.verifyContainerSettings("", hostConfig, &containertypes.Config{}, false)
+		wrns, err := d.verifyContainerSettings(hostConfig, &containertypes.Config{}, false)
 		assert.NilError(t, err)
 		assert.DeepEqual(t, tc.warnings, wrns)
 	}

daemon/create.go

@@ -16,7 +16,6 @@ import (
 	"github.com/docker/docker/errdefs"
 	"github.com/docker/docker/image"
 	"github.com/docker/docker/pkg/idtools"
-	"github.com/docker/docker/pkg/system"
 	"github.com/docker/docker/runconfig"
 	v1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/opencontainers/selinux/go-selinux"
@@ -61,31 +60,23 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.Container
 		return containertypes.ContainerCreateCreatedBody{}, errdefs.InvalidParameter(errors.New("Config cannot be empty in order to create a container"))
 	}
 
-	os := runtime.GOOS
-	var img *image.Image
-	if opts.params.Config.Image != "" {
-		var err error
-		img, err = daemon.imageService.GetImage(opts.params.Config.Image, opts.params.Platform)
-		if err == nil {
-			os = img.OS
-		}
-	}
-
-	warnings, err := daemon.verifyContainerSettings(os, opts.params.HostConfig, opts.params.Config, false)
+	warnings, err := daemon.verifyContainerSettings(opts.params.HostConfig, opts.params.Config, false)
 	if err != nil {
 		return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, errdefs.InvalidParameter(err)
 	}
 
-	if img != nil && opts.params.Platform == nil {
-		p := platforms.DefaultSpec()
-		imgPlat := v1.Platform{
-			OS:           img.OS,
-			Architecture: img.Architecture,
-			Variant:      img.Variant,
-		}
+	if opts.params.Platform == nil && opts.params.Config.Image != "" {
+		if img, _ := daemon.imageService.GetImage(opts.params.Config.Image, opts.params.Platform); img != nil {
+			p := platforms.DefaultSpec()
+			imgPlat := v1.Platform{
+				OS:           img.OS,
+				Architecture: img.Architecture,
+				Variant:      img.Variant,
+			}
 
-		if !images.OnlyPlatformWithFallback(p).Match(imgPlat) {
-			warnings = append(warnings, fmt.Sprintf("The requested image's platform (%s) does not match the detected host platform (%s) and no specific platform was requested", platforms.Format(imgPlat), platforms.Format(p)))
+			if !images.OnlyPlatformWithFallback(p).Match(imgPlat) {
+				warnings = append(warnings, fmt.Sprintf("The requested image's platform (%s) does not match the detected host platform (%s) and no specific platform was requested", platforms.Format(imgPlat), platforms.Format(p)))
+			}
 		}
 	}
 
@@ -132,21 +123,13 @@ func (daemon *Daemon) create(opts createOpts) (retC *container.Container, retErr
 		}
 		if img.OS != "" {
 			os = img.OS
-		} else {
-			// default to the host OS except on Windows with LCOW
-			if isWindows && system.LCOWSupported() {
-				os = "linux"
-			}
 		}
 		imgID = img.ID()
-
-		if isWindows && img.OS == "linux" && !system.LCOWSupported() {
+		if isWindows && img.OS == "linux" {
 			return nil, errors.New("operating system on which parent image was created is not Windows")
 		}
-	} else {
-		if isWindows {
-			os = "linux" // 'scratch' case.
-		}
+	} else if isWindows {
+		os = "linux" // 'scratch' case.
 	}
 
 	// On WCOW, if are not being invoked by the builder to create this container (where

daemon/create_windows.go

@@ -28,7 +28,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
 		}
 		hostConfig.Isolation = "hyperv"
 	}
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 	for spec := range config.Volumes {
 
 		mp, err := parser.ParseMountRaw(spec, hostConfig.VolumeDriver)

daemon/daemon_linux_test.go

@@ -116,7 +116,7 @@ func TestNotCleanupMounts(t *testing.T) {
 func TestValidateContainerIsolationLinux(t *testing.T) {
 	d := Daemon{}
 
-	_, err := d.verifyContainerSettings("linux", &containertypes.HostConfig{Isolation: containertypes.IsolationHyperV}, nil, false)
+	_, err := d.verifyContainerSettings(&containertypes.HostConfig{Isolation: containertypes.IsolationHyperV}, nil, false)
 	assert.Check(t, is.Error(err, "invalid isolation 'hyperv' on linux"))
 }
 

daemon/daemon_test.go

@@ -305,7 +305,7 @@ func TestMerge(t *testing.T) {
 func TestValidateContainerIsolation(t *testing.T) {
 	d := Daemon{}
 
-	_, err := d.verifyContainerSettings(runtime.GOOS, &containertypes.HostConfig{Isolation: containertypes.Isolation("invalid")}, nil, false)
+	_, err := d.verifyContainerSettings(&containertypes.HostConfig{Isolation: containertypes.Isolation("invalid")}, nil, false)
 	assert.Check(t, is.Error(err, "invalid isolation 'invalid' on "+runtime.GOOS))
 }
 

daemon/daemon_unix.go

@@ -719,7 +719,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.
 		return warnings, fmt.Errorf("Unknown runtime specified %s", hostConfig.Runtime)
 	}
 
-	parser := volumemounts.NewParser(runtime.GOOS)
+	parser := volumemounts.NewParser()
 	for dest := range hostConfig.Tmpfs {
 		if err := parser.ValidateTmpfsMountDestination(dest); err != nil {
 			return warnings, err

daemon/oci_linux.go

@@ -570,7 +570,7 @@ func WithMounts(daemon *Daemon, c *container.Container) coci.SpecOpts {
 		for _, m := range mounts {
 			if m.Source == "tmpfs" {
 				data := m.Data
-				parser := volumemounts.NewParser("linux")
+				parser := volumemounts.NewParser()
 				options := []string{"noexec", "nosuid", "nodev", string(parser.DefaultPropagationMode())}
 				if data != "" {
 					options = append(options, strings.Split(data, ",")...)

daemon/start.go

@@ -81,7 +81,7 @@ func (daemon *Daemon) ContainerStart(name string, hostConfig *containertypes.Hos
 
 	// check if hostConfig is in line with the current system settings.
 	// It may happen cgroups are umounted or the like.
-	if _, err = daemon.verifyContainerSettings(ctr.OS, ctr.HostConfig, nil, false); err != nil {
+	if _, err = daemon.verifyContainerSettings(ctr.HostConfig, nil, false); err != nil {
 		return errdefs.InvalidParameter(err)
 	}
 	// Adapt for old containers in case we have updates in this function and

daemon/update.go

@@ -13,12 +13,7 @@ import (
 func (daemon *Daemon) ContainerUpdate(name string, hostConfig *container.HostConfig) (container.ContainerUpdateOKBody, error) {
 	var warnings []string
 
-	c, err := daemon.GetContainer(name)
-	if err != nil {
-		return container.ContainerUpdateOKBody{Warnings: warnings}, err
-	}
-
-	warnings, err = daemon.verifyContainerSettings(c.OS, hostConfig, nil, true)
+	warnings, err := daemon.verifyContainerSettings(hostConfig, nil, true)
 	if err != nil {
 		return container.ContainerUpdateOKBody{Warnings: warnings}, errdefs.InvalidParameter(err)
 	}

daemon/volumes.go

@@ -62,7 +62,7 @@ func (m mounts) parts(i int) int {
 func (daemon *Daemon) registerMountPoints(container *container.Container, hostConfig *containertypes.HostConfig) (retErr error) {
 	binds := map[string]bool{}
 	mountPoints := map[string]*volumemounts.MountPoint{}
-	parser := volumemounts.NewParser(container.OS)
+	parser := volumemounts.NewParser()
 
 	ctx := context.TODO()
 	defer func() {
@@ -265,8 +265,6 @@ func (daemon *Daemon) backportMountSpec(container *container.Container) {
 	container.Lock()
 	defer container.Unlock()
 
-	parser := volumemounts.NewParser(container.OS)
-
 	maybeUpdate := make(map[string]bool)
 	for _, mp := range container.MountPoints {
 		if mp.Spec.Source != "" && mp.Type != "" {
@@ -283,6 +281,7 @@ func (daemon *Daemon) backportMountSpec(container *container.Container) {
 		mountSpecs[m.Target] = true
 	}
 
+	parser := volumemounts.NewParser()
 	binds := make(map[string]*volumemounts.MountPoint, len(container.HostConfig.Binds))
 	for _, rawSpec := range container.HostConfig.Binds {
 		mp, err := parser.ParseMountRaw(rawSpec, container.HostConfig.VolumeDriver)

daemon/volumes_unit_test.go

@@ -1,7 +1,6 @@
 package daemon // import "github.com/docker/docker/daemon"
 
 import (
-	"runtime"
 	"testing"
 
 	volumemounts "github.com/docker/docker/volume/mounts"
@@ -21,7 +20,7 @@ func TestParseVolumesFrom(t *testing.T) {
 		{"foobar:baz", "", "", true},
 	}
 
-	parser := volumemounts.NewParser(runtime.GOOS)
+	parser := volumemounts.NewParser()
 
 	for _, c := range cases {
 		id, mode, err := parser.ParseVolumesFrom(c.spec)

volume/mounts/parser.go

@@ -7,13 +7,6 @@ import (
 	"github.com/docker/docker/api/types/mount"
 )
 
-const (
-	// OSLinux is the same as runtime.GOOS on linux
-	OSLinux = "linux"
-	// OSWindows is the same as runtime.GOOS on windows
-	OSWindows = "windows"
-)
-
 // ErrVolumeTargetIsRoot is returned when the target destination is root.
 // It's used by both LCOW and Linux parsers.
 var ErrVolumeTargetIsRoot = errors.New("invalid specification: destination can't be '/'")
@@ -40,14 +33,10 @@ type Parser interface {
 	ValidateMountConfig(mt *mount.Mount) error
 }
 
-// NewParser creates a parser for a given container OS, depending on the current host OS (linux on a windows host will resolve to an lcowParser)
-func NewParser(containerOS string) Parser {
-	switch containerOS {
-	case OSWindows:
+// NewParser creates a parser for the current host OS
+func NewParser() Parser {
+	if runtime.GOOS == "windows" {
 		return &windowsParser{}
 	}
-	if runtime.GOOS == OSWindows {
-		return &lcowParser{}
-	}
 	return &linuxParser{}
 }

volume/mounts/parser_test.go

@@ -438,7 +438,7 @@ func TestParseMountSpec(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer os.RemoveAll(testDir)
-	parser := NewParser(runtime.GOOS)
+	parser := NewParser()
 	cases := []c{
 		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath, ReadOnly: true}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath, Propagation: parser.DefaultPropagationMode()}},
 		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: testDestinationPath}, MountPoint{Type: mount.TypeBind, Source: testDir, Destination: testDestinationPath, RW: true, Propagation: parser.DefaultPropagationMode()}},
@@ -519,7 +519,7 @@ func TestParseMountSpecBindWithFileinfoError(t *testing.T) {
 	}
 	m := mount.Mount{Type: mount.TypeBind, Source: p, Target: p}
 
-	parser := NewParser(runtime.GOOS)
+	parser := NewParser()
 
 	_, err := parser.ParseMountSpec(m)
 	assert.ErrorContains(t, err, "some crazy error")

volume/mounts/validate_test.go

@@ -48,7 +48,7 @@ func TestValidateMount(t *testing.T) {
 		{mount.Mount{Type: mount.TypeBind, Source: testDir, Target: "/foo"}, nil},
 		{mount.Mount{Type: "invalid", Target: "/foo"}, errors.New("mount type unknown")},
 	}
-	parser := NewParser(runtime.GOOS)
+	parser := NewParser()
 	for i, x := range cases {
 		err := parser.ValidateMountConfig(&x.input)
 		if err == nil && x.expected == nil {

volume/service/store.go

@@ -6,7 +6,6 @@ import (
 	"net"
 	"os"
 	"path/filepath"
-	"runtime"
 	"sync"
 	"time"
 
@@ -578,8 +577,7 @@ func (s *VolumeStore) create(ctx context.Context, name, driverName string, opts,
 	// Validate the name in a platform-specific manner
 
 	// volume name validation is specific to the host os and not on container image
-	// windows/lcow should have an equivalent volumename validation logic so we create a parser for current host OS
-	parser := volumemounts.NewParser(runtime.GOOS)
+	parser := volumemounts.NewParser()
 	err := parser.ValidateVolumeName(name)
 	if err != nil {
 		return nil, false, err