When first mounting any named volume, copy up

Previously, we only did this for volumes created at the same time
as the container. However, this is not correct behavior - Docker
does so for all named volumes, even those made with
'podman volume create' and mounted into a container later.

Fixes containers#3945

Signed-off-by: Matthew Heon <matthew.heon@pm.me>

libpod/container_internal.go

@@ -1236,29 +1236,20 @@ func (c *Container) mountStorage() (_ string, Err error) {
 
 	// Request a mount of all named volumes
 	for _, v := range c.config.NamedVolumes {
-		vol, err := c.runtime.state.Volume(v.Name)
+		vol, err := c.mountNamedVolume(v)
 		if err != nil {
-			return "", errors.Wrapf(err, "error retrieving named volume %s for container %s", v.Name, c.ID())
+			return "", err
 		}
-
-		if vol.needsMount() {
+		defer func() {
+			if Err == nil {
+				return
+			}
 			vol.lock.Lock()
-			if err := vol.mount(); err != nil {
-				vol.lock.Unlock()
-				return "", errors.Wrapf(err, "error mounting volume %s for container %s", vol.Name(), c.ID())
+			if err := vol.unmount(false); err != nil {
+				logrus.Errorf("Error unmounting volume %s after error mounting container %s: %v", vol.Name(), c.ID(), err)
 			}
 			vol.lock.Unlock()
-			defer func() {
-				if Err == nil {
-					return
-				}
-				vol.lock.Lock()
-				if err := vol.unmount(false); err != nil {
-					logrus.Errorf("Error unmounting volume %s after error mounting container %s: %v", vol.Name(), c.ID(), err)
-				}
-				vol.lock.Unlock()
-			}()
-		}
+		}()
 	}
 
 	// TODO: generalize this mount code so it will mount every mount in ctr.config.Mounts
@@ -1273,6 +1264,42 @@ func (c *Container) mountStorage() (_ string, Err error) {
 	return mountPoint, nil
 }
 
+// Mount a single named volume into the container.
+// If necessary, copy up image contents into the volume.
+// Does not verify that the name volume given is actually present in container
+// config.
+// Returns the volume that was mounted.
+func (c *Container) mountNamedVolume(v *ContainerNamedVolume) (*Volume, error) {
+	vol, err := c.runtime.state.Volume(v.Name)
+	if err != nil {
+		return nil, errors.Wrapf(err, "error retrieving named volume %s for container %s", v.Name, c.ID())
+	}
+
+	vol.lock.Lock()
+	defer vol.lock.Unlock()
+	if vol.needsMount() {
+		if err := vol.mount(); err != nil {
+			return nil, errors.Wrapf(err, "error mounting volume %s for container %s", vol.Name(), c.ID())
+		}
+	}
+	// The volume may need a copy-up. Check the state.
+	if err := vol.update(); err != nil {
+		return nil, err
+	}
+	if vol.state.NeedsCopyUp {
+		logrus.Debugf("Copying up contents from container %s to volume %s", c.ID(), vol.Name())
+		if err := c.copyWithTarFromImage(v.Dest, vol.MountPoint()); err != nil && !os.IsNotExist(err) {
+			return nil, errors.Wrapf(err, "error copying content from container %s into volume %s", c.ID(), vol.Name())
+		}
+
+		vol.state.NeedsCopyUp = false
+		if err := vol.save(); err != nil {
+			return nil, err
+		}
+	}
+	return vol, nil
+}
+
 // cleanupStorage unmounts and cleans up the container's root filesystem
 func (c *Container) cleanupStorage() error {
 	if !c.state.Mounted {
@@ -1614,13 +1641,10 @@ func (c *Container) unmount(force bool) error {
 }
 
 // this should be from chrootarchive.
+// Container MUST be mounted before calling.
 func (c *Container) copyWithTarFromImage(src, dest string) error {
-	mountpoint, err := c.mount()
-	if err != nil {
-		return err
-	}
 	a := archive.NewDefaultArchiver()
-	source := filepath.Join(mountpoint, src)
+	source := filepath.Join(c.state.Mountpint, src)
 
 	if err = c.copyOwnerAndPerms(source, dest); err != nil {
 		return err

libpod/container_internal_linux.go

@@ -115,7 +115,9 @@ func (c *Container) prepare() (Err error) {
 		createErr = createNetNSErr
 	}
 	if mountStorageErr != nil {
-		logrus.Errorf("Error preparing container %s: %v", c.ID(), createErr)
+		if createErr != nil {
+			logrus.Errorf("Error preparing container %s: %v", c.ID(), createErr)
+		}
 		createErr = mountStorageErr
 	}
 

libpod/runtime_ctr.go

@@ -275,10 +275,6 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (c *Contai
 			return nil, errors.Wrapf(err, "error creating named volume %q", vol.Name)
 		}
 
-		if err := ctr.copyWithTarFromImage(vol.Dest, newVol.MountPoint()); err != nil && !os.IsNotExist(err) {
-			return nil, errors.Wrapf(err, "Failed to copy content into new volume mount %q", vol.Name)
-		}
-
 		ctrNamedVolumes = append(ctrNamedVolumes, newVol)
 	}
 

libpod/volume.go

@@ -57,6 +57,13 @@ type VolumeState struct {
 	// On incrementing from 0, the volume will be mounted on the host.
 	// On decrementing to 0, the volume will be unmounted on the host.
 	MountCount uint `json:"mountCount"`
+	// NeedsCopyUp indicates that the next time the volume is mounted into
+	// a container, the container will "copy up" the contents of the
+	// mountpoint into the volume.
+	// This should only be done once. As such, this is set at container
+	// create time, then cleared after the copy up is done and never set
+	// again.
+	NeedsCopyUp bool `json:"notYetMounted,omitempty"`
 }
 
 // Name retrieves the volume's name

libpod/volume_internal.go

@@ -11,9 +11,11 @@ import (
 func newVolume(runtime *Runtime) (*Volume, error) {
 	volume := new(Volume)
 	volume.config = new(VolumeConfig)
+	volume.state = new(VolumeState)
 	volume.runtime = runtime
 	volume.config.Labels = make(map[string]string)
 	volume.config.Options = make(map[string]string)
+	volume.state.NeedsCopyUp = true
 
 	return volume, nil
 }