From 61b57b7d7d9f213577833b77a031b7439fb4ec7b Mon Sep 17 00:00:00 2001 From: Matt Heon Date: Thu, 14 Nov 2024 10:12:12 -0500 Subject: [PATCH] Overlay mounts supersede image volumes & volumes-from This matches the behavior of other volume and mount types. Image volumes and volumes/mounts from the `--volumes-from` flag should be overridden by actual user-specified named volumes and mounts, but this was not true for overlay mounts. Fortunately, our duplicate-mount detection logic still works, so we got a good error message at least. The fix is simple - extend our supersede logic, which currently only works with named volumes and mounts, to also work with overlay mounts. Fixes #24555 Signed-off-by: Matt Heon --- pkg/specgen/generate/storage.go | 14 +++++++++++++- test/e2e/run_volume_test.go | 20 ++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/pkg/specgen/generate/storage.go b/pkg/specgen/generate/storage.go index 7703e90815..a79a6cf6b2 100644 --- a/pkg/specgen/generate/storage.go +++ b/pkg/specgen/generate/storage.go @@ -157,6 +157,12 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru delete(baseMounts, dest) } + // Overlays are neither mounts nor volumes but should supersede both. + for dest := range unifiedOverlays { + delete(baseVolumes, dest) + delete(baseMounts, dest) + } + // Supersede volumes-from/image volumes with unified volumes from above. // This is an unconditional replacement. for dest, mount := range unifiedMounts { @@ -169,16 +175,22 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru // TODO: Investigate moving readonlyTmpfs into here. Would be more // correct. - // Check for conflicts between named volumes and mounts + // Check for conflicts between named volumes, mounts, and overlays for dest := range baseMounts { if _, ok := baseVolumes[dest]; ok { return nil, nil, nil, fmt.Errorf("baseMounts conflict at mount destination %v: %w", dest, specgen.ErrDuplicateDest) } + if _, ok := unifiedOverlays[dest]; ok { + return nil, nil, nil, fmt.Errorf("baseMounts conflict with overlay mount at mount destination %v: %w", dest, specgen.ErrDuplicateDest) + } } for dest := range baseVolumes { if _, ok := baseMounts[dest]; ok { return nil, nil, nil, fmt.Errorf("baseVolumes conflict at mount destination %v: %w", dest, specgen.ErrDuplicateDest) } + if _, ok := unifiedOverlays[dest]; ok { + return nil, nil, nil, fmt.Errorf("baseVolumes conflict with overlay mount at mount destination %v: %w", dest, specgen.ErrDuplicateDest) + } } if s.ReadWriteTmpfs != nil && *s.ReadWriteTmpfs { diff --git a/test/e2e/run_volume_test.go b/test/e2e/run_volume_test.go index 6f184c3218..b14ca83cc4 100644 --- a/test/e2e/run_volume_test.go +++ b/test/e2e/run_volume_test.go @@ -139,6 +139,10 @@ var _ = Describe("Podman run with volumes", func() { session := podmanTest.Podman([]string{"run", "-v", mountPath + ":" + dest, "-v", "/tmp" + ":" + dest, ALPINE, "ls"}) session.WaitWithDefaultTimeout() Expect(session).Should(ExitWithError(125, fmt.Sprintf("%s: duplicate mount destination", dest))) + + session = podmanTest.Podman([]string{"run", "-v", "myvol:" + dest, "-v", mountPath + ":" + dest + ":O", ALPINE, "ls", "/test"}) + session.WaitWithDefaultTimeout() + Expect(session).To(ExitWithError(125, fmt.Sprintf("%s: duplicate mount destination", dest))) }) It("podman run with conflict between image volume and user mount succeeds", func() { @@ -1089,4 +1093,20 @@ RUN chmod 755 /test1 /test2 /test3`, ALPINE) Expect(checkCtr.OutputToString()).To(ContainSubstring("foo")) Expect(checkCtr.OutputToString()).To(ContainSubstring("bar")) }) + + It("user-specified overlay supersedes image volume", func() { + err := podmanTest.RestoreArtifact(REDIS_IMAGE) + Expect(err).ToNot(HaveOccurred()) + mountPath := filepath.Join(podmanTest.TempDir, "secrets") + err = os.Mkdir(mountPath, 0755) + Expect(err).ToNot(HaveOccurred()) + testFile := filepath.Join(mountPath, "test1") + f, err := os.Create(testFile) + Expect(err).ToNot(HaveOccurred(), "os.Create(testfile)") + f.Close() + Expect(err).ToNot(HaveOccurred()) + session := podmanTest.Podman([]string{"run", "-v", fmt.Sprintf("%s:/data:O", mountPath), REDIS_IMAGE, "ls", "/data/test1"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitCleanly()) + }) })