From e392779be65e9d08d4a616ea3978a8462142bb7c Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Fri, 24 Sep 2021 14:06:50 +0200 Subject: [PATCH] libimage: prune: allow for removing external containers Support pruning images in use by external containers (e.g., build containers). Requires extending the containers filter, adding a callback to libpod and a new options for image removal. Tests will be added to Podman. Signed-off-by: Valentin Rothberg --- libimage/filters.go | 38 +++++++++++++++++++++++++++++--------- libimage/image.go | 34 +++++++++++++++++++++++++--------- libimage/image_test.go | 11 +++++++---- libimage/runtime.go | 33 +++++++++++++++++++++++++++++++-- 4 files changed, 92 insertions(+), 24 deletions(-) diff --git a/libimage/filters.go b/libimage/filters.go index 0cc5cc311..833f940cc 100644 --- a/libimage/filters.go +++ b/libimage/filters.go @@ -47,11 +47,11 @@ func filterImages(images []*Image, filters []filterFunc) ([]*Image, error) { // compileImageFilters creates `filterFunc`s for the specified filters. The // required format is `key=value` with the following supported keys: // after, since, before, containers, dangling, id, label, readonly, reference, intermediate -func (r *Runtime) compileImageFilters(ctx context.Context, filters []string) ([]filterFunc, error) { - logrus.Tracef("Parsing image filters %s", filters) +func (r *Runtime) compileImageFilters(ctx context.Context, options *ListImagesOptions) ([]filterFunc, error) { + logrus.Tracef("Parsing image filters %s", options.Filters) filterFuncs := []filterFunc{} - for _, filter := range filters { + for _, filter := range options.Filters { var key, value string split := strings.SplitN(filter, "=", 2) if len(split) != 2 { @@ -77,11 +77,16 @@ func (r *Runtime) compileImageFilters(ctx context.Context, filters []string) ([] filterFuncs = append(filterFuncs, filterBefore(img.Created())) case "containers": - containers, err := strconv.ParseBool(value) - if err != nil { - return nil, errors.Wrapf(err, "non-boolean value %q for dangling filter", value) + switch value { + case "false", "true": + case "external": + if options.IsExternalContainerFunc == nil { + return nil, fmt.Errorf("libimage error: external containers filter without callback") + } + default: + return nil, fmt.Errorf("unsupported value %q for containers filter", value) } - filterFuncs = append(filterFuncs, filterContainers(containers)) + filterFuncs = append(filterFuncs, filterContainers(value, options.IsExternalContainerFunc)) case "dangling": dangling, err := strconv.ParseBool(value) @@ -190,13 +195,28 @@ func filterReadOnly(value bool) filterFunc { } // filterContainers creates a container filter for matching the specified value. -func filterContainers(value bool) filterFunc { +func filterContainers(value string, fn IsExternalContainerFunc) filterFunc { return func(img *Image) (bool, error) { ctrs, err := img.Containers() if err != nil { return false, err } - return (len(ctrs) > 0) == value, nil + if value != "external" { + boolValue := value == "true" + return (len(ctrs) > 0) == boolValue, nil + } + + // Check whether all associated containers are external ones. + for _, c := range ctrs { + isExternal, err := fn(c) + if err != nil { + return false, fmt.Errorf("checking if %s is an external container in filter: %w", c, err) + } + if !isExternal { + return isExternal, nil + } + } + return true, nil } } diff --git a/libimage/image.go b/libimage/image.go index 4eb83652c..00a2d620e 100644 --- a/libimage/image.go +++ b/libimage/image.go @@ -2,6 +2,7 @@ package libimage import ( "context" + "fmt" "path/filepath" "sort" "strings" @@ -232,11 +233,15 @@ func (i *Image) Containers() ([]string, error) { } // removeContainers removes all containers using the image. -func (i *Image) removeContainers(fn RemoveContainerFunc) error { - // Execute the custom removal func if specified. - if fn != nil { +func (i *Image) removeContainers(options *RemoveImagesOptions) error { + if !options.Force && !options.ExternalContainers { + // Nothing to do. + return nil + } + + if options.Force && options.RemoveContainerFunc != nil { logrus.Debugf("Removing containers of image %s with custom removal function", i.ID()) - if err := fn(i.ID()); err != nil { + if err := options.RemoveContainerFunc(i.ID()); err != nil { return err } } @@ -246,6 +251,19 @@ func (i *Image) removeContainers(fn RemoveContainerFunc) error { return err } + if !options.Force && options.ExternalContainers { + // All containers must be external ones. + for _, cID := range containers { + isExternal, err := options.IsExternalContainerFunc(cID) + if err != nil { + return fmt.Errorf("checking if %s is an external container: %w", cID, err) + } + if !isExternal { + return fmt.Errorf("cannot remove container %s: not an external container", cID) + } + } + } + logrus.Debugf("Removing containers of image %s from the local containers storage", i.ID()) var multiE error for _, cID := range containers { @@ -392,11 +410,9 @@ func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveIma return processedIDs, nil } - // Perform the actual removal. First, remove containers if needed. - if options.Force { - if err := i.removeContainers(options.RemoveContainerFunc); err != nil { - return processedIDs, err - } + // Perform the container removal, if needed. + if err := i.removeContainers(options); err != nil { + return processedIDs, err } // Podman/Docker compat: we only report an image as removed if it has diff --git a/libimage/image_test.go b/libimage/image_test.go index 5ad67b3f5..de5513843 100644 --- a/libimage/image_test.go +++ b/libimage/image_test.go @@ -94,10 +94,13 @@ func TestImageFunctions(t *testing.T) { require.Nil(t, containers) // Since we have no containers here, we can only smoke test. - require.NoError(t, image.removeContainers(nil)) - require.Error(t, image.removeContainers(func(_ string) error { - return errors.New("TEST") - })) + rmOptions := &RemoveImagesOptions{ + RemoveContainerFunc: func(_ string) error { + return errors.New("TEST") + }, + Force: true, + } + require.Error(t, image.removeContainers(rmOptions)) // Two items since both names are "Named". namedRepoTags, err := image.NamedRepoTags() diff --git a/libimage/runtime.go b/libimage/runtime.go index eba8ecaff..dabadbec0 100644 --- a/libimage/runtime.go +++ b/libimage/runtime.go @@ -2,6 +2,7 @@ package libimage import ( "context" + "fmt" "os" "strings" @@ -484,10 +485,16 @@ func (r *Runtime) imageReferenceMatchesContext(ref types.ImageReference, options return true, nil } +// IsExternalContainerFunc allows for checking whether the specified container +// is an external one. The definition of an external container can be set by +// callers. +type IsExternalContainerFunc func(containerID string) (bool, error) + // ListImagesOptions allow for customizing listing images. type ListImagesOptions struct { // Filters to filter the listed images. Supported filters are // * after,before,since=image + // * containers=true,false,external // * dangling=true,false // * intermediate=true,false (useful for pruning images) // * id=id @@ -495,6 +502,11 @@ type ListImagesOptions struct { // * readonly=true,false // * reference=name[:tag] (wildcards allowed) Filters []string + // IsExternalContainerFunc allows for checking whether the specified + // container is an external one (when containers=external filter is + // used). The definition of an external container can be set by + // callers. + IsExternalContainerFunc IsExternalContainerFunc } // ListImages lists images in the local container storage. If names are @@ -525,7 +537,7 @@ func (r *Runtime) ListImages(ctx context.Context, names []string, options *ListI var filters []filterFunc if len(options.Filters) > 0 { - compiledFilters, err := r.compileImageFilters(ctx, options.Filters) + compiledFilters, err := r.compileImageFilters(ctx, options) if err != nil { return nil, err } @@ -550,8 +562,17 @@ type RemoveImagesOptions struct { // containers using a specific image. By default, all containers in // the local containers storage will be removed (if Force is set). RemoveContainerFunc RemoveContainerFunc + // IsExternalContainerFunc allows for checking whether the specified + // container is an external one (when containers=external filter is + // used). The definition of an external container can be set by + // callers. + IsExternalContainerFunc IsExternalContainerFunc + // Remove external containers even when Force is false. Requires + // IsExternalContainerFunc to be specified. + ExternalContainers bool // Filters to filter the removed images. Supported filters are // * after,before,since=image + // * containers=true,false,external // * dangling=true,false // * intermediate=true,false (useful for pruning images) // * id=id @@ -581,6 +602,10 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem options = &RemoveImagesOptions{} } + if options.ExternalContainers && options.IsExternalContainerFunc == nil { + return nil, []error{fmt.Errorf("libimage error: cannot remove external containers without callback")} + } + // The logic here may require some explanation. Image removal is // surprisingly complex since it is recursive (intermediate parents are // removed) and since multiple items in `names` may resolve to the @@ -635,7 +660,11 @@ func (r *Runtime) RemoveImages(ctx context.Context, names []string, options *Rem } default: - filteredImages, err := r.ListImages(ctx, nil, &ListImagesOptions{Filters: options.Filters}) + options := &ListImagesOptions{ + IsExternalContainerFunc: options.IsExternalContainerFunc, + Filters: options.Filters, + } + filteredImages, err := r.ListImages(ctx, nil, options) if err != nil { appendError(err) return nil, rmErrors