diff options
-rw-r--r-- | pkg/spec/storage.go | 73 | ||||
-rw-r--r-- | pkg/spec/storage_test.go | 2 |
2 files changed, 47 insertions, 28 deletions
diff --git a/pkg/spec/storage.go b/pkg/spec/storage.go index 6f8398513..9660c278f 100644 --- a/pkg/spec/storage.go +++ b/pkg/spec/storage.go @@ -79,6 +79,7 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount, } // Unify mounts from --mount, --volume, --tmpfs. + // Also add mounts + volumes directly from createconfig. // Start with --volume. for dest, mount := range volumeMounts { if _, ok := unifiedMounts[dest]; ok { @@ -99,8 +100,21 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount, } unifiedMounts[dest] = tmpfs } - - // TODO: Check for conflicts between volumes and named volumes. + // Now spec mounts and volumes + for _, mount := range config.Mounts { + dest := mount.Destination + if _, ok := unifiedMounts[dest]; ok { + return nil, nil, errors.Wrapf(errDuplicateDest, dest) + } + unifiedMounts[dest] = mount + } + for _, volume := range config.NamedVolumes { + dest := volume.Dest + if _, ok := unifiedVolumes[dest]; ok { + return nil, nil, errors.Wrapf(errDuplicateDest, dest) + } + unifiedVolumes[dest] = volume + } // If requested, add container init binary if config.Init { @@ -131,9 +145,17 @@ func (config *CreateConfig) parseVolumes(runtime *libpod.Runtime) ([]spec.Mount, baseVolumes[dest] = volume } - // TODO: Allow an explicit list of named volumes and spec mounts in the - // createconfig. Podman doesn't need them, but other libpod consumers - // might want them. + // Check for conflicts between named volumes and mounts + for dest := range unifiedMounts { + if _, ok := unifiedVolumes[dest]; ok { + return nil, nil, errors.Wrapf(errDuplicateDest, "conflict at mount destination %v", dest) + } + } + for dest := range unifiedVolumes { + if _, ok := unifiedMounts[dest]; ok { + return nil, nil, errors.Wrapf(errDuplicateDest, "conflict at mount destination %v", dest) + } + } // Final step: maps to arrays finalMounts := make([]spec.Mount, 0, len(baseMounts)) @@ -164,19 +186,11 @@ func (config *CreateConfig) getVolumesFrom(runtime *libpod.Runtime) (map[string] // Both of these are maps of mount destination to mount type. // We ensure that each destination is only mounted to once in this way. - // We tiebreak only within each category; we'll let libpod tiebreak - // between mounts and named volumes. - // TODO: Should we unmount subtree mounts? E.g., if /tmp/ is mounted by - // one mount, and we already have /tmp/a and /tmp/b, should we remove - // the /tmp/a and /tmp/b mounts in favor of the more general /tmp? finalMounts := make(map[string]spec.Mount) finalNamedVolumes := make(map[string]*libpod.ContainerNamedVolume) for _, vol := range config.VolumesFrom { options := []string{} - // TODO: now that container names may contain colons this is no - // longer safe. - // I don't know if there's much we can do here... splitVol := strings.SplitN(vol, ":", 2) if len(splitVol) == 2 { if strings.Contains(splitVol[1], "Z") || @@ -185,10 +199,10 @@ func (config *CreateConfig) getVolumesFrom(runtime *libpod.Runtime) (map[string] strings.Contains(splitVol[1], "shared") { return nil, nil, errors.Errorf("invalid options %q, can only specify 'ro', 'rw', and 'z", splitVol[1]) } - if err := ValidateVolumeOpts(splitVol[1]); err != nil { + options = strings.Split(splitVol[1], ",") + if err := ValidateVolumeOpts(options); err != nil { return nil, nil, err } - options = strings.Split(splitVol[1], ",") } ctr, err := runtime.LookupContainer(splitVol[0]) if err != nil { @@ -339,14 +353,12 @@ func getBindMount(args []string) (spec.Mount, error) { kv := strings.Split(val, "=") switch kv[0] { case "ro", "nosuid", "nodev", "noexec": - // TODO: detect duplication of these options + // TODO: detect duplication of these options. + // (Is this necessary?) newMount.Options = append(newMount.Options, kv[0]) case "shared", "rshared", "private", "rprivate", "slave", "rslave", "Z", "z": - // TODO: detect duplicate mount propagation options newMount.Options = append(newMount.Options, kv[0]) case "bind-propagation": - // TODO: this should conflict with the mount propagation - // options above. if len(kv) == 1 { return newMount, errors.Wrapf(optionArgError, kv[0]) } @@ -382,6 +394,10 @@ func getBindMount(args []string) (spec.Mount, error) { newMount.Source = newMount.Destination } + if err := ValidateVolumeOpts(newMount.Options); err != nil { + return newMount, err + } + return newMount, nil } @@ -509,28 +525,27 @@ func ValidateVolumeCtrDir(ctrDir string) error { } // ValidateVolumeOpts validates a volume's options -func ValidateVolumeOpts(option string) error { +func ValidateVolumeOpts(options []string) error { var foundRootPropagation, foundRWRO, foundLabelChange int - options := strings.Split(option, ",") for _, opt := range options { switch opt { case "rw", "ro": foundRWRO++ if foundRWRO > 1 { - return errors.Errorf("invalid options %q, can only specify 1 'rw' or 'ro' option", option) + return errors.Errorf("invalid options %q, can only specify 1 'rw' or 'ro' option", strings.Join(options, ", ")) } case "z", "Z": foundLabelChange++ if foundLabelChange > 1 { - return errors.Errorf("invalid options %q, can only specify 1 'z' or 'Z' option", option) + return errors.Errorf("invalid options %q, can only specify 1 'z' or 'Z' option", strings.Join(options, ", ")) } case "private", "rprivate", "shared", "rshared", "slave", "rslave": foundRootPropagation++ if foundRootPropagation > 1 { - return errors.Errorf("invalid options %q, can only specify 1 '[r]shared', '[r]private' or '[r]slave' option", option) + return errors.Errorf("invalid options %q, can only specify 1 '[r]shared', '[r]private' or '[r]slave' option", strings.Join(options, ", ")) } default: - return errors.Errorf("invalid option type %q", option) + return errors.Errorf("invalid option type %q", opt) } } return nil @@ -562,10 +577,10 @@ func (config *CreateConfig) getVolumeMounts() (map[string]spec.Mount, map[string dest = splitVol[1] } if len(splitVol) > 2 { - if err := ValidateVolumeOpts(splitVol[2]); err != nil { + options = strings.Split(splitVol[2], ",") + if err := ValidateVolumeOpts(options); err != nil { return nil, nil, err } - options = strings.Split(splitVol[2], ",") } if err := ValidateVolumeHostDir(src); err != nil { @@ -724,6 +739,10 @@ func processOptions(options []string) []string { return options } +// Supercede existing mounts in the spec with new, user-specified mounts. +// TODO: Should we unmount subtree mounts? E.g., if /tmp/ is mounted by +// one mount, and we already have /tmp/a and /tmp/b, should we remove +// the /tmp/a and /tmp/b mounts in favor of the more general /tmp? func supercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.Mount { if len(mounts) > 0 { // If we have overlappings mounts, remove them from the spec in favor of diff --git a/pkg/spec/storage_test.go b/pkg/spec/storage_test.go index 4d96a2de6..2439efb6f 100644 --- a/pkg/spec/storage_test.go +++ b/pkg/spec/storage_test.go @@ -13,7 +13,7 @@ func TestGetVolumeMountsOneVolume(t *testing.T) { Destination: "/foobar", Type: "bind", Source: "/tmp", - Options: []string{"ro", "rbind", "rprivate"}, + Options: []string{"ro"}, } config := CreateConfig{ Volumes: []string{"/tmp:/foobar:ro"}, |