Merge pull request #2774 from mheon/db_rework_named_volume

Rework named volumes in DB

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**
 

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

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)
 			}
 		}
 	}

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

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()

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{

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 {