diff options
-rw-r--r-- | docs/podman-volume-rm.1.md | 3 | ||||
-rw-r--r-- | libpod/boltdb_state.go | 79 | ||||
-rw-r--r-- | libpod/boltdb_state_internal.go | 53 | ||||
-rw-r--r-- | libpod/container.go | 33 | ||||
-rw-r--r-- | libpod/container_internal.go | 16 | ||||
-rw-r--r-- | libpod/container_internal_linux.go | 18 | ||||
-rw-r--r-- | libpod/in_memory_state.go | 32 | ||||
-rw-r--r-- | libpod/options.go | 92 | ||||
-rw-r--r-- | libpod/runtime_ctr.go | 67 | ||||
-rw-r--r-- | libpod/runtime_volume_linux.go | 24 | ||||
-rw-r--r-- | libpod/state.go | 4 | ||||
-rw-r--r-- | pkg/spec/createconfig.go | 127 | ||||
-rw-r--r-- | pkg/spec/spec.go | 40 | ||||
-rw-r--r-- | test/e2e/commit_test.go | 2 | ||||
-rw-r--r-- | test/e2e/run_test.go | 1 | ||||
-rw-r--r-- | test/e2e/volume_rm_test.go | 4 |
16 files changed, 322 insertions, 273 deletions
diff --git a/docs/podman-volume-rm.1.md b/docs/podman-volume-rm.1.md index c23d7675c..8c3765235 100644 --- a/docs/podman-volume-rm.1.md +++ b/docs/podman-volume-rm.1.md @@ -21,7 +21,8 @@ Remove all volumes. **-f**, **--force**="" -Remove a volume by force, even if it is being used by a container +Remove a volume by force. +If it is being used by containers, the containers will be removed first. **--help** diff --git a/libpod/boltdb_state.go b/libpod/boltdb_state.go index 92a7b1538..d8cfa2bda 100644 --- a/libpod/boltdb_state.go +++ b/libpod/boltdb_state.go @@ -1358,56 +1358,6 @@ func (s *BoltState) AddVolume(volume *Volume) error { return err } -// RemoveVolCtrDep updates the container dependencies sub bucket of the given volume. -// It deletes it from the bucket when found. -// This is important when force removing a volume and we want to get rid of the dependencies. -func (s *BoltState) RemoveVolCtrDep(volume *Volume, ctrID string) error { - if ctrID == "" { - return nil - } - - if !s.valid { - return ErrDBBadConfig - } - - if !volume.valid { - return ErrVolumeRemoved - } - - volName := []byte(volume.Name()) - - db, err := s.getDBCon() - if err != nil { - return err - } - defer s.closeDBCon(db) - - err = db.Update(func(tx *bolt.Tx) error { - volBkt, err := getVolBucket(tx) - if err != nil { - return err - } - - volDB := volBkt.Bucket(volName) - if volDB == nil { - volume.valid = false - return errors.Wrapf(ErrNoSuchVolume, "no volume with name %s found in database", volume.Name()) - } - - // Make a subbucket for the containers using the volume - ctrDepsBkt := volDB.Bucket(volDependenciesBkt) - depCtrID := []byte(ctrID) - if depExists := ctrDepsBkt.Get(depCtrID); depExists != nil { - if err := ctrDepsBkt.Delete(depCtrID); err != nil { - return errors.Wrapf(err, "error deleting container dependencies %q for volume %s in ctrDependencies bucket in DB", ctrID, volume.Name()) - } - } - - return nil - }) - return err -} - // RemoveVolume removes the given volume from the state func (s *BoltState) RemoveVolume(volume *Volume) error { if !s.valid { @@ -1433,6 +1383,11 @@ func (s *BoltState) RemoveVolume(volume *Volume) error { return err } + ctrBkt, err := getCtrBucket(tx) + if err != nil { + return err + } + // Check if the volume exists volDB := volBkt.Bucket(volName) if volDB == nil { @@ -1448,6 +1403,18 @@ func (s *BoltState) RemoveVolume(volume *Volume) error { if volCtrsBkt != nil { var deps []string err = volCtrsBkt.ForEach(func(id, value []byte) error { + // Alright, this is ugly. + // But we need it to work around the change in + // volume dependency handling, to make sure that + // older Podman versions don't cause DB + // corruption. + // Look up all dependencies and see that they + // still exist before appending. + ctrExists := ctrBkt.Bucket(id) + if ctrExists == nil { + return nil + } + deps = append(deps, string(id)) return nil }) @@ -1629,6 +1596,11 @@ func (s *BoltState) VolumeInUse(volume *Volume) ([]string, error) { return err } + ctrBucket, err := getCtrBucket(tx) + if err != nil { + return err + } + volDB := volBucket.Bucket([]byte(volume.Name())) if volDB == nil { volume.valid = false @@ -1642,6 +1614,13 @@ func (s *BoltState) VolumeInUse(volume *Volume) ([]string, error) { // Iterate through and add dependencies err = dependsBkt.ForEach(func(id, value []byte) error { + // Look up all dependencies and see that they + // still exist before appending. + ctrExists := ctrBucket.Bucket(id) + if ctrExists == nil { + return nil + } + depCtrs = append(depCtrs, string(id)) return nil diff --git a/libpod/boltdb_state_internal.go b/libpod/boltdb_state_internal.go index b6a0759b1..a6900a6d3 100644 --- a/libpod/boltdb_state_internal.go +++ b/libpod/boltdb_state_internal.go @@ -564,23 +564,17 @@ func (s *BoltState) addContainer(ctr *Container, pod *Pod) error { } } - // Add container to volume dependencies bucket if container is using a named volume - if ctr.runtime.config.VolumePath == "" { - return nil - } - for _, vol := range ctr.config.Spec.Mounts { - if strings.Contains(vol.Source, ctr.runtime.config.VolumePath) { - volName := strings.Split(vol.Source[len(ctr.runtime.config.VolumePath)+1:], "/")[0] - volDB := volBkt.Bucket([]byte(volName)) - if volDB == nil { - return errors.Wrapf(ErrNoSuchVolume, "no volume with name %s found in database", volName) - } + // Add container to named volume dependencies buckets + for _, vol := range ctr.config.NamedVolumes { + volDB := volBkt.Bucket([]byte(vol.Name)) + if volDB == nil { + return errors.Wrapf(ErrNoSuchVolume, "no volume with name %s found in database when adding container %s", vol.Name, ctr.ID()) + } - ctrDepsBkt := volDB.Bucket(volDependenciesBkt) - if depExists := ctrDepsBkt.Get(ctrID); depExists == nil { - if err := ctrDepsBkt.Put(ctrID, ctrID); err != nil { - return errors.Wrapf(err, "error storing container dependencies %q for volume %s in ctrDependencies bucket in DB", ctr.ID(), volName) - } + ctrDepsBkt := volDB.Bucket(volDependenciesBkt) + if depExists := ctrDepsBkt.Get(ctrID); depExists == nil { + if err := ctrDepsBkt.Put(ctrID, ctrID); err != nil { + return errors.Wrapf(err, "error adding container %s to volume %s dependencies", ctr.ID(), vol.Name) } } } @@ -745,22 +739,19 @@ func (s *BoltState) removeContainer(ctr *Container, pod *Pod, tx *bolt.Tx) error } } - // Remove container from volume dependencies bucket if container is using a named volume - for _, vol := range ctr.config.Spec.Mounts { - if strings.Contains(vol.Source, ctr.runtime.config.VolumePath) { - volName := strings.Split(vol.Source[len(ctr.runtime.config.VolumePath)+1:], "/")[0] - - volDB := volBkt.Bucket([]byte(volName)) - if volDB == nil { - // Let's assume the volume was already deleted and continue to remove the container - continue - } + // Remove container from named volume dependencies buckets + for _, vol := range ctr.config.NamedVolumes { + volDB := volBkt.Bucket([]byte(vol.Name)) + if volDB == nil { + // Let's assume the volume was already deleted and + // continue to remove the container + continue + } - ctrDepsBkt := volDB.Bucket(volDependenciesBkt) - if depExists := ctrDepsBkt.Get(ctrID); depExists != nil { - if err := ctrDepsBkt.Delete(ctrID); err != nil { - return errors.Wrapf(err, "error deleting container dependencies %q for volume %s in ctrDependencies bucket in DB", ctr.ID(), volName) - } + ctrDepsBkt := volDB.Bucket(volDependenciesBkt) + if depExists := ctrDepsBkt.Get(ctrID); depExists == nil { + if err := ctrDepsBkt.Delete(ctrID); err != nil { + return errors.Wrapf(err, "error deleting container %s dependency on volume %s", ctr.ID(), vol.Name) } } } diff --git a/libpod/container.go b/libpod/container.go index 739406e42..6d5e063ab 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -234,6 +234,8 @@ type ContainerConfig struct { // These include the SHM mount. // These must be unmounted before the container's rootfs is unmounted. Mounts []string `json:"mounts,omitempty"` + // NamedVolumes lists the named volumes to mount into the container. + NamedVolumes []*ContainerNamedVolume `json:"namedVolumes,omitempty"` // Security Config @@ -354,9 +356,6 @@ type ContainerConfig struct { // ExitCommand is the container's exit command. // This Command will be executed when the container exits ExitCommand []string `json:"exitCommand,omitempty"` - // LocalVolumes are the built-in volumes we get from the --volumes-from flag - // It picks up the built-in volumes of the container used by --volumes-from - LocalVolumes []spec.Mount // IsInfra is a bool indicating whether this container is an infra container used for // sharing kernel namespaces in a pod IsInfra bool `json:"pause"` @@ -368,6 +367,18 @@ type ContainerConfig struct { HealthCheckConfig *manifest.Schema2HealthConfig `json:"healthcheck"` } +// ContainerNamedVolume is a named volume that will be mounted into the +// container. Each named volume is a libpod Volume present in the state. +type ContainerNamedVolume struct { + // Name is the name of the volume to mount in. + // Must resolve to a valid volume present in this Podman. + Name string `json:"volumeName"` + // Dest is the mount's destination + Dest string `json:"dest"` + // Options are fstab style mount options + Options []string `json:"options,omitempty"` +} + // ContainerStatus returns a string representation for users // of a container state func (t ContainerStatus) String() string { @@ -488,6 +499,22 @@ func (c *Container) StaticDir() string { return c.config.StaticDir } +// NamedVolumes returns the container's named volumes. +// The name of each is guaranteed to point to a valid libpod Volume present in +// the state. +func (c *Container) NamedVolumes() []*ContainerNamedVolume { + volumes := []*ContainerNamedVolume{} + for _, vol := range c.config.NamedVolumes { + newVol := new(ContainerNamedVolume) + newVol.Name = vol.Name + newVol.Dest = vol.Dest + newVol.Options = vol.Options + volumes = append(volumes, newVol) + } + + return volumes +} + // Privileged returns whether the container is privileged func (c *Container) Privileged() bool { return c.config.Privileged diff --git a/libpod/container_internal.go b/libpod/container_internal.go index daa32007a..22df36c11 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -1403,22 +1403,6 @@ func getExcludedCGroups() (excludes []string) { return } -// namedVolumes returns named volumes for the container -func (c *Container) namedVolumes() ([]string, error) { - var volumes []string - for _, vol := range c.config.Spec.Mounts { - if strings.HasPrefix(vol.Source, c.runtime.config.VolumePath) { - volume := strings.TrimPrefix(vol.Source, c.runtime.config.VolumePath+"/") - split := strings.Split(volume, "/") - volume = split[0] - if _, err := c.runtime.state.Volume(volume); err == nil { - volumes = append(volumes, volume) - } - } - } - return volumes, nil -} - // this should be from chrootarchive. func (c *Container) copyWithTarFromImage(src, dest string) error { mountpoint, err := c.mount() diff --git a/libpod/container_internal_linux.go b/libpod/container_internal_linux.go index 504d6c135..4d6bf61a3 100644 --- a/libpod/container_internal_linux.go +++ b/libpod/container_internal_linux.go @@ -195,6 +195,7 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { if err := c.makeBindMounts(); err != nil { return nil, err } + // Check if the spec file mounts contain the label Relabel flags z or Z. // If they do, relabel the source directory and then remove the option. for i := range g.Config.Mounts { @@ -218,6 +219,23 @@ func (c *Container) generateSpec(ctx context.Context) (*spec.Spec, error) { g.SetProcessSelinuxLabel(c.ProcessLabel()) g.SetLinuxMountLabel(c.MountLabel()) + + // Add named volumes + for _, namedVol := range c.config.NamedVolumes { + volume, err := c.runtime.GetVolume(namedVol.Name) + if err != nil { + return nil, errors.Wrapf(err, "error retrieving volume %s to add to container %s", namedVol.Name, c.ID()) + } + mountPoint := volume.MountPoint() + volMount := spec.Mount{ + Type: "bind", + Source: mountPoint, + Destination: namedVol.Dest, + Options: namedVol.Options, + } + g.AddMount(volMount) + } + // Add bind mounts to container for dstPath, srcPath := range c.state.BindMounts { newMount := spec.Mount{ diff --git a/libpod/in_memory_state.go b/libpod/in_memory_state.go index ab4fc8ba7..2669206df 100644 --- a/libpod/in_memory_state.go +++ b/libpod/in_memory_state.go @@ -249,11 +249,8 @@ func (s *InMemoryState) AddContainer(ctr *Container) error { } // Add container to volume dependencies - for _, vol := range ctr.config.Spec.Mounts { - if strings.Contains(vol.Source, ctr.runtime.config.VolumePath) { - volName := strings.Split(vol.Source[len(ctr.runtime.config.VolumePath)+1:], "/")[0] - s.addCtrToVolDependsMap(ctr.ID(), volName) - } + for _, vol := range ctr.config.NamedVolumes { + s.addCtrToVolDependsMap(ctr.ID(), vol.Name) } return nil @@ -306,12 +303,9 @@ func (s *InMemoryState) RemoveContainer(ctr *Container) error { s.removeCtrFromDependsMap(ctr.ID(), depCtr) } - // Remove container from volume dependencies - for _, vol := range ctr.config.Spec.Mounts { - if strings.Contains(vol.Source, ctr.runtime.config.VolumePath) { - volName := strings.Split(vol.Source[len(ctr.runtime.config.VolumePath)+1:], "/")[0] - s.removeCtrFromVolDependsMap(ctr.ID(), volName) - } + // Remove this container from volume dependencies + for _, vol := range ctr.config.NamedVolumes { + s.removeCtrFromVolDependsMap(ctr.ID(), vol.Name) } return nil @@ -492,22 +486,6 @@ func (s *InMemoryState) RemoveVolume(volume *Volume) error { return nil } -// RemoveVolCtrDep updates the container dependencies of the volume -func (s *InMemoryState) RemoveVolCtrDep(volume *Volume, ctrID string) error { - if !volume.valid { - return errors.Wrapf(ErrVolumeRemoved, "volume with name %s is not valid", volume.Name()) - } - - if _, ok := s.volumes[volume.Name()]; !ok { - return errors.Wrapf(ErrNoSuchVolume, "volume with name %s doesn't exists in state", volume.Name()) - } - - // Remove container that is using this volume - s.removeCtrFromVolDependsMap(ctrID, volume.Name()) - - return nil -} - // VolumeInUse checks if the given volume is being used by at least one container func (s *InMemoryState) VolumeInUse(volume *Volume) ([]string, error) { if !volume.valid { diff --git a/libpod/options.go b/libpod/options.go index 24f126e66..9326e54e4 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -13,7 +13,6 @@ import ( "github.com/containers/storage" "github.com/containers/storage/pkg/idtools" "github.com/cri-o/ocicni/pkg/ocicni" - spec "github.com/opencontainers/runtime-spec/specs-go" "github.com/pkg/errors" ) @@ -1111,24 +1110,6 @@ func WithUserVolumes(volumes []string) CtrCreateOption { } } -// WithLocalVolumes sets the built-in volumes of the container retrieved -// from a container passed in to the --volumes-from flag. -// This stores the built-in volume information in the Config so we can -// add them when creating the container. -func WithLocalVolumes(volumes []spec.Mount) CtrCreateOption { - return func(ctr *Container) error { - if ctr.valid { - return ErrCtrFinalized - } - - if volumes != nil { - ctr.config.LocalVolumes = append(ctr.config.LocalVolumes, volumes...) - } - - return nil - } -} - // WithEntrypoint sets the entrypoint of the container. // This is not used to change the container's spec, but will instead be used // during commit to populate the entrypoint of the new image. @@ -1255,6 +1236,35 @@ func withIsInfra() CtrCreateOption { } } +// WithNamedVolumes adds the given named volumes to the container. +func WithNamedVolumes(volumes []*ContainerNamedVolume) CtrCreateOption { + return func(ctr *Container) error { + if ctr.valid { + return ErrCtrFinalized + } + + destinations := make(map[string]bool) + + for _, vol := range volumes { + // Don't check if they already exist. + // If they don't we will automatically create them. + + if _, ok := destinations[vol.Dest]; ok { + return errors.Wrapf(ErrInvalidArg, "two volumes found with destination %s", vol.Dest) + } + destinations[vol.Dest] = true + + ctr.config.NamedVolumes = append(ctr.config.NamedVolumes, &ContainerNamedVolume{ + Name: vol.Name, + Dest: vol.Dest, + Options: vol.Options, + }) + } + + return nil + } +} + // Volume Creation Options // WithVolumeName sets the name of the volume. @@ -1274,68 +1284,72 @@ func WithVolumeName(name string) VolumeCreateOption { } } -// WithVolumeUID sets the uid of the owner. -func WithVolumeUID(uid int) VolumeCreateOption { +// WithVolumeLabels sets the labels of the volume. +func WithVolumeLabels(labels map[string]string) VolumeCreateOption { return func(volume *Volume) error { if volume.valid { return ErrVolumeFinalized } - volume.config.UID = uid + + volume.config.Labels = make(map[string]string) + for key, value := range labels { + volume.config.Labels[key] = value + } + return nil } } -// WithVolumeGID sets the gid of the owner. -func WithVolumeGID(gid int) VolumeCreateOption { +// WithVolumeDriver sets the driver of the volume. +func WithVolumeDriver(driver string) VolumeCreateOption { return func(volume *Volume) error { if volume.valid { return ErrVolumeFinalized } - volume.config.GID = gid + + volume.config.Driver = driver + return nil } } -// WithVolumeLabels sets the labels of the volume. -func WithVolumeLabels(labels map[string]string) VolumeCreateOption { +// WithVolumeOptions sets the options of the volume. +func WithVolumeOptions(options map[string]string) VolumeCreateOption { return func(volume *Volume) error { if volume.valid { return ErrVolumeFinalized } - volume.config.Labels = make(map[string]string) - for key, value := range labels { - volume.config.Labels[key] = value + volume.config.Options = make(map[string]string) + for key, value := range options { + volume.config.Options[key] = value } return nil } } -// WithVolumeDriver sets the driver of the volume. -func WithVolumeDriver(driver string) VolumeCreateOption { +// WithVolumeUID sets the UID that the volume will be created as. +func WithVolumeUID(uid int) VolumeCreateOption { return func(volume *Volume) error { if volume.valid { return ErrVolumeFinalized } - volume.config.Driver = driver + volume.config.UID = uid return nil } } -// WithVolumeOptions sets the options of the volume. -func WithVolumeOptions(options map[string]string) VolumeCreateOption { +// WithVolumeGID sets the GID that the volume will be created as. +func WithVolumeGID(gid int) VolumeCreateOption { return func(volume *Volume) error { if volume.valid { return ErrVolumeFinalized } - volume.config.Options = make(map[string]string) - for key, value := range options { - volume.config.Options[key] = value - } + volume.config.GID = gid return nil } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index da2399685..800b42851 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -99,9 +99,6 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options .. ctr.state.State = ContainerStateConfigured ctr.runtime = r - ctr.valid = true - ctr.state.State = ContainerStateConfigured - var pod *Pod if ctr.config.Pod != "" { // Get the pod from state @@ -173,24 +170,29 @@ func (r *Runtime) newContainer(ctx context.Context, rSpec *spec.Spec, options .. ctr.config.ConmonPidFile = filepath.Join(ctr.config.StaticDir, "conmon.pid") } - // Go through the volume mounts and check for named volumes - // If the named volme already exists continue, otherwise create - // the storage for the named volume. - for i, vol := range ctr.config.Spec.Mounts { - if vol.Source[0] != '/' && isNamedVolume(vol.Source) { - volInfo, err := r.state.Volume(vol.Source) - if err != nil { - newVol, err := r.newVolume(ctx, WithVolumeName(vol.Source), withSetCtrSpecific(), WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())) - if err != nil { - return nil, errors.Wrapf(err, "error creating named volume %q", vol.Source) - } - ctr.config.Spec.Mounts[i].Source = newVol.MountPoint() - if err := ctr.copyWithTarFromImage(ctr.config.Spec.Mounts[i].Destination, ctr.config.Spec.Mounts[i].Source); err != nil && !os.IsNotExist(err) { - return nil, errors.Wrapf(err, "failed to copy content into new volume mount %q", vol.Source) - } - continue - } - ctr.config.Spec.Mounts[i].Source = volInfo.MountPoint() + // Go through named volumes and add them. + // If they don't exist they will be created using basic options. + for _, vol := range ctr.config.NamedVolumes { + // Check if it exists already + _, err := r.state.Volume(vol.Name) + if err == nil { + // The volume exists, we're good + continue + } else if errors.Cause(err) != ErrNoSuchVolume { + return nil, errors.Wrapf(err, "error retrieving named volume %s for new container", vol.Name) + } + + logrus.Debugf("Creating new volume %s for container", vol.Name) + + // The volume does not exist, so we need to create it. + newVol, err := r.newVolume(ctx, WithVolumeName(vol.Name), withSetCtrSpecific(), + WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())) + if err != nil { + return nil, errors.Wrapf(err, "error creating named volume %q", vol.Name) + } + + if err := ctr.copyWithTarFromImage(vol.Dest, newVol.MountPoint()); err != nil && !os.IsNotExist(err) { + return nil, errors.Wrapf(err, "Failed to copy content into new volume mount %q", vol.Name) } } @@ -344,13 +346,6 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, return errors.Wrapf(ErrCtrExists, "container %s has dependent containers which must be removed before it: %s", c.ID(), depsStr) } - var volumes []string - if removeVolume { - volumes, err = c.namedVolumes() - if err != nil { - logrus.Errorf("unable to retrieve builtin volumes for container %v: %v", c.ID(), err) - } - } var cleanupErr error // Remove the container from the state if c.config.Pod != "" { @@ -415,8 +410,12 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, } } - for _, v := range volumes { - if volume, err := runtime.state.Volume(v); err == nil { + if !removeVolume { + return cleanupErr + } + + for _, v := range c.config.NamedVolumes { + if volume, err := runtime.state.Volume(v.Name); err == nil { if !volume.IsCtrSpecific() { continue } @@ -548,14 +547,6 @@ func (r *Runtime) GetLatestContainer() (*Container, error) { return ctrs[lastCreatedIndex], nil } -// Check if volName is a named volume and not one of the default mounts we add to containers -func isNamedVolume(volName string) bool { - if volName != "proc" && volName != "tmpfs" && volName != "devpts" && volName != "shm" && volName != "mqueue" && volName != "sysfs" && volName != "cgroup" { - return true - } - return false -} - // Export is the libpod portion of exporting a container to a tar file func (r *Runtime) Export(name string, path string) error { ctr, err := r.LookupContainer(name) diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index db5c29242..40040fc52 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -98,12 +98,26 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool) error if !force { return errors.Wrapf(ErrVolumeBeingUsed, "volume %s is being used by the following container(s): %s", v.Name(), depsStr) } - // If using force, log the warning that the volume is being used by at least one container - logrus.Warnf("volume %s is being used by the following container(s): %s", v.Name(), depsStr) - // Remove the container dependencies so we can go ahead and delete the volume + + // We need to remove all containers using the volume for _, dep := range deps { - if err := r.state.RemoveVolCtrDep(v, dep); err != nil { - return errors.Wrapf(err, "unable to remove container dependency %q from volume %q while trying to delete volume by force", dep, v.Name()) + ctr, err := r.state.Container(dep) + if err != nil { + // If the container's removed, no point in + // erroring. + if errors.Cause(err) == ErrNoSuchCtr || errors.Cause(err) == ErrCtrRemoved { + continue + } + + return errors.Wrapf(err, "error removing container %s that depends on volume %s", dep, v.Name()) + } + + // TODO: do we want to set force here when removing + // containers? + // I'm inclined to say no, in case someone accidentally + // wipes a container they're using... + if err := r.removeContainer(ctx, ctr, false, false); err != nil { + return errors.Wrapf(err, "error removing container %s that depends on volume %s", ctr.ID(), v.Name()) } } } diff --git a/libpod/state.go b/libpod/state.go index 4296fc3cd..d0ad1a1f8 100644 --- a/libpod/state.go +++ b/libpod/state.go @@ -192,10 +192,6 @@ type State interface { // AddVolume adds the specified volume to state. The volume's name // must be unique within the list of existing volumes AddVolume(volume *Volume) error - // RemoveVolCtrDep updates the list of container dependencies that the - // volume has. It either deletes the dependent container ID from - // the sub-bucket - RemoveVolCtrDep(volume *Volume, ctrID string) error // RemoveVolume removes the specified volume. // Only volumes that have no container dependencies can be removed RemoveVolume(volume *Volume) error diff --git a/pkg/spec/createconfig.go b/pkg/spec/createconfig.go index a433fc16d..e71d9d3db 100644 --- a/pkg/spec/createconfig.go +++ b/pkg/spec/createconfig.go @@ -1,7 +1,6 @@ package createconfig import ( - "encoding/json" "fmt" "net" "os" @@ -23,18 +22,16 @@ import ( "golang.org/x/sys/unix" ) -type mountType string - // Type constants const ( bps = iota iops // TypeBind is the type for mounting host dir - TypeBind mountType = "bind" + TypeBind = "bind" // TypeVolume is the type for remote storage volumes - // TypeVolume mountType = "volume" // re-enable upon use + // TypeVolume = "volume" // re-enable upon use // TypeTmpfs is the type for mounting tmpfs - TypeTmpfs mountType = "tmpfs" + TypeTmpfs = "tmpfs" ) // CreateResourceConfig represents resource elements in CreateConfig @@ -130,15 +127,15 @@ type CreateConfig struct { Mounts []spec.Mount //mounts Volumes []string //volume VolumesFrom []string - WorkDir string //workdir - LabelOpts []string //SecurityOpts - NoNewPrivs bool //SecurityOpts - ApparmorProfile string //SecurityOpts - SeccompProfilePath string //SecurityOpts + NamedVolumes []*libpod.ContainerNamedVolume // Filled in by CreateConfigToOCISpec + WorkDir string //workdir + LabelOpts []string //SecurityOpts + NoNewPrivs bool //SecurityOpts + ApparmorProfile string //SecurityOpts + SeccompProfilePath string //SecurityOpts SecurityOpts []string Rootfs string - LocalVolumes []spec.Mount //Keeps track of the built-in volumes of container used in the --volumes-from flag - Syslog bool // Whether to enable syslog on exit commands + Syslog bool // Whether to enable syslog on exit commands } func u32Ptr(i int64) *uint32 { u := uint32(i); return &u } @@ -172,9 +169,9 @@ func (c *CreateConfig) AddContainerInitBinary(path string) error { c.Command = append([]string{"/dev/init", "--"}, c.Command...) c.Mounts = append(c.Mounts, spec.Mount{ Destination: "/dev/init", - Type: "bind", + Type: TypeBind, Source: path, - Options: []string{"bind", "ro"}, + Options: []string{TypeBind, "ro"}, }) return nil } @@ -217,9 +214,9 @@ func (c *CreateConfig) initFSMounts() []spec.Mount { return mounts } -//GetVolumeMounts takes user provided input for bind mounts and creates Mount structs +// GetVolumeMounts takes user provided input for bind mounts and creates Mount structs func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, error) { - m := c.LocalVolumes + m := []spec.Mount{} for _, i := range c.Volumes { var options []string spliti := strings.Split(i, ":") @@ -255,9 +252,11 @@ func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, e mount.Source = "tmpfs" mount.Options = append(mount.Options, "tmpcopyup") } else { + // TODO: Move support for this and tmpfs into libpod + // Should tmpfs also be handled as named volumes? Wouldn't be hard // This will cause a new local Volume to be created on your system mount.Source = stringid.GenerateNonCryptoID() - mount.Options = append(mount.Options, "bind") + mount.Options = append(mount.Options, TypeBind) } m = append(m, mount) } @@ -268,13 +267,12 @@ func (c *CreateConfig) GetVolumeMounts(specMounts []spec.Mount) ([]spec.Mount, e // GetVolumesFrom reads the create-config artifact of the container to get volumes from // and adds it to c.Volumes of the current container. func (c *CreateConfig) GetVolumesFrom() error { - var options string - if os.Geteuid() != 0 { return nil } for _, vol := range c.VolumesFrom { + options := "" splitVol := strings.SplitN(vol, ":", 2) if len(splitVol) == 2 { options = splitVol[1] @@ -283,41 +281,60 @@ func (c *CreateConfig) GetVolumesFrom() error { if err != nil { return errors.Wrapf(err, "error looking up container %q", splitVol[0]) } - inspect, err := ctr.Inspect(false) - if err != nil { - return errors.Wrapf(err, "error inspecting %q", splitVol[0]) - } - var createArtifact CreateConfig - artifact, err := ctr.GetArtifact("create-config") - if err != nil { - return errors.Wrapf(err, "error getting create-config artifact for %q", splitVol[0]) + + logrus.Debugf("Adding volumes from container %s", ctr.ID()) + + // Look up the container's user volumes. This gets us the + // destinations of all mounts the user added to the container. + userVolumesArr := ctr.UserVolumes() + + // We're going to need to access them a lot, so convert to a map + // to reduce looping. + // We'll also use the map to indicate if we missed any volumes along the way. + userVolumes := make(map[string]bool) + for _, dest := range userVolumesArr { + userVolumes[dest] = false } - if err := json.Unmarshal(artifact, &createArtifact); err != nil { - return err + + // Now we get the container's spec and loop through its volumes + // and append them in if we can find them. + spec := ctr.Spec() + if spec == nil { + return errors.Errorf("error retrieving container %s spec", ctr.ID()) } - for key := range createArtifact.BuiltinImgVolumes { - for _, m := range inspect.Mounts { - if m.Destination == key { - c.LocalVolumes = append(c.LocalVolumes, m) - break + for _, mnt := range spec.Mounts { + if mnt.Type != TypeBind { + continue + } + if _, exists := userVolumes[mnt.Destination]; exists { + userVolumes[mnt.Destination] = true + localOptions := options + if localOptions == "" { + localOptions = strings.Join(mnt.Options, ",") } + c.Volumes = append(c.Volumes, fmt.Sprintf("%s:%s:%s", mnt.Source, mnt.Destination, localOptions)) } } - for _, i := range createArtifact.Volumes { - // Volumes format is host-dir:ctr-dir[:options], so get the host and ctr dir - // and add on the options given by the user to the flag. - spliti := strings.SplitN(i, ":", 3) - // Throw error if mounting volume from container with Z option (private label) - // Override this by adding 'z' to options. - if len(spliti) > 2 && strings.Contains(spliti[2], "Z") && !strings.Contains(options, "z") { - return errors.Errorf("volume mounted with private option 'Z' in %q. Use option 'z' to mount in current container", ctr.ID()) + // We're done with the spec mounts. Add named volumes. + // Add these unconditionally - none of them are automatically + // part of the container, as some spec mounts are. + namedVolumes := ctr.NamedVolumes() + for _, namedVol := range namedVolumes { + if _, exists := userVolumes[namedVol.Dest]; exists { + userVolumes[namedVol.Dest] = true } - if options == "" { - // Mount the volumes with the default options - c.Volumes = append(c.Volumes, createArtifact.Volumes...) - } else { - c.Volumes = append(c.Volumes, spliti[0]+":"+spliti[1]+":"+options) + localOptions := options + if localOptions == "" { + localOptions = strings.Join(namedVol.Options, ",") + } + c.Volumes = append(c.Volumes, fmt.Sprintf("%s:%s:%s", namedVol.Name, namedVol.Dest, localOptions)) + } + + // Check if we missed any volumes + for volDest, found := range userVolumes { + if !found { + logrus.Warnf("Unable to match volume %s from container %s for volumes-from", volDest, ctr.ID()) } } } @@ -417,14 +434,20 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l // others, if they are included volumes := make([]string, 0, len(c.Volumes)) for _, vol := range c.Volumes { - volumes = append(volumes, strings.SplitN(vol, ":", 2)[0]) + // We always want the volume destination + splitVol := strings.SplitN(vol, ":", 3) + if len(splitVol) > 1 { + volumes = append(volumes, splitVol[1]) + } else { + volumes = append(volumes, splitVol[0]) + } } options = append(options, libpod.WithUserVolumes(volumes)) } - if len(c.LocalVolumes) != 0 { - options = append(options, libpod.WithLocalVolumes(c.LocalVolumes)) + if len(c.NamedVolumes) != 0 { + options = append(options, libpod.WithNamedVolumes(c.NamedVolumes)) } if len(c.Command) != 0 { @@ -538,7 +561,7 @@ func (c *CreateConfig) GetContainerCreateOptions(runtime *libpod.Runtime, pod *l options = append(options, libpod.WithPrivileged(c.Privileged)) - useImageVolumes := c.ImageVolumeType == "bind" + useImageVolumes := c.ImageVolumeType == TypeBind // Gather up the options for NewContainer which consist of With... funcs options = append(options, libpod.WithRootFSFromImage(c.ImageID, c.Image, useImageVolumes)) options = append(options, libpod.WithSecLabels(c.LabelOpts)) diff --git a/pkg/spec/spec.go b/pkg/spec/spec.go index a61741f73..9b6bd089e 100644 --- a/pkg/spec/spec.go +++ b/pkg/spec/spec.go @@ -6,6 +6,7 @@ import ( "path/filepath" "strings" + "github.com/containers/libpod/libpod" "github.com/containers/libpod/pkg/rootless" "github.com/containers/storage/pkg/mount" pmount "github.com/containers/storage/pkg/mount" @@ -48,6 +49,33 @@ func supercedeUserMounts(mounts []spec.Mount, configMount []spec.Mount) []spec.M return configMount } +// Split named volumes from normal volumes +func splitNamedVolumes(mounts []spec.Mount) ([]spec.Mount, []*libpod.ContainerNamedVolume) { + newMounts := make([]spec.Mount, 0) + namedVolumes := make([]*libpod.ContainerNamedVolume, 0) + for _, mount := range mounts { + // If it's not a named volume, append unconditionally + if mount.Type != TypeBind { + newMounts = append(newMounts, mount) + continue + } + // Volumes that are not named volumes must be an absolute or + // relative path. + // Volume names may not begin with a non-alphanumeric character + // so the HasPrefix() check is safe here. + if strings.HasPrefix(mount.Source, "/") || strings.HasPrefix(mount.Source, ".") { + newMounts = append(newMounts, mount) + } else { + namedVolume := new(libpod.ContainerNamedVolume) + namedVolume.Name = mount.Source + namedVolume.Dest = mount.Destination + namedVolume.Options = mount.Options + namedVolumes = append(namedVolumes, namedVolume) + } + } + return newMounts, namedVolumes +} + func getAvailableGids() (int64, error) { idMap, err := user.ParseIDMapFile("/proc/self/gid_map") if err != nil { @@ -99,7 +127,7 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint } sysMnt := spec.Mount{ Destination: "/sys", - Type: "bind", + Type: TypeBind, Source: "/sys", Options: []string{"rprivate", "nosuid", "noexec", "nodev", r, "rbind"}, } @@ -126,7 +154,7 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint g.RemoveMount("/dev/mqueue") devMqueue := spec.Mount{ Destination: "/dev/mqueue", - Type: "bind", + Type: TypeBind, Source: "/dev/mqueue", Options: []string{"bind", "nosuid", "noexec", "nodev"}, } @@ -136,7 +164,7 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint g.RemoveMount("/proc") procMount := spec.Mount{ Destination: "/proc", - Type: "bind", + Type: TypeBind, Source: "/proc", Options: []string{"rbind", "nosuid", "noexec", "nodev"}, } @@ -377,6 +405,12 @@ func CreateConfigToOCISpec(config *CreateConfig) (*spec.Spec, error) { //nolint configSpec.Mounts = supercedeUserMounts(volumeMounts, configSpec.Mounts) //--mount configSpec.Mounts = supercedeUserMounts(config.initFSMounts(), configSpec.Mounts) + + // Split normal mounts and named volumes + newMounts, namedVolumes := splitNamedVolumes(configSpec.Mounts) + configSpec.Mounts = newMounts + config.NamedVolumes = namedVolumes + // BLOCK IO blkio, err := config.CreateBlockIO() if err != nil { diff --git a/test/e2e/commit_test.go b/test/e2e/commit_test.go index bf9c88de5..fe4ae64cf 100644 --- a/test/e2e/commit_test.go +++ b/test/e2e/commit_test.go @@ -144,7 +144,7 @@ var _ = Describe("Podman commit", func() { inspect.WaitWithDefaultTimeout() Expect(inspect.ExitCode()).To(Equal(0)) image := inspect.InspectImageJSON() - _, ok := image[0].Config.Volumes["/tmp"] + _, ok := image[0].Config.Volumes["/foo"] Expect(ok).To(BeTrue()) r := podmanTest.Podman([]string{"run", "newimage"}) diff --git a/test/e2e/run_test.go b/test/e2e/run_test.go index 2daf2fe5b..a89ee491b 100644 --- a/test/e2e/run_test.go +++ b/test/e2e/run_test.go @@ -611,7 +611,6 @@ USER mail` session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) Expect(session.OutputToString()).To(ContainSubstring("data")) - }) It("podman run --volumes flag with multiple volumes", func() { diff --git a/test/e2e/volume_rm_test.go b/test/e2e/volume_rm_test.go index 888474670..39628d56f 100644 --- a/test/e2e/volume_rm_test.go +++ b/test/e2e/volume_rm_test.go @@ -32,7 +32,7 @@ var _ = Describe("Podman volume rm", func() { }) - It("podman rm volume", func() { + It("podman volume rm", func() { session := podmanTest.Podman([]string{"volume", "create", "myvol"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) @@ -47,7 +47,7 @@ var _ = Describe("Podman volume rm", func() { Expect(len(session.OutputToStringArray())).To(Equal(0)) }) - It("podman rm with --force flag", func() { + It("podman volume rm with --force flag", func() { SkipIfRemote() session := podmanTest.Podman([]string{"create", "-v", "myvol:/myvol", ALPINE, "ls"}) cid := session.OutputToString() |