summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorOpenShift Merge Robot <openshift-merge-robot@users.noreply.github.com>2019-09-10 22:24:43 +0200
committerGitHub <noreply@github.com>2019-09-10 22:24:43 +0200
commit997c4b56ed2121726e966afe9a102ed16ba78f93 (patch)
tree35a0b0a43f9c5b6d484f93564f01a373802e44df
parentc1761ba1ac4155bab82fb9b847ccf96489b98265 (diff)
parentb6106341fb3a749ec37e59ee3290257a1729def1 (diff)
downloadpodman-997c4b56ed2121726e966afe9a102ed16ba78f93.tar.gz
podman-997c4b56ed2121726e966afe9a102ed16ba78f93.tar.bz2
podman-997c4b56ed2121726e966afe9a102ed16ba78f93.zip
Merge pull request #3961 from mheon/copy_volume_contents
When first mounting any named volume, copy up
-rw-r--r--libpod/container_internal.go100
-rw-r--r--libpod/container_internal_linux.go4
-rw-r--r--libpod/runtime_ctr.go4
-rw-r--r--libpod/volume.go7
-rw-r--r--libpod/volume_internal.go2
-rw-r--r--test/e2e/run_volume_test.go21
6 files changed, 101 insertions, 37 deletions
diff --git a/libpod/container_internal.go b/libpod/container_internal.go
index ffc6c11ee..78305d761 100644
--- a/libpod/container_internal.go
+++ b/libpod/container_internal.go
@@ -21,6 +21,7 @@ import (
"github.com/containers/storage"
"github.com/containers/storage/pkg/archive"
"github.com/containers/storage/pkg/mount"
+ "github.com/cyphar/filepath-securejoin"
spec "github.com/opencontainers/runtime-spec/specs-go"
"github.com/opencontainers/runtime-tools/generate"
"github.com/opencontainers/selinux/go-selinux/label"
@@ -1234,43 +1235,82 @@ func (c *Container) mountStorage() (_ string, Err error) {
}()
}
+ // We need to mount the container before volumes - to ensure the copyup
+ // works properly.
+ mountPoint := c.config.Rootfs
+ if mountPoint == "" {
+ mountPoint, err = c.mount()
+ if err != nil {
+ return "", err
+ }
+ defer func() {
+ if Err != nil {
+ if err := c.unmount(false); err != nil {
+ logrus.Errorf("Error unmounting container %s after mount error: %v", c.ID(), err)
+ }
+ }
+ }()
+ }
+
// 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, mountPoint)
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
- mountPoint := c.config.Rootfs
- if mountPoint == "" {
- mountPoint, err = c.mount()
- if err != nil {
- return "", err
+ 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, mountpoint string) (*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())
+ srcDir, err := securejoin.SecureJoin(mountpoint, v.Dest)
+ if err != nil {
+ return nil, errors.Wrapf(err, "error calculating destination path to copy up container %s volume %s", c.ID(), vol.Name())
+ }
+ if err := c.copyWithTarFromImage(srcDir, 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())
+ }
- return mountPoint, nil
+ 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
@@ -1614,15 +1654,11 @@ func (c *Container) unmount(force bool) error {
}
// this should be from chrootarchive.
-func (c *Container) copyWithTarFromImage(src, dest string) error {
- mountpoint, err := c.mount()
- if err != nil {
- return err
- }
+// Container MUST be mounted before calling.
+func (c *Container) copyWithTarFromImage(source, dest string) error {
a := archive.NewDefaultArchiver()
- source := filepath.Join(mountpoint, src)
- if err = c.copyOwnerAndPerms(source, dest); err != nil {
+ if err := c.copyOwnerAndPerms(source, dest); err != nil {
return err
}
return a.CopyWithTar(source, dest)
diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go
index e96af8536..03374a060 100644
--- a/libpod/container_internal_linux.go
+++ b/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
}
diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go
index acd317d20..0b4fbda7a 100644
--- a/libpod/runtime_ctr.go
+++ b/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)
}
diff --git a/libpod/volume.go b/libpod/volume.go
index b4de3aedc..c4771bbb8 100644
--- a/libpod/volume.go
+++ b/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
diff --git a/libpod/volume_internal.go b/libpod/volume_internal.go
index 2e886e1b0..42b935e7c 100644
--- a/libpod/volume_internal.go
+++ b/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
}
diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go
index 551e86b93..fc1998ab2 100644
--- a/test/e2e/run_volume_test.go
+++ b/test/e2e/run_volume_test.go
@@ -249,4 +249,25 @@ var _ = Describe("Podman run with volumes", func() {
fmt.Printf("Output: %s", mountOut3)
Expect(strings.Contains(mountOut3, volName)).To(BeFalse())
})
+
+ It("podman named volume copyup", func() {
+ baselineSession := podmanTest.Podman([]string{"run", "--rm", "-t", "-i", ALPINE, "ls", "/etc/apk/"})
+ baselineSession.WaitWithDefaultTimeout()
+ Expect(baselineSession.ExitCode()).To(Equal(0))
+ baselineOutput := baselineSession.OutputToString()
+
+ inlineVolumeSession := podmanTest.Podman([]string{"run", "--rm", "-t", "-i", "-v", "testvol1:/etc/apk", ALPINE, "ls", "/etc/apk/"})
+ inlineVolumeSession.WaitWithDefaultTimeout()
+ Expect(inlineVolumeSession.ExitCode()).To(Equal(0))
+ Expect(inlineVolumeSession.OutputToString()).To(Equal(baselineOutput))
+
+ makeVolumeSession := podmanTest.Podman([]string{"volume", "create", "testvol2"})
+ makeVolumeSession.WaitWithDefaultTimeout()
+ Expect(makeVolumeSession.ExitCode()).To(Equal(0))
+
+ separateVolumeSession := podmanTest.Podman([]string{"run", "--rm", "-t", "-i", "-v", "testvol2:/etc/apk", ALPINE, "ls", "/etc/apk/"})
+ separateVolumeSession.WaitWithDefaultTimeout()
+ Expect(separateVolumeSession.ExitCode()).To(Equal(0))
+ Expect(separateVolumeSession.OutputToString()).To(Equal(baselineOutput))
+ })
})