From 57e38e9550406d99dbad169d3ad4e571310e7842 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 21 May 2019 10:16:02 -0400 Subject: When superceding mounts, check for opposite types When we supercede low-priority mounts and volumes (image volumes, and volumes sourced from --volumes-from) with higher-priority ones (the --volume and --mount flags), we always replaced lower-priority mounts of the same type (e.g. a user mount to /tmp/test1 would supercede a volumes-from mount to the same destination). However, we did not supercede the opposite type - a named volume from image volumes at /tmp/test1 would be allowed to remain and create a conflict, preventing container creation. Solve this by destroying opposite types before merging (we can't do it in the same loop, as then named volumes, which go second, might trample changes made by mounts). Fixes #3174 Signed-off-by: Matthew Heon --- pkg/spec/storage.go | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go index 55148b606..dcc149b55 100644 --- a/pkg/spec/storage.go +++ b/pkg/spec/storage.go @@ -135,6 +135,29 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount, unifiedMounts[initMount.Destination] = initMount } + // Before superceding, we need to find volume mounts which conflict with + // named volumes, and vice versa. + // We'll delete the conflicts here as we supercede. + for dest := range unifiedMounts { + if _, ok := baseVolumes[dest]; ok { + delete(baseVolumes, dest) + } + } + for dest := range unifiedVolumes { + if _, ok := baseMounts[dest]; ok { + delete(baseMounts, dest) + } + } + + // Supercede volumes-from/image volumes with unified volumes from above. + // This is an unconditional replacement. + for dest, mount := range unifiedMounts { + baseMounts[dest] = mount + } + for dest, volume := range unifiedVolumes { + baseVolumes[dest] = volume + } + // If requested, add tmpfs filesystems for read-only containers. // Need to keep track of which we created, so we don't modify options // for them later... @@ -146,14 +169,14 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount, if config.ReadOnlyRootfs && config.ReadOnlyTmpfs { options := []string{"rw", "rprivate", "nosuid", "nodev", "tmpcopyup", "size=65536k"} for dest := range readonlyTmpfs { - if _, ok := unifiedMounts[dest]; ok { + if _, ok := baseMounts[dest]; ok { continue } localOpts := options if dest == "/run" { localOpts = append(localOpts, "noexec") } - unifiedMounts[dest] = spec.Mount{ + baseMounts[dest] = spec.Mount{ Destination: dest, Type: "tmpfs", Source: "tmpfs", @@ -163,15 +186,6 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount, } } - // Supercede volumes-from/image volumes with unified volumes from above. - // This is an unconditional replacement. - for dest, mount := range unifiedMounts { - baseMounts[dest] = mount - } - for dest, volume := range unifiedVolumes { - baseVolumes[dest] = volume - } - // Check for conflicts between named volumes and mounts for dest := range baseMounts { if _, ok := baseVolumes[dest]; ok { -- cgit v1.2.3-54-g00ecf From 43f28cdc166f467f3df0f553f696ecaaf0881979 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Tue, 21 May 2019 10:46:40 -0400 Subject: Add test for image volume conflict with user volume Signed-off-by: Matthew Heon --- test/e2e/run_volume_test.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index e27b2aa55..d031ca143 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -104,4 +104,18 @@ var _ = Describe("Podman run with volumes", func() { session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(125)) }) + + It("podman run with conflict between image volume and user mount succeeds", func() { + podmanTest.RestoreArtifact(redis) + mountPath := filepath.Join(podmanTest.TempDir, "secrets") + err := os.Mkdir(mountPath, 0755) + Expect(err).To(BeNil()) + testFile := filepath.Join(mountPath, "test1") + f, err := os.Create(testFile) + f.Close() + Expect(err).To(BeNil()) + session := podmanTest.Podman([]string{"run", "-v", fmt.Sprintf("%s:/data", mountPath), redis, "ls", "/data/test1"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + }) }) -- cgit v1.2.3-54-g00ecf