Merge pull request #16165 from rhatdan/dups

Allow volume mount dups, iff source and dest dirs
This commit is contained in:
OpenShift Merge Robot
2022-10-17 10:11:09 -04:00
committed by GitHub
5 changed files with 93 additions and 32 deletions

View File

@ -20,8 +20,6 @@ import (
"github.com/sirupsen/logrus"
)
var errDuplicateDest = errors.New("duplicate mount destination")
// Produce final mounts and named volumes for a container
func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Runtime, rtc *config.Config, img *libimage.Image) ([]spec.Mount, []*specgen.NamedVolume, []*specgen.OverlayVolume, error) {
// Get image volumes
@ -63,7 +61,7 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru
}
cleanDestination := filepath.Clean(m.Destination)
if _, ok := unifiedMounts[cleanDestination]; ok {
return nil, nil, nil, fmt.Errorf("conflict in specified mounts - multiple mounts at %q: %w", cleanDestination, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("%q: %w", cleanDestination, specgen.ErrDuplicateDest)
}
unifiedMounts[cleanDestination] = m
}
@ -84,7 +82,7 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru
}
cleanDestination := filepath.Clean(v.Dest)
if _, ok := unifiedVolumes[cleanDestination]; ok {
return nil, nil, nil, fmt.Errorf("conflict in specified volumes - multiple volumes at %q: %w", cleanDestination, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("conflict in specified volumes - multiple volumes at %q: %w", cleanDestination, specgen.ErrDuplicateDest)
}
unifiedVolumes[cleanDestination] = v
}
@ -105,7 +103,7 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru
}
cleanDestination := filepath.Clean(v.Destination)
if _, ok := unifiedOverlays[cleanDestination]; ok {
return nil, nil, nil, fmt.Errorf("conflict in specified volumes - multiple volumes at %q: %w", cleanDestination, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("conflict in specified volumes - multiple volumes at %q: %w", cleanDestination, specgen.ErrDuplicateDest)
}
unifiedOverlays[cleanDestination] = v
}
@ -131,7 +129,7 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru
return nil, nil, nil, err
}
if _, ok := unifiedMounts[initMount.Destination]; ok {
return nil, nil, nil, fmt.Errorf("conflict with mount added by --init to %q: %w", initMount.Destination, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("conflict with mount added by --init to %q: %w", initMount.Destination, specgen.ErrDuplicateDest)
}
unifiedMounts[initMount.Destination] = initMount
}
@ -161,12 +159,12 @@ func finalizeMounts(ctx context.Context, s *specgen.SpecGenerator, rt *libpod.Ru
// Check for conflicts between named volumes and mounts
for dest := range baseMounts {
if _, ok := baseVolumes[dest]; ok {
return nil, nil, nil, fmt.Errorf("conflict at mount destination %v: %w", dest, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("conflict at mount destination %v: %w", dest, specgen.ErrDuplicateDest)
}
}
for dest := range baseVolumes {
if _, ok := baseMounts[dest]; ok {
return nil, nil, nil, fmt.Errorf("conflict at mount destination %v: %w", dest, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("conflict at mount destination %v: %w", dest, specgen.ErrDuplicateDest)
}
}
// Final step: maps to arrays

View File

@ -581,6 +581,8 @@ var (
// ErrNoStaticMACRootless is used when a rootless user requests to assign a static MAC address
// to a pod or container
ErrNoStaticMACRootless = errors.New("rootless containers and pods cannot be assigned static MAC addresses")
// Multiple volume mounts to the same destination is not allowed
ErrDuplicateDest = errors.New("duplicate mount destination")
)
// NewSpecGenerator returns a SpecGenerator struct given one of two mandatory inputs
@ -607,3 +609,15 @@ func NewSpecGeneratorWithRootfs(rootfs string) *SpecGenerator {
csc := ContainerStorageConfig{Rootfs: rootfs}
return &SpecGenerator{ContainerStorageConfig: csc}
}
func StringSlicesEqual(a, b []string) bool {
if len(a) != len(b) {
return false
}
for i, v := range a {
if v != b[i] {
return false
}
}
return true
}

View File

@ -55,8 +55,6 @@ type ImageVolume struct {
// GenVolumeMounts parses user input into mounts, volumes and overlay volumes
func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*NamedVolume, map[string]*OverlayVolume, error) {
errDuplicateDest := errors.New("duplicate mount destination")
mounts := make(map[string]spec.Mount)
volumes := make(map[string]*NamedVolume)
overlayVolumes := make(map[string]*OverlayVolume)
@ -153,8 +151,12 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na
newOverlayVol.Source = source
newOverlayVol.Options = options
if _, ok := overlayVolumes[newOverlayVol.Destination]; ok {
return nil, nil, nil, fmt.Errorf("%v: %w", newOverlayVol.Destination, errDuplicateDest)
if vol, ok := overlayVolumes[newOverlayVol.Destination]; ok {
if vol.Source == newOverlayVol.Source &&
StringSlicesEqual(vol.Options, newOverlayVol.Options) {
continue
}
return nil, nil, nil, fmt.Errorf("%v: %w", newOverlayVol.Destination, ErrDuplicateDest)
}
overlayVolumes[newOverlayVol.Destination] = newOverlayVol
} else {
@ -164,8 +166,13 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na
Source: src,
Options: options,
}
if _, ok := mounts[newMount.Destination]; ok {
return nil, nil, nil, fmt.Errorf("%v: %w", newMount.Destination, errDuplicateDest)
if vol, ok := mounts[newMount.Destination]; ok {
if vol.Source == newMount.Source &&
StringSlicesEqual(vol.Options, newMount.Options) {
continue
}
return nil, nil, nil, fmt.Errorf("%v: %w", newMount.Destination, ErrDuplicateDest)
}
mounts[newMount.Destination] = newMount
}
@ -176,8 +183,11 @@ func GenVolumeMounts(volumeFlag []string) (map[string]spec.Mount, map[string]*Na
newNamedVol.Dest = dest
newNamedVol.Options = options
if _, ok := volumes[newNamedVol.Dest]; ok {
return nil, nil, nil, fmt.Errorf("%v: %w", newNamedVol.Dest, errDuplicateDest)
if vol, ok := volumes[newNamedVol.Dest]; ok {
if vol.Name == newNamedVol.Name {
continue
}
return nil, nil, nil, fmt.Errorf("%v: %w", newNamedVol.Dest, ErrDuplicateDest)
}
volumes[newNamedVol.Dest] = newNamedVol
}

View File

@ -15,7 +15,6 @@ import (
)
var (
errDuplicateDest = errors.New("duplicate mount destination")
errOptionArg = errors.New("must provide an argument for option")
errNoDest = errors.New("must set volume destination")
errInvalidSyntax = errors.New("incorrect mount format: should be --mount type=<bind|tmpfs|volume>,[src=<host-dir|volume-name>,]target=<ctr-dir>[,options]")
@ -49,21 +48,32 @@ func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bo
// Unify mounts from --mount, --volume, --tmpfs.
// Start with --volume.
for dest, mount := range volumeMounts {
if _, ok := unifiedMounts[dest]; ok {
return nil, nil, nil, nil, fmt.Errorf("%v: %w", dest, errDuplicateDest)
if vol, ok := unifiedMounts[dest]; ok {
if mount.Source == vol.Source &&
specgen.StringSlicesEqual(vol.Options, mount.Options) {
continue
}
return nil, nil, nil, nil, fmt.Errorf("%v: %w", dest, specgen.ErrDuplicateDest)
}
unifiedMounts[dest] = mount
}
for dest, volume := range volumeVolumes {
if _, ok := unifiedVolumes[dest]; ok {
return nil, nil, nil, nil, fmt.Errorf("%v: %w", dest, errDuplicateDest)
if vol, ok := unifiedVolumes[dest]; ok {
if volume.Name == vol.Name &&
specgen.StringSlicesEqual(vol.Options, volume.Options) {
continue
}
return nil, nil, nil, nil, fmt.Errorf("%v: %w", dest, specgen.ErrDuplicateDest)
}
unifiedVolumes[dest] = volume
}
// Now --tmpfs
for dest, tmpfs := range tmpfsMounts {
if _, ok := unifiedMounts[dest]; ok {
return nil, nil, nil, nil, fmt.Errorf("%v: %w", dest, errDuplicateDest)
if vol, ok := unifiedMounts[dest]; ok {
if vol.Type != define.TypeTmpfs {
return nil, nil, nil, nil, fmt.Errorf("%v: %w", dest, specgen.ErrDuplicateDest)
}
continue
}
unifiedMounts[dest] = tmpfs
}
@ -93,7 +103,7 @@ func parseVolumes(volumeFlag, mountFlag, tmpfsFlag []string, addReadOnlyTmpfs bo
allMounts := make(map[string]bool)
testAndSet := func(dest string) error {
if _, ok := allMounts[dest]; ok {
return fmt.Errorf("conflict at mount destination %v: %w", dest, errDuplicateDest)
return fmt.Errorf("%v: %w", dest, specgen.ErrDuplicateDest)
}
allMounts[dest] = true
return nil
@ -199,7 +209,7 @@ func Mounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.Name
return nil, nil, nil, err
}
if _, ok := finalMounts[mount.Destination]; ok {
return nil, nil, nil, fmt.Errorf("%v: %w", mount.Destination, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("%v: %w", mount.Destination, specgen.ErrDuplicateDest)
}
finalMounts[mount.Destination] = mount
case define.TypeTmpfs:
@ -208,7 +218,7 @@ func Mounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.Name
return nil, nil, nil, err
}
if _, ok := finalMounts[mount.Destination]; ok {
return nil, nil, nil, fmt.Errorf("%v: %w", mount.Destination, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("%v: %w", mount.Destination, specgen.ErrDuplicateDest)
}
finalMounts[mount.Destination] = mount
case define.TypeDevpts:
@ -217,7 +227,7 @@ func Mounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.Name
return nil, nil, nil, err
}
if _, ok := finalMounts[mount.Destination]; ok {
return nil, nil, nil, fmt.Errorf("%v: %w", mount.Destination, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("%v: %w", mount.Destination, specgen.ErrDuplicateDest)
}
finalMounts[mount.Destination] = mount
case "image":
@ -226,7 +236,7 @@ func Mounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.Name
return nil, nil, nil, err
}
if _, ok := finalImageVolumes[volume.Destination]; ok {
return nil, nil, nil, fmt.Errorf("%v: %w", volume.Destination, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("%v: %w", volume.Destination, specgen.ErrDuplicateDest)
}
finalImageVolumes[volume.Destination] = volume
case "volume":
@ -235,7 +245,7 @@ func Mounts(mountFlag []string) (map[string]spec.Mount, map[string]*specgen.Name
return nil, nil, nil, err
}
if _, ok := finalNamedVolumes[volume.Dest]; ok {
return nil, nil, nil, fmt.Errorf("%v: %w", volume.Dest, errDuplicateDest)
return nil, nil, nil, fmt.Errorf("%v: %w", volume.Dest, specgen.ErrDuplicateDest)
}
finalNamedVolumes[volume.Dest] = volume
default:
@ -665,10 +675,12 @@ func getTmpfsMounts(tmpfsFlag []string) (map[string]spec.Mount, error) {
options = strings.Split(spliti[1], ",")
}
if _, ok := m[destPath]; ok {
return nil, fmt.Errorf("%v: %w", destPath, errDuplicateDest)
if vol, ok := m[destPath]; ok {
if specgen.StringSlicesEqual(vol.Options, options) {
continue
}
return nil, fmt.Errorf("%v: %w", destPath, specgen.ErrDuplicateDest)
}
mount := spec.Mount{
Destination: unixPathClean(destPath),
Type: define.TypeTmpfs,

View File

@ -64,6 +64,33 @@ function teardown() {
}
@test "podman volume duplicates" {
vol1=${PODMAN_TMPDIR}/v1_$(random_string)
vol2=${PODMAN_TMPDIR}/v2_$(random_string)
mkdir $vol1 $vol2
# if volumes source and dest match then pass
run_podman run --rm -v $vol1:/vol1 -v $vol1:/vol1 $IMAGE /bin/true
run_podman 125 run --rm -v $vol1:$vol1 -v $vol2:$vol1 $IMAGE /bin/true
is "$output" "Error: $vol1: duplicate mount destination" "diff volumes mounted on same dest should fail"
# if named volumes source and dest match then pass
run_podman run --rm -v vol1:/vol1 -v vol1:/vol1 $IMAGE /bin/true
run_podman 125 run --rm -v vol1:/vol1 -v vol2:/vol1 $IMAGE /bin/true
is "$output" "Error: /vol1: duplicate mount destination" "diff named volumes mounted on same dest should fail"
# if tmpfs volumes source and dest match then pass
run_podman run --rm --tmpfs /vol1 --tmpfs /vol1 $IMAGE /bin/true
run_podman 125 run --rm --tmpfs $vol1 --tmpfs $vol1:ro $IMAGE /bin/true
is "$output" "Error: $vol1: duplicate mount destination" "diff named volumes and tmpfs mounted on same dest should fail"
run_podman 125 run --rm -v vol2:/vol2 --tmpfs /vol2 $IMAGE /bin/true
is "$output" "Error: /vol2: duplicate mount destination" "diff named volumes and tmpfs mounted on same dest should fail"
run_podman 125 run --rm -v $vol1:/vol1 --tmpfs /vol1 $IMAGE /bin/true
is "$output" "Error: /vol1: duplicate mount destination" "diff named volumes and tmpfs mounted on same dest should fail"
}
# Filter volumes by name
@test "podman volume filter --name" {
suffix=$(random_string)