Skip to content

Commit 300c11c

Browse files
thaJeztahthaJeztah
committed
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 <[email protected]>
1 parent f3d08d5 commit 300c11c

19 files changed

Lines changed: 49 additions & 102 deletions

‎container/container.go‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -474,11 +474,7 @@ func (container *Container) ShouldRestart() bool {
474474

475475
// AddMountPointWithVolume adds a new mount point configured with a volume to the container.
476476
func (container *Container) AddMountPointWithVolume(destination string, vol volume.Volume, rw bool) {
477-
operatingSystem := container.OS
478-
if operatingSystem == "" {
479-
operatingSystem = runtime.GOOS
480-
}
481-
volumeParser := volumemounts.NewParser(operatingSystem)
477+
volumeParser := volumemounts.NewParser()
482478
container.MountPoints[destination] = &volumemounts.MountPoint{
483479
Type: mounttypes.TypeVolume,
484480
Name: vol.Name(),

‎container/container_unix.go‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ func (container *Container) BuildHostnameFile() error {
6464
func (container *Container) NetworkMounts() []Mount {
6565
var mounts []Mount
6666
shared := container.HostConfig.NetworkMode.IsContainer()
67-
parser := volumemounts.NewParser(container.OS)
67+
parser := volumemounts.NewParser()
6868
if container.ResolvConfPath != "" {
6969
if _, err := os.Stat(container.ResolvConfPath); err != nil {
7070
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 {
199199
// IpcMounts returns the list of IPC mounts
200200
func (container *Container) IpcMounts() []Mount {
201201
var mounts []Mount
202-
parser := volumemounts.NewParser(container.OS)
202+
parser := volumemounts.NewParser()
203203

204204
if container.HasMountFor("/dev/shm") {
205205
return mounts
@@ -419,7 +419,6 @@ func copyExistingContents(source, destination string) error {
419419

420420
// TmpfsMounts returns the list of tmpfs mounts
421421
func (container *Container) TmpfsMounts() ([]Mount, error) {
422-
parser := volumemounts.NewParser(container.OS)
423422
var mounts []Mount
424423
for dest, data := range container.HostConfig.Tmpfs {
425424
mounts = append(mounts, Mount{
@@ -428,6 +427,7 @@ func (container *Container) TmpfsMounts() ([]Mount, error) {
428427
Data: data,
429428
})
430429
}
430+
parser := volumemounts.NewParser()
431431
for dest, mnt := range container.MountPoints {
432432
if mnt.Type == mounttypes.TypeTmpfs {
433433
data, err := parser.ConvertTmpfsOptions(mnt.Spec.TmpfsOptions, mnt.Spec.ReadOnly)

‎daemon/archive_unix.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import (
1212
// cannot be configured with a read-only rootfs.
1313
func checkIfPathIsInAVolume(container *container.Container, absPath string) (bool, error) {
1414
var toVolume bool
15-
parser := volumemounts.NewParser(container.OS)
15+
parser := volumemounts.NewParser()
1616
for _, mnt := range container.MountPoints {
1717
if toVolume = parser.HasResource(mnt, absPath); toVolume {
1818
if mnt.RW {

‎daemon/container.go‎

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -3,10 +3,8 @@ package daemon // import "github.com/docker/docker/daemon"
33
import (
44
"fmt"
55
"os"
6-
"path"
76
"path/filepath"
87
"runtime"
9-
"strings"
108
"time"
119

1210
containertypes "github.com/docker/docker/api/types/container"
@@ -231,12 +229,12 @@ func (daemon *Daemon) setHostConfig(container *container.Container, hostConfig *
231229

232230
// verifyContainerSettings performs validation of the hostconfig and config
233231
// structures.
234-
func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
232+
func (daemon *Daemon) verifyContainerSettings(hostConfig *containertypes.HostConfig, config *containertypes.Config, update bool) (warnings []string, err error) {
235233
// First perform verification of settings common across all platforms.
236-
if err = validateContainerConfig(config, platform); err != nil {
234+
if err = validateContainerConfig(config); err != nil {
237235
return warnings, err
238236
}
239-
if err := validateHostConfig(hostConfig, platform); err != nil {
237+
if err := validateHostConfig(hostConfig); err != nil {
240238
return warnings, err
241239
}
242240

@@ -248,11 +246,11 @@ func (daemon *Daemon) verifyContainerSettings(platform string, hostConfig *conta
248246
return warnings, err
249247
}
250248

251-
func validateContainerConfig(config *containertypes.Config, platform string) error {
249+
func validateContainerConfig(config *containertypes.Config) error {
252250
if config == nil {
253251
return nil
254252
}
255-
if err := translateWorkingDir(config, platform); err != nil {
253+
if err := translateWorkingDir(config); err != nil {
256254
return err
257255
}
258256
if len(config.StopSignal) > 0 {
@@ -269,7 +267,7 @@ func validateContainerConfig(config *containertypes.Config, platform string) err
269267
return validateHealthCheck(config.Healthcheck)
270268
}
271269

272-
func validateHostConfig(hostConfig *containertypes.HostConfig, platform string) error {
270+
func validateHostConfig(hostConfig *containertypes.HostConfig) error {
273271
if hostConfig == nil {
274272
return nil
275273
}
@@ -278,7 +276,7 @@ func validateHostConfig(hostConfig *containertypes.HostConfig, platform string)
278276
return errors.Errorf("can't create 'AutoRemove' container with restart policy")
279277
}
280278
// Validate mounts; check if host directories still exist
281-
parser := volumemounts.NewParser(platform)
279+
parser := volumemounts.NewParser()
282280
for _, c := range hostConfig.Mounts {
283281
cfg := c
284282
if err := parser.ValidateMountConfig(&cfg); err != nil {
@@ -373,23 +371,13 @@ func validateRestartPolicy(policy containertypes.RestartPolicy) error {
373371

374372
// translateWorkingDir translates the working-dir for the target platform,
375373
// and returns an error if the given path is not an absolute path.
376-
func translateWorkingDir(config *containertypes.Config, platform string) error {
374+
func translateWorkingDir(config *containertypes.Config) error {
377375
if config.WorkingDir == "" {
378376
return nil
379377
}
380-
wd := config.WorkingDir
381-
switch {
382-
case runtime.GOOS != platform:
383-
// LCOW. Force Unix semantics
384-
wd = strings.Replace(wd, string(os.PathSeparator), "/", -1)
385-
if !path.IsAbs(wd) {
386-
return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
387-
}
388-
default:
389-
wd = filepath.FromSlash(wd) // Ensure in platform semantics
390-
if !system.IsAbs(wd) {
391-
return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
392-
}
378+
wd := filepath.FromSlash(config.WorkingDir) // Ensure in platform semantics
379+
if !system.IsAbs(wd) {
380+
return fmt.Errorf("the working directory '%s' is invalid, it needs to be an absolute path", config.WorkingDir)
393381
}
394382
config.WorkingDir = wd
395383
return nil

‎daemon/container_unix_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ func TestContainerWarningHostAndPublishPorts(t *testing.T) {
3838
},
3939
}
4040
d := &Daemon{configStore: cs}
41-
wrns, err := d.verifyContainerSettings("", hostConfig, &containertypes.Config{}, false)
41+
wrns, err := d.verifyContainerSettings(hostConfig, &containertypes.Config{}, false)
4242
assert.NilError(t, err)
4343
assert.DeepEqual(t, tc.warnings, wrns)
4444
}

‎daemon/create.go‎

Lines changed: 15 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"github.com/docker/docker/errdefs"
1717
"github.com/docker/docker/image"
1818
"github.com/docker/docker/pkg/idtools"
19-
"github.com/docker/docker/pkg/system"
2019
"github.com/docker/docker/runconfig"
2120
v1 "github.com/opencontainers/image-spec/specs-go/v1"
2221
"github.com/opencontainers/selinux/go-selinux"
@@ -61,31 +60,23 @@ func (daemon *Daemon) containerCreate(opts createOpts) (containertypes.Container
6160
return containertypes.ContainerCreateCreatedBody{}, errdefs.InvalidParameter(errors.New("Config cannot be empty in order to create a container"))
6261
}
6362

64-
os := runtime.GOOS
65-
var img *image.Image
66-
if opts.params.Config.Image != "" {
67-
var err error
68-
img, err = daemon.imageService.GetImage(opts.params.Config.Image, opts.params.Platform)
69-
if err == nil {
70-
os = img.OS
71-
}
72-
}
73-
74-
warnings, err := daemon.verifyContainerSettings(os, opts.params.HostConfig, opts.params.Config, false)
63+
warnings, err := daemon.verifyContainerSettings(opts.params.HostConfig, opts.params.Config, false)
7564
if err != nil {
7665
return containertypes.ContainerCreateCreatedBody{Warnings: warnings}, errdefs.InvalidParameter(err)
7766
}
7867

79-
if img != nil && opts.params.Platform == nil {
80-
p := platforms.DefaultSpec()
81-
imgPlat := v1.Platform{
82-
OS: img.OS,
83-
Architecture: img.Architecture,
84-
Variant: img.Variant,
85-
}
68+
if opts.params.Platform == nil && opts.params.Config.Image != "" {
69+
if img, _ := daemon.imageService.GetImage(opts.params.Config.Image, opts.params.Platform); img != nil {
70+
p := platforms.DefaultSpec()
71+
imgPlat := v1.Platform{
72+
OS: img.OS,
73+
Architecture: img.Architecture,
74+
Variant: img.Variant,
75+
}
8676

87-
if !images.OnlyPlatformWithFallback(p).Match(imgPlat) {
88-
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)))
77+
if !images.OnlyPlatformWithFallback(p).Match(imgPlat) {
78+
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)))
79+
}
8980
}
9081
}
9182

@@ -132,21 +123,13 @@ func (daemon *Daemon) create(opts createOpts) (retC *container.Container, retErr
132123
}
133124
if img.OS != "" {
134125
os = img.OS
135-
} else {
136-
// default to the host OS except on Windows with LCOW
137-
if isWindows && system.LCOWSupported() {
138-
os = "linux"
139-
}
140126
}
141127
imgID = img.ID()
142-
143-
if isWindows && img.OS == "linux" && !system.LCOWSupported() {
128+
if isWindows && img.OS == "linux" {
144129
return nil, errors.New("operating system on which parent image was created is not Windows")
145130
}
146-
} else {
147-
if isWindows {
148-
os = "linux" // 'scratch' case.
149-
}
131+
} else if isWindows {
132+
os = "linux" // 'scratch' case.
150133
}
151134

152135
// On WCOW, if are not being invoked by the builder to create this container (where

‎daemon/create_windows.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ func (daemon *Daemon) createContainerOSSpecificSettings(container *container.Con
2828
}
2929
hostConfig.Isolation = "hyperv"
3030
}
31-
parser := volumemounts.NewParser(container.OS)
31+
parser := volumemounts.NewParser()
3232
for spec := range config.Volumes {
3333

3434
mp, err := parser.ParseMountRaw(spec, hostConfig.VolumeDriver)

‎daemon/daemon_linux_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ func TestNotCleanupMounts(t *testing.T) {
116116
func TestValidateContainerIsolationLinux(t *testing.T) {
117117
d := Daemon{}
118118

119-
_, err := d.verifyContainerSettings("linux", &containertypes.HostConfig{Isolation: containertypes.IsolationHyperV}, nil, false)
119+
_, err := d.verifyContainerSettings(&containertypes.HostConfig{Isolation: containertypes.IsolationHyperV}, nil, false)
120120
assert.Check(t, is.Error(err, "invalid isolation 'hyperv' on linux"))
121121
}
122122

‎daemon/daemon_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,7 @@ func TestMerge(t *testing.T) {
305305
func TestValidateContainerIsolation(t *testing.T) {
306306
d := Daemon{}
307307

308-
_, err := d.verifyContainerSettings(runtime.GOOS, &containertypes.HostConfig{Isolation: containertypes.Isolation("invalid")}, nil, false)
308+
_, err := d.verifyContainerSettings(&containertypes.HostConfig{Isolation: containertypes.Isolation("invalid")}, nil, false)
309309
assert.Check(t, is.Error(err, "invalid isolation 'invalid' on "+runtime.GOOS))
310310
}
311311

‎daemon/daemon_unix.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -719,7 +719,7 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes.
719719
return warnings, fmt.Errorf("Unknown runtime specified %s", hostConfig.Runtime)
720720
}
721721

722-
parser := volumemounts.NewParser(runtime.GOOS)
722+
parser := volumemounts.NewParser()
723723
for dest := range hostConfig.Tmpfs {
724724
if err := parser.ValidateTmpfsMountDestination(dest); err != nil {
725725
return warnings, err

0 commit comments

Comments
 (0)