diff options
-rw-r--r-- | libpod/boltdb_state.go | 79 | ||||
-rw-r--r-- | libpod/boltdb_state_internal.go | 53 | ||||
-rw-r--r-- | libpod/container.go | 18 | ||||
-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/runtime_ctr.go | 58 | ||||
-rw-r--r-- | libpod/runtime_volume_linux.go | 14 | ||||
-rw-r--r-- | libpod/state.go | 4 |
9 files changed, 126 insertions, 166 deletions
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 b02e536c5..74dc5be5c 100644 --- a/libpod/container.go +++ b/libpod/container.go @@ -371,7 +371,7 @@ type ContainerConfig struct { } // ContainerNamedVolume is a named volume that will be mounted into the -// container. +// 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. @@ -502,6 +502,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/runtime_ctr.go b/libpod/runtime_ctr.go index da2399685..ec43d5dcb 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -173,26 +173,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 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()) + // 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 := os.Chown(ctr.config.Spec.Mounts[i].Source, ctr.RootUID(), ctr.RootGID()); err != nil { + // return nil, errors.Wrapf(err, "cannot chown %q to %d:%d", ctr.config.Spec.Mounts[i].Source, ctr.RootUID(), ctr.RootGID()) + // } + // 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() + // } + // } if ctr.config.LogPath == "" { ctr.config.LogPath = filepath.Join(ctr.config.StaticDir, "ctr.log") @@ -344,13 +347,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 +411,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 } diff --git a/libpod/runtime_volume_linux.go b/libpod/runtime_volume_linux.go index db5c29242..39c2f808d 100644 --- a/libpod/runtime_volume_linux.go +++ b/libpod/runtime_volume_linux.go @@ -98,14 +98,12 @@ 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 - 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()) - } - } + + // TODO: force-removing a volume makes *no sense* + // I do not believe we should be allowing this at all. + // For now, return angry errors. + // TODO: need to do something more sane in this case + return errors.Wrapf(ErrVolumeBeingUsed, "TODO: FIXME - still refusing to remove because force-removing an in-use volume is not good.") } // Set volume as invalid so it can no longer be used 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 |