From 7120976d74195a60334c688a061270a4d95f9aeb Mon Sep 17 00:00:00 2001 From: Kir Kolyshkin Date: Tue, 27 Jun 2017 14:58:50 -0700 Subject: [PATCH] Implement none, private, and shareable ipc modes Since the commit d88fe447df0e8 ("Add support for sharing /dev/shm/ and /dev/mqueue between containers") container's /dev/shm is mounted on the host first, then bind-mounted inside the container. This is done that way in order to be able to share this container's IPC namespace (and the /dev/shm mount point) with another container. Unfortunately, this functionality breaks container checkpoint/restore (even if IPC is not shared). Since /dev/shm is an external mount, its contents is not saved by `criu checkpoint`, and so upon restore any application that tries to access data under /dev/shm is severily disappointed (which usually results in a fatal crash). This commit solves the issue by introducing new IPC modes for containers (in addition to 'host' and 'container:ID'). The new modes are: - 'shareable': enables sharing this container's IPC with others (this used to be the implicit default); - 'private': disables sharing this container's IPC. In 'private' mode, container's /dev/shm is truly mounted inside the container, without any bind-mounting from the host, which solves the issue. While at it, let's also implement 'none' mode. The motivation, as eloquently put by Justin Cormack, is: > I wondered a while back about having a none shm mode, as currently it is > not possible to have a totally unwriteable container as there is always > a /dev/shm writeable mount. It is a bit of a niche case (and clearly > should never be allowed to be daemon default) but it would be trivial to > add now so maybe we should... ...so here's yet yet another mode: - 'none': no /dev/shm mount inside the container (though it still has its own private IPC namespace). Now, to ultimately solve the abovementioned checkpoint/restore issue, we'd need to make 'private' the default mode, but unfortunately it breaks the backward compatibility. So, let's make the default container IPC mode per-daemon configurable (with the built-in default set to 'shareable' for now). The default can be changed either via a daemon CLI option (--default-shm-mode) or a daemon.json configuration file parameter of the same name. Note one can only set either 'shareable' or 'private' IPC modes as a daemon default (i.e. in this context 'host', 'container', or 'none' do not make much sense). Some other changes this patch introduces are: 1. A mount for /dev/shm is added to default OCI Linux spec. 2. IpcMode.Valid() is simplified to remove duplicated code that parsed 'container:ID' form. Note the old version used to check that ID does not contain a semicolon -- this is no longer the case (tests are modified accordingly). The motivation is we should either do a proper check for container ID validity, or don't check it at all (since it is checked in other places anyway). I chose the latter. 3. IpcMode.Container() is modified to not return container ID if the mode value does not start with "container:", unifying the check to be the same as in IpcMode.IsContainer(). 3. IPC mode unit tests (runconfig/hostconfig_test.go) are modified to add checks for newly added values. [v2: addressed review at https://github.com/moby/moby/pull/34087#pullrequestreview-51345997] [v3: addressed review at https://github.com/moby/moby/pull/34087#pullrequestreview-53902833] [v4: addressed the case of upgrading from older daemon, in this case container.HostConfig.IpcMode is unset and this is valid] [v5: document old and new IpcMode values in api/swagger.yaml] [v6: add the 'none' mode, changelog entry to docs/api/version-history.md] Signed-off-by: Kir Kolyshkin --- api/swagger.yaml | 12 +++++- api/types/container/host_config.go | 39 ++++++++++-------- cmd/dockerd/config_unix.go | 1 + container/container_unix.go | 62 ++++++++++++++--------------- container/container_windows.go | 5 ++- daemon/config/config.go | 5 +++ daemon/config/config_solaris.go | 5 +++ daemon/config/config_unix.go | 25 ++++++++++++ daemon/config/config_windows.go | 5 +++ daemon/container_operations_unix.go | 55 ++++++++++++++++++------- daemon/daemon_solaris.go | 3 +- daemon/daemon_unix.go | 22 +++++++++- daemon/daemon_windows.go | 3 +- daemon/oci_linux.go | 38 +++++++++++++++--- daemon/reload.go | 4 +- daemon/start.go | 4 +- docs/api/version-history.md | 3 ++ oci/defaults.go | 6 +++ runconfig/hostconfig_test.go | 61 ++++++++++++---------------- 19 files changed, 246 insertions(+), 112 deletions(-) diff --git a/api/swagger.yaml b/api/swagger.yaml index e974a21f84..2ce4240956 100644 --- a/api/swagger.yaml +++ b/api/swagger.yaml @@ -665,7 +665,17 @@ definitions: type: "string" IpcMode: type: "string" - description: "IPC namespace to use for the container." + description: | + IPC sharing mode for the container. Possible values are: + + - `"none"`: own private IPC namespace, with /dev/shm not mounted + - `"private"`: own private IPC namespace + - `"shareable"`: own private IPC namespace, with a possibility to share it with other containers + - `"container:"`: join another (shareable) container's IPC namespace + - `"host"`: use the host system's IPC namespace + + If not specified, daemon default is used, which can either be `"private"` + or `"shareable"`, depending on daemon version and configuration. Cgroup: type: "string" description: "Cgroup to use for the container." diff --git a/api/types/container/host_config.go b/api/types/container/host_config.go index 9fea9eb04b..bb421b3889 100644 --- a/api/types/container/host_config.go +++ b/api/types/container/host_config.go @@ -23,41 +23,46 @@ func (i Isolation) IsDefault() bool { // IpcMode represents the container ipc stack. type IpcMode string -// IsPrivate indicates whether the container uses its private ipc stack. +// IsPrivate indicates whether the container uses its own private ipc namespace which can not be shared. func (n IpcMode) IsPrivate() bool { - return !(n.IsHost() || n.IsContainer()) + return n == "private" } -// IsHost indicates whether the container uses the host's ipc stack. +// IsHost indicates whether the container shares the host's ipc namespace. func (n IpcMode) IsHost() bool { return n == "host" } -// IsContainer indicates whether the container uses a container's ipc stack. +// IsShareable indicates whether the container's ipc namespace can be shared with another container. +func (n IpcMode) IsShareable() bool { + return n == "shareable" +} + +// IsContainer indicates whether the container uses another container's ipc namespace. func (n IpcMode) IsContainer() bool { parts := strings.SplitN(string(n), ":", 2) return len(parts) > 1 && parts[0] == "container" } -// Valid indicates whether the ipc stack is valid. +// IsNone indicates whether container IpcMode is set to "none". +func (n IpcMode) IsNone() bool { + return n == "none" +} + +// IsEmpty indicates whether container IpcMode is empty +func (n IpcMode) IsEmpty() bool { + return n == "" +} + +// Valid indicates whether the ipc mode is valid. func (n IpcMode) Valid() bool { - parts := strings.Split(string(n), ":") - switch mode := parts[0]; mode { - case "", "host": - case "container": - if len(parts) != 2 || parts[1] == "" { - return false - } - default: - return false - } - return true + return n.IsEmpty() || n.IsNone() || n.IsPrivate() || n.IsHost() || n.IsShareable() || n.IsContainer() } // Container returns the name of the container ipc stack is going to be used. func (n IpcMode) Container() string { parts := strings.SplitN(string(n), ":", 2) - if len(parts) > 1 { + if len(parts) > 1 && parts[0] == "container" { return parts[1] } return "" diff --git a/cmd/dockerd/config_unix.go b/cmd/dockerd/config_unix.go index ba37121e96..148fa87459 100644 --- a/cmd/dockerd/config_unix.go +++ b/cmd/dockerd/config_unix.go @@ -47,6 +47,7 @@ func installConfigFlags(conf *config.Config, flags *pflag.FlagSet) { flags.StringVar(&conf.SeccompProfile, "seccomp-profile", "", "Path to seccomp profile") flags.Var(&conf.ShmSize, "default-shm-size", "Default shm size for containers") flags.BoolVar(&conf.NoNewPrivileges, "no-new-privileges", false, "Set no-new-privileges by default for new containers") + flags.StringVar(&conf.IpcMode, "default-ipc-mode", config.DefaultIpcMode, `Default mode for containers ipc ("shareable" | "private")`) attachExperimentalFlags(conf, flags) } diff --git a/container/container_unix.go b/container/container_unix.go index 6a08e562ea..e7a27d212c 100644 --- a/container/container_unix.go +++ b/container/container_unix.go @@ -7,7 +7,6 @@ import ( "io/ioutil" "os" "path/filepath" - "strings" "github.com/docker/docker/api/types" containertypes "github.com/docker/docker/api/types/container" @@ -19,6 +18,7 @@ import ( "github.com/docker/docker/pkg/system" "github.com/docker/docker/volume" "github.com/opencontainers/selinux/go-selinux/label" + "github.com/pkg/errors" "github.com/sirupsen/logrus" "golang.org/x/sys/unix" ) @@ -171,47 +171,47 @@ func (container *Container) HasMountFor(path string) bool { return exists } -// UnmountIpcMounts uses the provided unmount function to unmount shm and mqueue if they were mounted -func (container *Container) UnmountIpcMounts(unmount func(pth string) error) { - if container.HostConfig.IpcMode.IsContainer() || container.HostConfig.IpcMode.IsHost() { - return +// UnmountIpcMount uses the provided unmount function to unmount shm if it was mounted +func (container *Container) UnmountIpcMount(unmount func(pth string) error) error { + if container.HasMountFor("/dev/shm") { + return nil } - var warnings []string - - if !container.HasMountFor("/dev/shm") { - shmPath, err := container.ShmResourcePath() - if err != nil { - logrus.Error(err) - warnings = append(warnings, err.Error()) - } else if shmPath != "" { - if err := unmount(shmPath); err != nil && !os.IsNotExist(err) { - if mounted, mErr := mount.Mounted(shmPath); mounted || mErr != nil { - warnings = append(warnings, fmt.Sprintf("failed to umount %s: %v", shmPath, err)) - } - } - + // container.ShmPath should not be used here as it may point + // to the host's or other container's /dev/shm + shmPath, err := container.ShmResourcePath() + if err != nil { + return err + } + if shmPath == "" { + return nil + } + if err = unmount(shmPath); err != nil && !os.IsNotExist(err) { + if mounted, mErr := mount.Mounted(shmPath); mounted || mErr != nil { + return errors.Wrapf(err, "umount %s", shmPath) } } - - if len(warnings) > 0 { - logrus.Warnf("failed to cleanup ipc mounts:\n%v", strings.Join(warnings, "\n")) - } + return nil } // IpcMounts returns the list of IPC mounts func (container *Container) IpcMounts() []Mount { var mounts []Mount - if !container.HasMountFor("/dev/shm") { - label.SetFileLabel(container.ShmPath, container.MountLabel) - mounts = append(mounts, Mount{ - Source: container.ShmPath, - Destination: "/dev/shm", - Writable: true, - Propagation: string(volume.DefaultPropagationMode), - }) + if container.HasMountFor("/dev/shm") { + return mounts } + if container.ShmPath == "" { + return mounts + } + + label.SetFileLabel(container.ShmPath, container.MountLabel) + mounts = append(mounts, Mount{ + Source: container.ShmPath, + Destination: "/dev/shm", + Writable: true, + Propagation: string(volume.DefaultPropagationMode), + }) return mounts } diff --git a/container/container_windows.go b/container/container_windows.go index 0f2a45df99..9c52d00a46 100644 --- a/container/container_windows.go +++ b/container/container_windows.go @@ -24,9 +24,10 @@ type ExitStatus struct { ExitCode int } -// UnmountIpcMounts unmounts Ipc related mounts. +// UnmountIpcMount unmounts Ipc related mounts. // This is a NOOP on windows. -func (container *Container) UnmountIpcMounts(unmount func(pth string) error) { +func (container *Container) UnmountIpcMount(unmount func(pth string) error) error { + return nil } // IpcMounts returns the list of Ipc related mounts. diff --git a/daemon/config/config.go b/daemon/config/config.go index 61c8deff70..74441d1a5c 100644 --- a/daemon/config/config.go +++ b/daemon/config/config.go @@ -513,6 +513,11 @@ func Validate(config *Config) error { } } + // validate platform-specific settings + if err := config.ValidatePlatformConfig(); err != nil { + return err + } + return nil } diff --git a/daemon/config/config_solaris.go b/daemon/config/config_solaris.go index f4f0802701..4741befac9 100644 --- a/daemon/config/config_solaris.go +++ b/daemon/config/config_solaris.go @@ -27,3 +27,8 @@ type BridgeConfig struct { func (conf *Config) IsSwarmCompatible() error { return nil } + +// ValidatePlatformConfig checks if any platform-specific configuration settings are invalid. +func (conf *Config) ValidatePlatformConfig() error { + return nil +} diff --git a/daemon/config/config_unix.go b/daemon/config/config_unix.go index 8f1da59198..3a9e1cf53c 100644 --- a/daemon/config/config_unix.go +++ b/daemon/config/config_unix.go @@ -5,10 +5,16 @@ package config import ( "fmt" + containertypes "github.com/docker/docker/api/types/container" "github.com/docker/docker/opts" units "github.com/docker/go-units" ) +const ( + // DefaultIpcMode is default for container's IpcMode, if not set otherwise + DefaultIpcMode = "shareable" // TODO: change to private +) + // Config defines the configuration of a docker daemon. // It includes json tags to deserialize configuration from a file // using the same names that the flags in the command line uses. @@ -31,6 +37,7 @@ type Config struct { SeccompProfile string `json:"seccomp-profile,omitempty"` ShmSize opts.MemBytes `json:"default-shm-size,omitempty"` NoNewPrivileges bool `json:"no-new-privileges,omitempty"` + IpcMode string `json:"default-ipc-mode,omitempty"` } // BridgeConfig stores all the bridge driver specific @@ -61,3 +68,21 @@ func (conf *Config) IsSwarmCompatible() error { } return nil } + +func verifyDefaultIpcMode(mode string) error { + const hint = "Use \"shareable\" or \"private\"." + + dm := containertypes.IpcMode(mode) + if !dm.Valid() { + return fmt.Errorf("Default IPC mode setting (%v) is invalid. "+hint, dm) + } + if dm != "" && !dm.IsPrivate() && !dm.IsShareable() { + return fmt.Errorf("IPC mode \"%v\" is not supported as default value. "+hint, dm) + } + return nil +} + +// ValidatePlatformConfig checks if any platform-specific configuration settings are invalid. +func (conf *Config) ValidatePlatformConfig() error { + return verifyDefaultIpcMode(conf.IpcMode) +} diff --git a/daemon/config/config_windows.go b/daemon/config/config_windows.go index 849acc1ac6..6f994e391d 100644 --- a/daemon/config/config_windows.go +++ b/daemon/config/config_windows.go @@ -50,3 +50,8 @@ func (conf *Config) GetExecRoot() string { func (conf *Config) IsSwarmCompatible() error { return nil } + +// ValidatePlatformConfig checks if any platform-specific configuration settings are invalid. +func (conf *Config) ValidatePlatformConfig() error { + return nil +} diff --git a/daemon/container_operations_unix.go b/daemon/container_operations_unix.go index a98a368640..f47e9e2835 100644 --- a/daemon/container_operations_unix.go +++ b/daemon/container_operations_unix.go @@ -57,13 +57,27 @@ func (daemon *Daemon) setupLinkedContainers(container *container.Container) ([]s return env, nil } -func (daemon *Daemon) getIpcContainer(container *container.Container) (*container.Container, error) { - containerID := container.HostConfig.IpcMode.Container() - container, err := daemon.GetContainer(containerID) +func (daemon *Daemon) getIpcContainer(id string) (*container.Container, error) { + errMsg := "can't join IPC of container " + id + // Check the container exists + container, err := daemon.GetContainer(id) if err != nil { - return nil, errors.Wrapf(err, "cannot join IPC of a non running container: %s", container.ID) + return nil, errors.Wrap(err, errMsg) } - return container, daemon.checkContainer(container, containerIsRunning, containerIsNotRestarting) + // Check the container is running and not restarting + if err := daemon.checkContainer(container, containerIsRunning, containerIsNotRestarting); err != nil { + return nil, errors.Wrap(err, errMsg) + } + // Check the container ipc is shareable + if st, err := os.Stat(container.ShmPath); err != nil || !st.IsDir() { + if err == nil || os.IsNotExist(err) { + return nil, errors.New(errMsg + ": non-shareable IPC") + } + // stat() failed? + return nil, errors.Wrap(err, errMsg+": unexpected error from stat "+container.ShmPath) + } + + return container, nil } func (daemon *Daemon) getPidContainer(container *container.Container) (*container.Container, error) { @@ -90,25 +104,33 @@ func containerIsNotRestarting(c *container.Container) error { } func (daemon *Daemon) setupIpcDirs(c *container.Container) error { - var err error + ipcMode := c.HostConfig.IpcMode - c.ShmPath, err = c.ShmResourcePath() - if err != nil { - return err - } - - if c.HostConfig.IpcMode.IsContainer() { - ic, err := daemon.getIpcContainer(c) + switch { + case ipcMode.IsContainer(): + ic, err := daemon.getIpcContainer(ipcMode.Container()) if err != nil { return err } c.ShmPath = ic.ShmPath - } else if c.HostConfig.IpcMode.IsHost() { + + case ipcMode.IsHost(): if _, err := os.Stat("/dev/shm"); err != nil { return fmt.Errorf("/dev/shm is not mounted, but must be for --ipc=host") } c.ShmPath = "/dev/shm" - } else { + + case ipcMode.IsPrivate(), ipcMode.IsNone(): + // c.ShmPath will/should not be used, so make it empty. + // Container's /dev/shm mount comes from OCI spec. + c.ShmPath = "" + + case ipcMode.IsEmpty(): + // A container was created by an older version of the daemon. + // The default behavior used to be what is now called "shareable". + fallthrough + + case ipcMode.IsShareable(): rootIDs := daemon.idMappings.RootPair() if !c.HasMountFor("/dev/shm") { shmPath, err := c.ShmResourcePath() @@ -127,8 +149,11 @@ func (daemon *Daemon) setupIpcDirs(c *container.Container) error { if err := os.Chown(shmPath, rootIDs.UID, rootIDs.GID); err != nil { return err } + c.ShmPath = shmPath } + default: + return fmt.Errorf("invalid IPC mode: %v", ipcMode) } return nil diff --git a/daemon/daemon_solaris.go b/daemon/daemon_solaris.go index 2caefe7cce..905ed5843a 100644 --- a/daemon/daemon_solaris.go +++ b/daemon/daemon_solaris.go @@ -308,7 +308,8 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes. // reloadPlatform updates configuration with platform specific options // and updates the passed attributes -func (daemon *Daemon) reloadPlatform(conf *config.Config, attributes map[string]string) { +func (daemon *Daemon) reloadPlatform(conf *config.Config, attributes map[string]string) error { + return nil } // verifyDaemonSettings performs validation of daemon config struct diff --git a/daemon/daemon_unix.go b/daemon/daemon_unix.go index d5aa8f2dd0..5b317293c2 100644 --- a/daemon/daemon_unix.go +++ b/daemon/daemon_unix.go @@ -276,6 +276,15 @@ func (daemon *Daemon) adaptContainerSettings(hostConfig *containertypes.HostConf hostConfig.ShmSize = int64(daemon.configStore.ShmSize) } } + // Set default IPC mode, if unset for container + if hostConfig.IpcMode.IsEmpty() { + m := config.DefaultIpcMode + if daemon.configStore != nil { + m = daemon.configStore.IpcMode + } + hostConfig.IpcMode = containertypes.IpcMode(m) + } + var err error opts, err := daemon.generateSecurityOpt(hostConfig) if err != nil { @@ -581,7 +590,11 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes. // reloadPlatform updates configuration with platform specific options // and updates the passed attributes -func (daemon *Daemon) reloadPlatform(conf *config.Config, attributes map[string]string) { +func (daemon *Daemon) reloadPlatform(conf *config.Config, attributes map[string]string) error { + if err := conf.ValidatePlatformConfig(); err != nil { + return err + } + if conf.IsValueSet("runtimes") { daemon.configStore.Runtimes = conf.Runtimes // Always set the default one @@ -596,6 +609,10 @@ func (daemon *Daemon) reloadPlatform(conf *config.Config, attributes map[string] daemon.configStore.ShmSize = conf.ShmSize } + if conf.IpcMode != "" { + daemon.configStore.IpcMode = conf.IpcMode + } + // Update attributes var runtimeList bytes.Buffer for name, rt := range daemon.configStore.Runtimes { @@ -608,6 +625,9 @@ func (daemon *Daemon) reloadPlatform(conf *config.Config, attributes map[string] attributes["runtimes"] = runtimeList.String() attributes["default-runtime"] = daemon.configStore.DefaultRuntime attributes["default-shm-size"] = fmt.Sprintf("%d", daemon.configStore.ShmSize) + attributes["default-ipc-mode"] = daemon.configStore.IpcMode + + return nil } // verifyDaemonSettings performs validation of daemon config struct diff --git a/daemon/daemon_windows.go b/daemon/daemon_windows.go index d299d52df6..4cb7560fc2 100644 --- a/daemon/daemon_windows.go +++ b/daemon/daemon_windows.go @@ -231,7 +231,8 @@ func verifyPlatformContainerSettings(daemon *Daemon, hostConfig *containertypes. // reloadPlatform updates configuration with platform specific options // and updates the passed attributes -func (daemon *Daemon) reloadPlatform(config *config.Config, attributes map[string]string) { +func (daemon *Daemon) reloadPlatform(config *config.Config, attributes map[string]string) error { + return nil } // verifyDaemonSettings performs validation of daemon config struct diff --git a/daemon/oci_linux.go b/daemon/oci_linux.go index 9f666f8799..a80de1be31 100644 --- a/daemon/oci_linux.go +++ b/daemon/oci_linux.go @@ -301,10 +301,13 @@ func setNamespaces(daemon *Daemon, s *specs.Spec, c *container.Container) error } setNamespace(s, ns) } + // ipc - if c.HostConfig.IpcMode.IsContainer() { + ipcMode := c.HostConfig.IpcMode + switch { + case ipcMode.IsContainer(): ns := specs.LinuxNamespace{Type: "ipc"} - ic, err := daemon.getIpcContainer(c) + ic, err := daemon.getIpcContainer(ipcMode.Container()) if err != nil { return err } @@ -316,12 +319,19 @@ func setNamespaces(daemon *Daemon, s *specs.Spec, c *container.Container) error nsUser.Path = fmt.Sprintf("/proc/%d/ns/user", ic.State.GetPID()) setNamespace(s, nsUser) } - } else if c.HostConfig.IpcMode.IsHost() { + case ipcMode.IsHost(): oci.RemoveNamespace(s, specs.LinuxNamespaceType("ipc")) - } else { + case ipcMode.IsEmpty(): + // A container was created by an older version of the daemon. + // The default behavior used to be what is now called "shareable". + fallthrough + case ipcMode.IsPrivate(), ipcMode.IsShareable(), ipcMode.IsNone(): ns := specs.LinuxNamespace{Type: "ipc"} setNamespace(s, ns) + default: + return fmt.Errorf("Invalid IPC mode: %v", ipcMode) } + // pid if c.HostConfig.PidMode.IsContainer() { ns := specs.LinuxNamespace{Type: "pid"} @@ -486,10 +496,16 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c userMounts[m.Destination] = struct{}{} } - // Filter out mounts that are overridden by user supplied mounts + // Filter out mounts from spec + noIpc := c.HostConfig.IpcMode.IsNone() var defaultMounts []specs.Mount _, mountDev := userMounts["/dev"] for _, m := range s.Mounts { + // filter out /dev/shm mount if case IpcMode is none + if noIpc && m.Destination == "/dev/shm" { + continue + } + // filter out mount overridden by a user supplied mount if _, ok := userMounts[m.Destination]; !ok { if mountDev && strings.HasPrefix(m.Destination, "/dev/") { continue @@ -589,6 +605,14 @@ func setMounts(daemon *Daemon, s *specs.Spec, c *container.Container, mounts []c s.Linux.MaskedPaths = nil } + // Set size for /dev/shm mount that comes from spec (IpcMode: private only) + for i, m := range s.Mounts { + if m.Destination == "/dev/shm" { + sizeOpt := "size=" + strconv.FormatInt(c.HostConfig.ShmSize, 10) + s.Mounts[i].Options = append(s.Mounts[i].Options, sizeOpt) + } + } + // TODO: until a kernel/mount solution exists for handling remount in a user namespace, // we must clear the readonly flag for the cgroups mount (@mrunalp concurs) if uidMap := daemon.idMappings.UIDs(); uidMap != nil || c.HostConfig.Privileged { @@ -745,7 +769,9 @@ func (daemon *Daemon) createSpec(c *container.Container) (*specs.Spec, error) { return nil, err } - ms = append(ms, c.IpcMounts()...) + if !c.HostConfig.IpcMode.IsPrivate() && !c.HostConfig.IpcMode.IsEmpty() { + ms = append(ms, c.IpcMounts()...) + } tmpfsMounts, err := c.TmpfsMounts() if err != nil { diff --git a/daemon/reload.go b/daemon/reload.go index a71d31efc2..a6674ec951 100644 --- a/daemon/reload.go +++ b/daemon/reload.go @@ -37,7 +37,9 @@ func (daemon *Daemon) Reload(conf *config.Config) (err error) { } }() - daemon.reloadPlatform(conf, attributes) + if err := daemon.reloadPlatform(conf, attributes); err != nil { + return err + } daemon.reloadDebug(conf, attributes) daemon.reloadMaxConcurrentDownloadsAndUploads(conf, attributes) daemon.reloadShutdownTimeout(conf, attributes) diff --git a/daemon/start.go b/daemon/start.go index 290b099e3d..61af1cf9ec 100644 --- a/daemon/start.go +++ b/daemon/start.go @@ -198,7 +198,9 @@ func (daemon *Daemon) containerStart(container *container.Container, checkpoint func (daemon *Daemon) Cleanup(container *container.Container) { daemon.releaseNetwork(container) - container.UnmountIpcMounts(detachMounted) + if err := container.UnmountIpcMount(detachMounted); err != nil { + logrus.Warnf("%s cleanup: failed to unmount IPC: %s", container.ID, err) + } if err := daemon.conditionalUnmountOnCleanup(container); err != nil { // FIXME: remove once reference counting for graphdrivers has been refactored diff --git a/docs/api/version-history.md b/docs/api/version-history.md index 4fed59bf2c..387ce2950c 100644 --- a/docs/api/version-history.md +++ b/docs/api/version-history.md @@ -17,6 +17,9 @@ keywords: "API, Docker, rcli, REST, documentation" [Docker Engine API v1.32](https://docs.docker.com/engine/api/v1.32/) documentation +* `POST /containers/create` now accepts additional values for the + `HostConfig.IpcMode` property. New values are `private`, `shareable`, + and `none`. ## v1.31 API changes diff --git a/oci/defaults.go b/oci/defaults.go index 083726e12d..c1d5909713 100644 --- a/oci/defaults.go +++ b/oci/defaults.go @@ -117,6 +117,12 @@ func DefaultLinuxSpec() specs.Spec { Source: "mqueue", Options: []string{"nosuid", "noexec", "nodev"}, }, + { + Destination: "/dev/shm", + Type: "tmpfs", + Source: "shm", + Options: []string{"nosuid", "noexec", "nodev", "mode=1777"}, + }, } s.Process.Capabilities = &specs.LinuxCapabilities{ Bounding: defaultCapabilities(), diff --git a/runconfig/hostconfig_test.go b/runconfig/hostconfig_test.go index a6a3eef7cd..ec9846ab16 100644 --- a/runconfig/hostconfig_test.go +++ b/runconfig/hostconfig_test.go @@ -10,6 +10,7 @@ import ( "github.com/docker/docker/api/types/container" "github.com/docker/docker/pkg/sysinfo" + "github.com/stretchr/testify/assert" ) // TODO Windows: This will need addressing for a Windows daemon. @@ -61,43 +62,33 @@ func TestNetworkModeTest(t *testing.T) { } func TestIpcModeTest(t *testing.T) { - ipcModes := map[container.IpcMode][]bool{ - // private, host, container, valid - "": {true, false, false, true}, - "something:weird": {true, false, false, false}, - ":weird": {true, false, false, true}, - "host": {false, true, false, true}, - "container:name": {false, false, true, true}, - "container:name:something": {false, false, true, false}, - "container:": {false, false, true, false}, + ipcModes := map[container.IpcMode]struct { + private bool + host bool + container bool + shareable bool + valid bool + ctrName string + }{ + "": {valid: true}, + "private": {private: true, valid: true}, + "something:weird": {}, + ":weird": {}, + "host": {host: true, valid: true}, + "container": {}, + "container:": {container: true, valid: true, ctrName: ""}, + "container:name": {container: true, valid: true, ctrName: "name"}, + "container:name1:name2": {container: true, valid: true, ctrName: "name1:name2"}, + "shareable": {shareable: true, valid: true}, } + for ipcMode, state := range ipcModes { - if ipcMode.IsPrivate() != state[0] { - t.Fatalf("IpcMode.IsPrivate for %v should have been %v but was %v", ipcMode, state[0], ipcMode.IsPrivate()) - } - if ipcMode.IsHost() != state[1] { - t.Fatalf("IpcMode.IsHost for %v should have been %v but was %v", ipcMode, state[1], ipcMode.IsHost()) - } - if ipcMode.IsContainer() != state[2] { - t.Fatalf("IpcMode.IsContainer for %v should have been %v but was %v", ipcMode, state[2], ipcMode.IsContainer()) - } - if ipcMode.Valid() != state[3] { - t.Fatalf("IpcMode.Valid for %v should have been %v but was %v", ipcMode, state[3], ipcMode.Valid()) - } - } - containerIpcModes := map[container.IpcMode]string{ - "": "", - "something": "", - "something:weird": "weird", - "container": "", - "container:": "", - "container:name": "name", - "container:name1:name2": "name1:name2", - } - for ipcMode, container := range containerIpcModes { - if ipcMode.Container() != container { - t.Fatalf("Expected %v for %v but was %v", container, ipcMode, ipcMode.Container()) - } + assert.Equal(t, state.private, ipcMode.IsPrivate(), "IpcMode.IsPrivate() parsing failed for %q", ipcMode) + assert.Equal(t, state.host, ipcMode.IsHost(), "IpcMode.IsHost() parsing failed for %q", ipcMode) + assert.Equal(t, state.container, ipcMode.IsContainer(), "IpcMode.IsContainer() parsing failed for %q", ipcMode) + assert.Equal(t, state.shareable, ipcMode.IsShareable(), "IpcMode.IsShareable() parsing failed for %q", ipcMode) + assert.Equal(t, state.valid, ipcMode.Valid(), "IpcMode.Valid() parsing failed for %q", ipcMode) + assert.Equal(t, state.ctrName, ipcMode.Container(), "IpcMode.Container() parsing failed for %q", ipcMode) } }