Allow identical duplicate volumes and mounts

Docker allows exactly identical mounts and volumes to be passed
without throwing an error. If a volume is exactly identical, we
should not error - otherwise, we'll still give a duplicate mount
destination error.

Fixes #4217

Signed-off-by: Matthew Heon <[email protected]>

pkg/spec/storage.go

@@ -82,13 +82,21 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount,
 	// Also add mounts + volumes directly from createconfig.
 	// Start with --volume.
 	for dest, mount := range volumeMounts {
-		if _, ok := unifiedMounts[dest]; ok {
+		if oldMount, ok := unifiedMounts[dest]; ok {
+			// Identical mounts are allowed, just dedup them.
+			if mountsEqual(mount, oldMount) {
+				continue
+			}
 			return nil, nil, errors.Wrapf(errDuplicateDest, dest)
 		}
 		unifiedMounts[dest] = mount
 	}
 	for dest, volume := range volumeVolumes {
-		if _, ok := unifiedVolumes[dest]; ok {
+		if oldVolume, ok := unifiedVolumes[dest]; ok {
+			// Identical volumes are allowed, just dedup them.
+			if namedVolumesEqual(volume, oldVolume) {
+				continue
+			}
 			return nil, nil, errors.Wrapf(errDuplicateDest, dest)
 		}
 		unifiedVolumes[dest] = volume
@@ -919,3 +927,50 @@ func findMount(target string, mounts []*pmount.Info) (*pmount.Info, error) {
 	}
 	return bestSoFar, nil
 }
+
+// Check if two spec.Mount structs are equal
+func mountsEqual(a, b spec.Mount) bool {
+	if a.Destination != b.Destination {
+		return false
+	}
+	if a.Type != b.Type {
+		return false
+	}
+	if a.Source != b.Source {
+		return false
+	}
+	return mountOptsEqual(a.Options, b.Options)
+}
+
+// Check if two named volumes are equal
+func namedVolumesEqual(a, b *libpod.ContainerNamedVolume) bool {
+	if a.Name != b.Name {
+		return false
+	}
+	if a.Dest != b.Dest {
+		return false
+	}
+	return mountOptsEqual(a.Options, b.Options)
+}
+
+// Check if two sets of mount options are equal
+func mountOptsEqual(a, b []string) bool {
+	if len(a) != len(b) {
+		return false
+	}
+	// Quick short-circuit to avoid map allocation for the most common case
+	// (no options).
+	if len(a) == 0 {
+		return true
+	}
+	optsMap := make(map[string]bool)
+	for _, opt := range a {
+		optsMap[opt] = true
+	}
+	for _, opt := range b {
+		if _, exists := optsMap[opt]; !exists {
+			return false
+		}
+	}
+	return true
+}

test/e2e/run_volume_test.go

@@ -375,4 +375,20 @@ var _ = Describe("Podman run with volumes", func() {
 		volMount.WaitWithDefaultTimeout()
 		Expect(volMount.ExitCode()).To(Not(Equal(0)))
 	})
+
+	It("podman mount with duplicate volumes conflicting succeeds", func() {
+		mount1 := "/tmp:/testmnt"
+		mount2 := "/tmp:/testmnt2:ro,rshared"
+		volume1 := "testvol:/testvol"
+		volume2 := "testvol2:/testvol2:ro,rshared"
+
+		for _, vol := range []string{mount1, mount2, volume1, volume2} {
+			dst := strings.Split(vol, ":")[1]
+			test := podmanTest.Podman([]string{"create", "-v", vol, "-v", vol, ALPINE, "grep", dst, "/proc/self/mountinfo"})
+			test.WaitWithDefaultTimeout()
+			Expect(test.ExitCode()).To(Equal(0))
+			found, matches := test.GrepString(dst)
+			Expect(found).Should(BeTrue())
+		}
+	})
 })