From 1a17bc6bc4a61bdbca5c840c556039b4660ab74a Mon Sep 17 00:00:00 2001 From: Brian Goff <cpuguy83@gmail.com> Date: Thu, 2 Feb 2017 23:08:35 -0500 Subject: [PATCH] Make propagated mount persist outside rootfs This persists the "propagated mount" for plugins outside the main rootfs. This enables `docker plugin upgrade` to not remove potentially important data during upgrade rather than forcing plugin authors to hard code a host path to persist data to. Also migrates old plugins that have a propagated mount which is in the rootfs on daemon startup. Signed-off-by: Brian Goff <cpuguy83@gmail.com> (cherry picked from commit e8307b868de9f19bb97f5cafcd727df5c5f501be) Signed-off-by: Brian Goff <cpuguy83@gmail.com> --- docs/extend/config.md | 2 ++ integration-cli/docker_cli_plugins_test.go | 7 +++- plugin/backend_linux.go | 42 ++++++++++++++++++++++ plugin/manager.go | 25 +++++++++++++ plugin/manager_linux.go | 18 ++++++++-- 5 files changed, 91 insertions(+), 3 deletions(-) diff --git a/docs/extend/config.md b/docs/extend/config.md index b98be592a0..096d2d0822 100644 --- a/docs/extend/config.md +++ b/docs/extend/config.md @@ -114,6 +114,8 @@ Config provides the base accessible fields for working with V0 plugin format - **`propagatedMount`** *string* path to be mounted as rshared, so that mounts under that path are visible to docker. This is useful for volume plugins. + This path will be bind-mounted outisde of the plugin rootfs so it's contents + are preserved on upgrade. - **`env`** *PluginEnv array* diff --git a/integration-cli/docker_cli_plugins_test.go b/integration-cli/docker_cli_plugins_test.go index 6376da78c1..380357d303 100644 --- a/integration-cli/docker_cli_plugins_test.go +++ b/integration-cli/docker_cli_plugins_test.go @@ -366,6 +366,9 @@ func (s *DockerSuite) TestPluginUpgrade(c *check.C) { pluginV2 := "cpuguy83/docker-volume-driver-plugin-local:v2" dockerCmd(c, "plugin", "install", "--grant-all-permissions", plugin) + dockerCmd(c, "volume", "create", "--driver", plugin, "bananas") + dockerCmd(c, "run", "--rm", "-v", "bananas:/apple", "busybox", "sh", "-c", "touch /apple/core") + out, _, err := dockerCmdWithError("plugin", "upgrade", "--grant-all-permissions", plugin, pluginV2) c.Assert(err, checker.NotNil, check.Commentf(out)) c.Assert(out, checker.Contains, "disabled before upgrading") @@ -377,7 +380,7 @@ func (s *DockerSuite) TestPluginUpgrade(c *check.C) { _, err = os.Stat(filepath.Join(dockerBasePath, "plugins", id, "rootfs", "v2")) c.Assert(os.IsNotExist(err), checker.True, check.Commentf(out)) - dockerCmd(c, "plugin", "disable", plugin) + dockerCmd(c, "plugin", "disable", "-f", plugin) dockerCmd(c, "plugin", "upgrade", "--grant-all-permissions", "--skip-remote-check", plugin, pluginV2) // make sure "v2" file exists @@ -385,4 +388,6 @@ func (s *DockerSuite) TestPluginUpgrade(c *check.C) { c.Assert(err, checker.IsNil) dockerCmd(c, "plugin", "enable", plugin) + dockerCmd(c, "volume", "inspect", "bananas") + dockerCmd(c, "run", "--rm", "-v", "bananas:/apple", "busybox", "sh", "-c", "ls -lh /apple/core") } diff --git a/plugin/backend_linux.go b/plugin/backend_linux.go index 97046ee185..33200d8efa 100644 --- a/plugin/backend_linux.go +++ b/plugin/backend_linux.go @@ -13,6 +13,7 @@ import ( "os" "path" "path/filepath" + "sort" "strings" "github.com/Sirupsen/logrus" @@ -25,6 +26,7 @@ import ( "github.com/docker/docker/image" "github.com/docker/docker/layer" "github.com/docker/docker/pkg/chrootarchive" + "github.com/docker/docker/pkg/mount" "github.com/docker/docker/pkg/pools" "github.com/docker/docker/pkg/progress" "github.com/docker/docker/plugin/v2" @@ -560,6 +562,9 @@ func (pm *Manager) Remove(name string, config *types.PluginRmConfig) error { id := p.GetID() pm.config.Store.Remove(p) pluginDir := filepath.Join(pm.config.Root, id) + if err := recursiveUnmount(pm.config.Root); err != nil { + logrus.WithField("dir", pm.config.Root).WithField("id", id).Warn(err) + } if err := os.RemoveAll(pluginDir); err != nil { logrus.Warnf("unable to remove %q from plugin remove: %v", pluginDir, err) } @@ -567,6 +572,43 @@ func (pm *Manager) Remove(name string, config *types.PluginRmConfig) error { return nil } +func getMounts(root string) ([]string, error) { + infos, err := mount.GetMounts() + if err != nil { + return nil, errors.Wrap(err, "failed to read mount table while performing recursive unmount") + } + + var mounts []string + for _, m := range infos { + if strings.HasPrefix(m.Mountpoint, root) { + mounts = append(mounts, m.Mountpoint) + } + } + + return mounts, nil +} + +func recursiveUnmount(root string) error { + mounts, err := getMounts(root) + if err != nil { + return err + } + + // sort in reverse-lexicographic order so the root mount will always be last + sort.Sort(sort.Reverse(sort.StringSlice(mounts))) + + for i, m := range mounts { + if err := mount.Unmount(m); err != nil { + if i == len(mounts)-1 { + return errors.Wrapf(err, "error performing recursive unmount on %s", root) + } + logrus.WithError(err).WithField("mountpoint", m).Warn("could not unmount") + } + } + + return nil +} + // Set sets plugin args func (pm *Manager) Set(name string, args []string) error { p, err := pm.config.Store.GetV2Plugin(name) diff --git a/plugin/manager.go b/plugin/manager.go index 2598321e60..f260aa61a7 100644 --- a/plugin/manager.go +++ b/plugin/manager.go @@ -145,6 +145,10 @@ func (pm *Manager) StateChanged(id string, e libcontainerd.StateInfo) error { if err := mount.Unmount(p.PropagatedMount); err != nil { logrus.Warnf("Could not unmount %s: %v", p.PropagatedMount, err) } + propRoot := filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") + if err := mount.Unmount(propRoot); err != nil { + logrus.Warn("Could not unmount %s: %v", propRoot, err) + } } if restart { @@ -193,6 +197,27 @@ func (pm *Manager) reload() error { // todo: restore for _, typ := range p.PluginObj.Config.Interface.Types { if (typ.Capability == "volumedriver" || typ.Capability == "graphdriver") && typ.Prefix == "docker" && strings.HasPrefix(typ.Version, "1.") { if p.PluginObj.Config.PropagatedMount != "" { + propRoot := filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") + + // check if we need to migrate an older propagated mount from before + // these mounts were stored outside the plugin rootfs + if _, err := os.Stat(propRoot); os.IsNotExist(err) { + if _, err := os.Stat(p.PropagatedMount); err == nil { + // make sure nothing is mounted here + // don't care about errors + mount.Unmount(p.PropagatedMount) + if err := os.Rename(p.PropagatedMount, propRoot); err != nil { + logrus.WithError(err).WithField("dir", propRoot).Error("error migrating propagated mount storage") + } + if err := os.MkdirAll(p.PropagatedMount, 0755); err != nil { + logrus.WithError(err).WithField("dir", p.PropagatedMount).Error("error migrating propagated mount storage") + } + } + } + + if err := os.MkdirAll(propRoot, 0755); err != nil { + logrus.Errorf("failed to create PropagatedMount directory at %s: %v", propRoot, err) + } // TODO: sanitize PropagatedMount and prevent breakout p.PropagatedMount = filepath.Join(p.Rootfs, p.PluginObj.Config.PropagatedMount) if err := os.MkdirAll(p.PropagatedMount, 0755); err != nil { diff --git a/plugin/manager_linux.go b/plugin/manager_linux.go index f13b43c415..ad66616628 100644 --- a/plugin/manager_linux.go +++ b/plugin/manager_linux.go @@ -40,9 +40,20 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error { pm.cMap[p] = c pm.mu.Unlock() + var propRoot string if p.PropagatedMount != "" { - if err := mount.MakeRShared(p.PropagatedMount); err != nil { - return errors.WithStack(err) + propRoot = filepath.Join(filepath.Dir(p.Rootfs), "propagated-mount") + + if err := os.MkdirAll(propRoot, 0755); err != nil { + logrus.Errorf("failed to create PropagatedMount directory at %s: %v", propRoot, err) + } + + if err := mount.MakeRShared(propRoot); err != nil { + return errors.Wrap(err, "error setting up propagated mount dir") + } + + if err := mount.Mount(propRoot, p.PropagatedMount, "none", "rbind"); err != nil { + return errors.Wrap(err, "error creating mount for propagated mount") } } @@ -55,6 +66,9 @@ func (pm *Manager) enable(p *v2.Plugin, c *controller, force bool) error { if err := mount.Unmount(p.PropagatedMount); err != nil { logrus.Warnf("Could not unmount %s: %v", p.PropagatedMount, err) } + if err := mount.Unmount(propRoot); err != nil { + logrus.Warnf("Could not unmount %s: %v", propRoot, err) + } } return errors.WithStack(err) }