mirror of
https://github.com/containers/podman
synced 2024-10-20 01:03:51 +00:00
Merge pull request #3893 from mheon/readd_volume_locks
Re-add volume locks
This commit is contained in:
commit
bdf9e56813
|
@ -870,7 +870,7 @@ func (s *BoltState) RewritePodConfig(pod *Pod, newCfg *PodConfig) error {
|
|||
|
||||
newCfgJSON, err := json.Marshal(newCfg)
|
||||
if err != nil {
|
||||
return errors.Wrapf(err, "error marshalling new configuration JSON for container %s", pod.ID())
|
||||
return errors.Wrapf(err, "error marshalling new configuration JSON for pod %s", pod.ID())
|
||||
}
|
||||
|
||||
db, err := s.getDBCon()
|
||||
|
@ -900,6 +900,50 @@ func (s *BoltState) RewritePodConfig(pod *Pod, newCfg *PodConfig) error {
|
|||
return err
|
||||
}
|
||||
|
||||
// RewriteVolumeConfig rewrites a volume's configuration.
|
||||
// WARNING: This function is DANGEROUS. Do not use without reading the full
|
||||
// comment on this function in state.go.
|
||||
func (s *BoltState) RewriteVolumeConfig(volume *Volume, newCfg *VolumeConfig) error {
|
||||
if !s.valid {
|
||||
return define.ErrDBClosed
|
||||
}
|
||||
|
||||
if !volume.valid {
|
||||
return define.ErrVolumeRemoved
|
||||
}
|
||||
|
||||
newCfgJSON, err := json.Marshal(newCfg)
|
||||
if err != nil {
|
||||
return errors.Wrapf(err, "error marshalling new configuration JSON for volume %q", volume.Name())
|
||||
}
|
||||
|
||||
db, err := s.getDBCon()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
defer s.deferredCloseDBCon(db)
|
||||
|
||||
err = db.Update(func(tx *bolt.Tx) error {
|
||||
volBkt, err := getVolBucket(tx)
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
volDB := volBkt.Bucket([]byte(volume.Name()))
|
||||
if volDB == nil {
|
||||
volume.valid = false
|
||||
return errors.Wrapf(define.ErrNoSuchVolume, "no volume with name %q found in DB", volume.Name())
|
||||
}
|
||||
|
||||
if err := volDB.Put(configKey, newCfgJSON); err != nil {
|
||||
return errors.Wrapf(err, "error updating volume %q config JSON", volume.Name())
|
||||
}
|
||||
|
||||
return nil
|
||||
})
|
||||
return err
|
||||
}
|
||||
|
||||
// Pod retrieves a pod given its full ID
|
||||
func (s *BoltState) Pod(id string) (*Pod, error) {
|
||||
if id == "" {
|
||||
|
|
|
@ -449,6 +449,13 @@ func (s *BoltState) getVolumeFromDB(name []byte, volume *Volume, volBkt *bolt.Bu
|
|||
return errors.Wrapf(err, "error unmarshalling volume %s config from DB", string(name))
|
||||
}
|
||||
|
||||
// Get the lock
|
||||
lock, err := s.runtime.lockManager.RetrieveLock(volume.config.LockID)
|
||||
if err != nil {
|
||||
return errors.Wrapf(err, "error retrieving lock for volume %q", string(name))
|
||||
}
|
||||
volume.lock = lock
|
||||
|
||||
volume.runtime = s.runtime
|
||||
volume.valid = true
|
||||
|
||||
|
|
|
@ -425,6 +425,26 @@ func (s *InMemoryState) RewritePodConfig(pod *Pod, newCfg *PodConfig) error {
|
|||
return nil
|
||||
}
|
||||
|
||||
// RewriteVolumeConfig rewrites a volume's configuration.
|
||||
// This function is DANGEROUS, even with in-memory state.
|
||||
// Please read the full comment in state.go before using it.
|
||||
func (s *InMemoryState) RewriteVolumeConfig(volume *Volume, newCfg *VolumeConfig) error {
|
||||
if !volume.valid {
|
||||
return define.ErrVolumeRemoved
|
||||
}
|
||||
|
||||
// If the volume does not exist, return error
|
||||
stateVol, ok := s.volumes[volume.Name()]
|
||||
if !ok {
|
||||
volume.valid = false
|
||||
return errors.Wrapf(define.ErrNoSuchVolume, "volume with name %q not found in state", volume.Name())
|
||||
}
|
||||
|
||||
stateVol.config = newCfg
|
||||
|
||||
return nil
|
||||
}
|
||||
|
||||
// Volume retrieves a volume from its full name
|
||||
func (s *InMemoryState) Volume(name string) (*Volume, error) {
|
||||
if name == "" {
|
||||
|
|
|
@ -253,10 +253,13 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (c *Contai
|
|||
|
||||
// Go through named volumes and add them.
|
||||
// If they don't exist they will be created using basic options.
|
||||
// Maintain an array of them - we need to lock them later.
|
||||
ctrNamedVolumes := make([]*Volume, 0, len(ctr.config.NamedVolumes))
|
||||
for _, vol := range ctr.config.NamedVolumes {
|
||||
// Check if it exists already
|
||||
_, err := r.state.Volume(vol.Name)
|
||||
dbVol, err := r.state.Volume(vol.Name)
|
||||
if err == nil {
|
||||
ctrNamedVolumes = append(ctrNamedVolumes, dbVol)
|
||||
// The volume exists, we're good
|
||||
continue
|
||||
} else if errors.Cause(err) != config2.ErrNoSuchVolume {
|
||||
|
@ -275,6 +278,8 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (c *Contai
|
|||
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)
|
||||
}
|
||||
|
||||
ctrNamedVolumes = append(ctrNamedVolumes, newVol)
|
||||
}
|
||||
|
||||
if ctr.config.LogPath == "" && ctr.config.LogDriver != JournaldLogging {
|
||||
|
@ -291,6 +296,14 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (c *Contai
|
|||
ctr.config.Mounts = append(ctr.config.Mounts, ctr.config.ShmDir)
|
||||
}
|
||||
|
||||
// Lock all named volumes we are adding ourself to, to ensure we can't
|
||||
// use a volume being removed.
|
||||
for _, namedVol := range ctrNamedVolumes {
|
||||
toLock := namedVol
|
||||
toLock.lock.Lock()
|
||||
defer toLock.lock.Unlock()
|
||||
}
|
||||
|
||||
// Add the container to the state
|
||||
// TODO: May be worth looking into recovering from name/ID collisions here
|
||||
if ctr.config.Pod != "" {
|
||||
|
|
|
@ -53,6 +53,23 @@ func (r *Runtime) renumberLocks() error {
|
|||
return err
|
||||
}
|
||||
}
|
||||
allVols, err := r.state.AllVolumes()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
for _, vol := range allVols {
|
||||
lock, err := r.lockManager.AllocateLock()
|
||||
if err != nil {
|
||||
return errors.Wrapf(err, "error allocating lock for volume %s", vol.Name())
|
||||
}
|
||||
|
||||
vol.config.LockID = lock.ID()
|
||||
|
||||
// Write the new lock ID
|
||||
if err := r.state.RewriteVolumeConfig(vol, vol.config); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
r.newSystemEvent(events.Renumber)
|
||||
|
||||
|
|
|
@ -36,6 +36,10 @@ func (r *Runtime) RemoveVolume(ctx context.Context, v *Volume, force bool) error
|
|||
return nil
|
||||
}
|
||||
}
|
||||
|
||||
v.lock.Lock()
|
||||
defer v.lock.Unlock()
|
||||
|
||||
return r.removeVolume(ctx, v, force)
|
||||
}
|
||||
|
||||
|
|
|
@ -28,7 +28,7 @@ func (r *Runtime) NewVolume(ctx context.Context, options ...VolumeCreateOption)
|
|||
}
|
||||
|
||||
// newVolume creates a new empty volume
|
||||
func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) (*Volume, error) {
|
||||
func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption) (_ *Volume, Err error) {
|
||||
volume, err := newVolume(r)
|
||||
if err != nil {
|
||||
return nil, errors.Wrapf(err, "error creating volume")
|
||||
|
@ -68,6 +68,21 @@ func (r *Runtime) newVolume(ctx context.Context, options ...VolumeCreateOption)
|
|||
}
|
||||
volume.config.MountPoint = fullVolPath
|
||||
|
||||
lock, err := r.lockManager.AllocateLock()
|
||||
if err != nil {
|
||||
return nil, errors.Wrapf(err, "error allocating lock for new volume")
|
||||
}
|
||||
volume.lock = lock
|
||||
volume.config.LockID = volume.lock.ID()
|
||||
|
||||
defer func() {
|
||||
if Err != nil {
|
||||
if err := volume.lock.Free(); err != nil {
|
||||
logrus.Errorf("Error freeing volume lock after failed creation: %v", err)
|
||||
}
|
||||
}
|
||||
}()
|
||||
|
||||
volume.valid = true
|
||||
|
||||
// Add the volume to state
|
||||
|
@ -110,6 +125,8 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool) error
|
|||
return errors.Wrapf(err, "error removing container %s that depends on volume %s", dep, v.Name())
|
||||
}
|
||||
|
||||
logrus.Debugf("Removing container %s (depends on volume %q)", ctr.ID(), v.Name())
|
||||
|
||||
// TODO: do we want to set force here when removing
|
||||
// containers?
|
||||
// I'm inclined to say no, in case someone accidentally
|
||||
|
@ -128,12 +145,24 @@ func (r *Runtime) removeVolume(ctx context.Context, v *Volume, force bool) error
|
|||
return errors.Wrapf(err, "error removing volume %s", v.Name())
|
||||
}
|
||||
|
||||
// Delete the mountpoint path of the volume, that is delete the volume from /var/lib/containers/storage/volumes
|
||||
var removalErr error
|
||||
|
||||
// Free the volume's lock
|
||||
if err := v.lock.Free(); err != nil {
|
||||
removalErr = errors.Wrapf(err, "error freeing lock for volume %s", v.Name())
|
||||
}
|
||||
|
||||
// Delete the mountpoint path of the volume, that is delete the volume
|
||||
// from /var/lib/containers/storage/volumes
|
||||
if err := v.teardownStorage(); err != nil {
|
||||
return errors.Wrapf(err, "error cleaning up volume storage for %q", v.Name())
|
||||
if removalErr == nil {
|
||||
removalErr = errors.Wrapf(err, "error cleaning up volume storage for %q", v.Name())
|
||||
} else {
|
||||
logrus.Errorf("error cleaning up volume storage for volume %q: %v", v.Name(), err)
|
||||
}
|
||||
}
|
||||
|
||||
defer v.newVolumeEvent(events.Remove)
|
||||
logrus.Debugf("Removed volume %s", v.Name())
|
||||
return nil
|
||||
return removalErr
|
||||
}
|
||||
|
|
|
@ -115,12 +115,20 @@ type State interface {
|
|||
// answer is this: use this only very sparingly, and only if you really
|
||||
// know what you're doing.
|
||||
RewriteContainerConfig(ctr *Container, newCfg *ContainerConfig) error
|
||||
// PLEASE READ THE ABOVE DESCRIPTION BEFORE USING.
|
||||
// PLEASE READ THE DESCRIPTION FOR RewriteContainerConfig BEFORE USING.
|
||||
// This function is identical to RewriteContainerConfig, save for the
|
||||
// fact that it is used with pods instead.
|
||||
// It is subject to the same conditions as RewriteContainerConfig.
|
||||
// Please do not use this unless you know what you're doing.
|
||||
RewritePodConfig(pod *Pod, newCfg *PodConfig) error
|
||||
// PLEASE READ THE DESCRIPTION FOR RewriteContainerConfig BEFORE USING.
|
||||
// This function is identical to RewriteContainerConfig, save for the
|
||||
// fact that it is used with volumes instead.
|
||||
// It is subject to the same conditions as RewriteContainerConfig.
|
||||
// The exception is that volumes do not have IDs, so only volume name
|
||||
// cannot be altered.
|
||||
// Please do not use this unless you know what you're doing.
|
||||
RewriteVolumeConfig(volume *Volume, newCfg *VolumeConfig) error
|
||||
|
||||
// Accepts full ID of pod.
|
||||
// If the pod given is not in the set namespace, an error will be
|
||||
|
|
|
@ -2,6 +2,8 @@ package libpod
|
|||
|
||||
import (
|
||||
"time"
|
||||
|
||||
"github.com/containers/libpod/libpod/lock"
|
||||
)
|
||||
|
||||
// Volume is the type used to create named volumes
|
||||
|
@ -11,21 +13,35 @@ type Volume struct {
|
|||
|
||||
valid bool
|
||||
runtime *Runtime
|
||||
lock lock.Locker
|
||||
}
|
||||
|
||||
// VolumeConfig holds the volume's config information
|
||||
type VolumeConfig struct {
|
||||
// Name of the volume
|
||||
// Name of the volume.
|
||||
Name string `json:"name"`
|
||||
|
||||
Labels map[string]string `json:"labels"`
|
||||
Driver string `json:"driver"`
|
||||
MountPoint string `json:"mountPoint"`
|
||||
CreatedTime time.Time `json:"createdAt,omitempty"`
|
||||
Options map[string]string `json:"options"`
|
||||
IsCtrSpecific bool `json:"ctrSpecific"`
|
||||
UID int `json:"uid"`
|
||||
GID int `json:"gid"`
|
||||
// ID of the volume's lock.
|
||||
LockID uint32 `json:"lockID"`
|
||||
// Labels for the volume.
|
||||
Labels map[string]string `json:"labels"`
|
||||
// The volume driver. Empty string or local does not activate a volume
|
||||
// driver, all other volumes will.
|
||||
Driver string `json:"driver"`
|
||||
// The location the volume is mounted at.
|
||||
MountPoint string `json:"mountPoint"`
|
||||
// Time the volume was created.
|
||||
CreatedTime time.Time `json:"createdAt,omitempty"`
|
||||
// Options to pass to the volume driver. For the local driver, this is
|
||||
// a list of mount options. For other drivers, they are passed to the
|
||||
// volume driver handling the volume.
|
||||
Options map[string]string `json:"options"`
|
||||
// Whether this volume was created for a specific container and will be
|
||||
// removed with it.
|
||||
IsCtrSpecific bool `json:"ctrSpecific"`
|
||||
// UID the volume will be created as.
|
||||
UID int `json:"uid"`
|
||||
// GID the volume will be created as.
|
||||
GID int `json:"gid"`
|
||||
}
|
||||
|
||||
// Name retrieves the volume's name
|
||||
|
|
|
@ -154,4 +154,12 @@ var _ = Describe("Podman run with volumes", func() {
|
|||
session.WaitWithDefaultTimeout()
|
||||
Expect(session.ExitCode()).To(Not(Equal(0)))
|
||||
})
|
||||
|
||||
It("podman run with volume flag and multiple named volumes", func() {
|
||||
session := podmanTest.Podman([]string{"run", "--rm", "-v", "testvol1:/testvol1", "-v", "testvol2:/testvol2", ALPINE, "grep", "/testvol", "/proc/self/mountinfo"})
|
||||
session.WaitWithDefaultTimeout()
|
||||
Expect(session.ExitCode()).To(Equal(0))
|
||||
Expect(session.OutputToString()).To(ContainSubstring("/testvol1"))
|
||||
Expect(session.OutputToString()).To(ContainSubstring("/testvol2"))
|
||||
})
|
||||
})
|
||||
|
|
Loading…
Reference in a new issue