From 5d93c731afd5b21d88c025069da0e82455f09445 Mon Sep 17 00:00:00 2001 From: Matthew Heon Date: Wed, 29 Jan 2020 14:04:51 -0500 Subject: [PATCH] Deprecate & remove IsCtrSpecific in favor of IsAnon In Podman 1.6.3, we added support for anonymous volumes - fixing our old, broken support for named volumes that were created with containers. Unfortunately, this reused the database field we used for the old implementation, and toggled volume removal on for `podman run --rm` - so now, we were removing *named* volumes created with older versions of Podman. We can't modify these old volumes in the DB, so the next-safest thing to do is swap to a new field to indicate volumes should be removed. Problem: Volumes created with 1.6.3 and up until this lands, even anonymous volumes, will not be removed. However, this is safer than removing too many volumes, as we were doing before. Fixes #5009 Signed-off-by: Matthew Heon --- libpod/container_commit.go | 2 +- libpod/options.go | 11 +++++------ libpod/runtime_ctr.go | 6 +++--- libpod/runtime_pod_linux.go | 2 +- libpod/volume.go | 14 ++++++-------- libpod/volume_inspect.go | 10 +++++----- 6 files changed, 21 insertions(+), 24 deletions(-) diff --git a/libpod/container_commit.go b/libpod/container_commit.go index ccc23621ef..fa6e95b38c 100644 --- a/libpod/container_commit.go +++ b/libpod/container_commit.go @@ -137,7 +137,7 @@ func (c *Container) Commit(ctx context.Context, destImage string, options Contai if err != nil { return nil, errors.Wrapf(err, "volume %s used in container %s has been removed", v.Name, c.ID()) } - if vol.IsCtrSpecific() { + if vol.Anonymous() { importBuilder.AddVolume(v.Dest) } } diff --git a/libpod/options.go b/libpod/options.go index 5930373825..923e7292c0 100644 --- a/libpod/options.go +++ b/libpod/options.go @@ -1546,17 +1546,16 @@ func WithVolumeGID(gid int) VolumeCreateOption { } } -// withSetCtrSpecific sets a bool notifying libpod that a volume was created -// specifically for a container. -// These volumes will be removed when the container is removed and volumes are -// also specified for removal. -func withSetCtrSpecific() VolumeCreateOption { +// withSetAnon sets a bool notifying libpod that this volume is anonymous and +// should be removed when containers using it are removed and volumes are +// specified for removal. +func withSetAnon() VolumeCreateOption { return func(volume *Volume) error { if volume.valid { return define.ErrVolumeFinalized } - volume.config.IsCtrSpecific = true + volume.config.IsAnon = true return nil } diff --git a/libpod/runtime_ctr.go b/libpod/runtime_ctr.go index e8952967db..3ad09f27c0 100644 --- a/libpod/runtime_ctr.go +++ b/libpod/runtime_ctr.go @@ -319,7 +319,7 @@ func (r *Runtime) setupContainer(ctx context.Context, ctr *Container) (c *Contai // The volume does not exist, so we need to create it. volOptions := []VolumeCreateOption{WithVolumeName(vol.Name), WithVolumeUID(ctr.RootUID()), WithVolumeGID(ctr.RootGID())} if isAnonymous { - volOptions = append(volOptions, withSetCtrSpecific()) + volOptions = append(volOptions, withSetAnon()) } newVol, err := r.newVolume(ctx, volOptions...) if err != nil { @@ -569,7 +569,7 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force bool, for _, v := range c.config.NamedVolumes { if volume, err := runtime.state.Volume(v.Name); err == nil { - if !volume.IsCtrSpecific() { + if !volume.Anonymous() { continue } if err := runtime.removeVolume(ctx, volume, false); err != nil && errors.Cause(err) != define.ErrNoSuchVolume { @@ -707,7 +707,7 @@ func (r *Runtime) evictContainer(ctx context.Context, idOrName string, removeVol for _, v := range c.config.NamedVolumes { if volume, err := r.state.Volume(v.Name); err == nil { - if !volume.IsCtrSpecific() { + if !volume.Anonymous() { continue } if err := r.removeVolume(ctx, volume, false); err != nil && err != define.ErrNoSuchVolume && err != define.ErrVolumeBeingUsed { diff --git a/libpod/runtime_pod_linux.go b/libpod/runtime_pod_linux.go index 450c64d243..5b0111b858 100644 --- a/libpod/runtime_pod_linux.go +++ b/libpod/runtime_pod_linux.go @@ -261,7 +261,7 @@ func (r *Runtime) removePod(ctx context.Context, p *Pod, removeCtrs, force bool) logrus.Errorf("Error retrieving volume %s: %v", volName, err) continue } - if !volume.IsCtrSpecific() { + if !volume.Anonymous() { continue } if err := r.removeVolume(ctx, volume, false); err != nil { diff --git a/libpod/volume.go b/libpod/volume.go index c4771bbb81..1ffed872e8 100644 --- a/libpod/volume.go +++ b/libpod/volume.go @@ -38,9 +38,8 @@ type VolumeConfig struct { // a list of mount options. For other drivers, they are passed to the // volume driver handling the volume. Options map[string]string `json:"volumeOptions,omitempty"` - // Whether this volume was created for a specific container and will be - // removed with it. - IsCtrSpecific bool `json:"ctrSpecific"` + // Whether this volume is anonymous (will be removed on container exit) + IsAnon bool `json:"isAnon"` // UID the volume will be created as. UID int `json:"uid"` // GID the volume will be created as. @@ -106,11 +105,10 @@ func (v *Volume) Options() map[string]string { return options } -// IsCtrSpecific returns whether this volume was created specifically for a -// given container. Images with this set to true will be removed when the -// container is removed with the Volumes parameter set to true. -func (v *Volume) IsCtrSpecific() bool { - return v.config.IsCtrSpecific +// Anonymous returns whether this volume is anonymous. Anonymous volumes were +// created with a container, and will be removed when that container is removed. +func (v *Volume) Anonymous() bool { + return v.config.IsAnon } // UID returns the UID the volume will be created as. diff --git a/libpod/volume_inspect.go b/libpod/volume_inspect.go index c333b89618..136f9da5e4 100644 --- a/libpod/volume_inspect.go +++ b/libpod/volume_inspect.go @@ -37,10 +37,10 @@ type InspectVolumeData struct { UID int `json:"UID,omitempty"` // GID is the GID that the volume was created with. GID int `json:"GID,omitempty"` - // ContainerSpecific indicates that the volume was created as part of a - // specific container, and will be removed when that container is - // removed. - ContainerSpecific bool `json:"ContainerSpecific,omitempty"` + // Anonymous indicates that the volume was created as an anonymous + // volume for a specific container, and will be be removed when any + // container using it is removed. + Anonymous bool `json:"Anonymous,omitempty"` } // Inspect provides detailed information about the configuration of the given @@ -67,7 +67,7 @@ func (v *Volume) Inspect() (*InspectVolumeData, error) { } data.UID = v.config.UID data.GID = v.config.GID - data.ContainerSpecific = v.config.IsCtrSpecific + data.Anonymous = v.config.IsAnon return data, nil }