podman rmi: refactor logic

While this commit was initially meant to fix #5847, it has turned into a
bigger refactoring which I did not manage to break into smaller pieces:

 * Fix #5847 by refactoring the image-removal logic.

 * Make the api handler for image-removal use the ABI code. This way,
   both (i.e., ABI and Tunnel) end up using the same code.  Achieving
   this code share required to move some code around to prevent circular
   dependencies.

 * Everything in pkg/api (excluding pkg/api/types) must now only be
   accessed from code using `ABISupport`.

 * Avoid imports from entities on handlers to prevent circular
   dependencies.

 * Move `podman system service` logic into `cmd` to prevent circular
   dependencies - it depends on pkg/api.

 * Also remove the build header from infra/abi files.  It will otherwise
   confuse swagger and other tools; errors we cannot fix as go doesn't
   expose a build-tag env variable.

Fixes: #5847
Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

14 files changed, 137 insertions, 157 deletions

diff --git a/pkg/domain/infra/abi/containers.go b/pkg/domain/infra/abi/containers.go
index 50003dbe2..e71ceb536 100644
--- a/pkg/domain/infra/abi/containers.go
+++ b/pkg/domain/infra/abi/containers.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package abi
 
 import (
diff --git a/pkg/domain/infra/abi/events.go b/pkg/domain/infra/abi/events.go
index 9540a5b96..20773cdce 100644
--- a/pkg/domain/infra/abi/events.go
+++ b/pkg/domain/infra/abi/events.go
@@ -1,5 +1,3 @@
-//+build ABISupport
-
 package abi
 
 import (
diff --git a/pkg/domain/infra/abi/healthcheck.go b/pkg/domain/infra/abi/healthcheck.go
index 699483243..351bf4f7e 100644
--- a/pkg/domain/infra/abi/healthcheck.go
+++ b/pkg/domain/infra/abi/healthcheck.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package abi
 
 import (
diff --git a/pkg/domain/infra/abi/images.go b/pkg/domain/infra/abi/images.go
index 4353e0798..32f7d75e5 100644
--- a/pkg/domain/infra/abi/images.go
+++ b/pkg/domain/infra/abi/images.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package abi
 
 import (
@@ -23,6 +21,7 @@ import (
 	domainUtils "github.com/containers/libpod/pkg/domain/utils"
 	"github.com/containers/libpod/pkg/util"
 	"github.com/containers/storage"
+	"github.com/hashicorp/go-multierror"
 	imgspecv1 "github.com/opencontainers/image-spec/specs-go/v1"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
@@ -36,76 +35,6 @@ func (ir *ImageEngine) Exists(_ context.Context, nameOrId string) (*entities.Boo
 	return &entities.BoolReport{Value: err == nil}, nil
 }
 
-func (ir *ImageEngine) Delete(ctx context.Context, nameOrId []string, opts entities.ImageDeleteOptions) (*entities.ImageDeleteReport, error) {
-	report := entities.ImageDeleteReport{}
-
-	if opts.All {
-		var previousTargets []*libpodImage.Image
-	repeatRun:
-		targets, err := ir.Libpod.ImageRuntime().GetRWImages()
-		if err != nil {
-			return &report, errors.Wrapf(err, "unable to query local images")
-		}
-		if len(targets) == 0 {
-			return &report, nil
-		}
-		if len(targets) > 0 && len(targets) == len(previousTargets) {
-			return &report, errors.New("unable to delete all images; re-run the rmi command again.")
-		}
-		previousTargets = targets
-
-		for _, img := range targets {
-			isParent, err := img.IsParent(ctx)
-			if err != nil {
-				return &report, err
-			}
-			if isParent {
-				continue
-			}
-			err = ir.deleteImage(ctx, img, opts, report)
-			report.Errors = append(report.Errors, err)
-		}
-		if len(previousTargets) != 1 {
-			goto repeatRun
-		}
-		return &report, nil
-	}
-
-	for _, id := range nameOrId {
-		image, err := ir.Libpod.ImageRuntime().NewFromLocal(id)
-		if err != nil {
-			return nil, err
-		}
-
-		err = ir.deleteImage(ctx, image, opts, report)
-		if err != nil {
-			return &report, err
-		}
-	}
-	return &report, nil
-}
-
-func (ir *ImageEngine) deleteImage(ctx context.Context, img *libpodImage.Image, opts entities.ImageDeleteOptions, report entities.ImageDeleteReport) error {
-	results, err := ir.Libpod.RemoveImage(ctx, img, opts.Force)
-	switch errors.Cause(err) {
-	case nil:
-		break
-	case storage.ErrImageUsedByContainer:
-		report.ImageInUse = errors.New(
-			fmt.Sprintf("A container associated with containers/storage, i.e. via Buildah, CRI-O, etc., may be associated with this image: %-12.12s\n", img.ID()))
-		return nil
-	case libpodImage.ErrNoSuchImage:
-		report.ImageNotFound = err
-		return nil
-	default:
-		return err
-	}
-
-	report.Deleted = append(report.Deleted, results.Deleted)
-	report.Untagged = append(report.Untagged, results.Untagged...)
-	return nil
-}
-
 func (ir *ImageEngine) Prune(ctx context.Context, opts entities.ImagePruneOptions) (*entities.ImagePruneReport, error) {
 	results, err := ir.Libpod.ImageRuntime().PruneImages(ctx, opts.All, opts.Filter)
 	if err != nil {
@@ -488,3 +417,135 @@ func (ir *ImageEngine) Tree(ctx context.Context, nameOrId string, opts entities.
 	}
 	return &entities.ImageTreeReport{Tree: results}, nil
 }
+
+// Remove removes one or more images from local storage.
+func (ir *ImageEngine) Remove(ctx context.Context, images []string, opts entities.ImageRemoveOptions) (report *entities.ImageRemoveReport, finalError error) {
+	var (
+		// noSuchImageErrors indicates that at least one image was not found.
+		noSuchImageErrors bool
+		// inUseErrors indicates that at least one image is being used by a
+		// container.
+		inUseErrors bool
+		// otherErrors indicates that at least one error other than the two
+		// above occured.
+		otherErrors bool
+		// deleteError is a multierror to conveniently collect errors during
+		// removal. We really want to delete as many images as possible and not
+		// error out immediately.
+		deleteError *multierror.Error
+	)
+
+	report = &entities.ImageRemoveReport{}
+
+	// Set the removalCode and the error after all work is done.
+	defer func() {
+		switch {
+		// 2
+		case inUseErrors:
+			// One of the specified images has child images or is
+			// being used by a container.
+			report.ExitCode = 2
+		// 1
+		case noSuchImageErrors && !(otherErrors || inUseErrors):
+			// One of the specified images did not exist, and no other
+			// failures.
+			report.ExitCode = 1
+		// 0
+		default:
+			// Nothing to do.
+		}
+		if deleteError != nil {
+			// go-multierror has a trailing new line which we need to remove to normalize the string.
+			finalError = deleteError.ErrorOrNil()
+			finalError = errors.New(strings.TrimSpace(finalError.Error()))
+		}
+	}()
+
+	// deleteImage is an anonymous function to conveniently delete an image
+	// withouth having to pass all local data around.
+	deleteImage := func(img *image.Image) error {
+		results, err := ir.Libpod.RemoveImage(ctx, img, opts.Force)
+		switch errors.Cause(err) {
+		case nil:
+			break
+		case storage.ErrImageUsedByContainer:
+			inUseErrors = true // Important for exit codes in Podman.
+			return errors.New(
+				fmt.Sprintf("A container associated with containers/storage, i.e. via Buildah, CRI-O, etc., may be associated with this image: %-12.12s\n", img.ID()))
+		default:
+			otherErrors = true // Important for exit codes in Podman.
+			return err
+		}
+
+		report.Deleted = append(report.Deleted, results.Deleted)
+		report.Untagged = append(report.Untagged, results.Untagged...)
+		return nil
+	}
+
+	// Delete all images from the local storage.
+	if opts.All {
+		previousImages := 0
+		// Remove all images one-by-one.
+		for {
+			storageImages, err := ir.Libpod.ImageRuntime().GetRWImages()
+			if err != nil {
+				deleteError = multierror.Append(deleteError,
+					errors.Wrapf(err, "unable to query local images"))
+				otherErrors = true // Important for exit codes in Podman.
+				return
+			}
+			// No images (left) to remove, so we're done.
+			if len(storageImages) == 0 {
+				return
+			}
+			// Prevent infinity loops by making a delete-progress check.
+			if previousImages == len(storageImages) {
+				otherErrors = true // Important for exit codes in Podman.
+				deleteError = multierror.Append(deleteError,
+					errors.New("unable to delete all images, check errors and re-run image removal if needed"))
+				break
+			}
+			previousImages = len(storageImages)
+			// Delete all "leaves" (i.e., images without child images).
+			for _, img := range storageImages {
+				isParent, err := img.IsParent(ctx)
+				if err != nil {
+					otherErrors = true // Important for exit codes in Podman.
+					deleteError = multierror.Append(deleteError, err)
+				}
+				// Skip parent images.
+				if isParent {
+					continue
+				}
+				if err := deleteImage(img); err != nil {
+					deleteError = multierror.Append(deleteError, err)
+				}
+			}
+		}
+
+		return
+	}
+
+	// Delete only the specified images.
+	for _, id := range images {
+		img, err := ir.Libpod.ImageRuntime().NewFromLocal(id)
+		switch errors.Cause(err) {
+		case nil:
+			break
+		case image.ErrNoSuchImage:
+			noSuchImageErrors = true // Important for exit codes in Podman.
+			fallthrough
+		default:
+			deleteError = multierror.Append(deleteError, err)
+			continue
+		}
+
+		err = deleteImage(img)
+		if err != nil {
+			otherErrors = true // Important for exit codes in Podman.
+			deleteError = multierror.Append(deleteError, err)
+		}
+	}
+
+	return
+}
diff --git a/pkg/domain/infra/abi/images_list.go b/pkg/domain/infra/abi/images_list.go
index 68b961cb6..9add915ea 100644
--- a/pkg/domain/infra/abi/images_list.go
+++ b/pkg/domain/infra/abi/images_list.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package abi
 
 import (
diff --git a/pkg/domain/infra/abi/pods.go b/pkg/domain/infra/abi/pods.go
index 6b6e13e24..c4ae9efbf 100644
--- a/pkg/domain/infra/abi/pods.go
+++ b/pkg/domain/infra/abi/pods.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package abi
 
 import (
diff --git a/pkg/domain/infra/abi/runtime.go b/pkg/domain/infra/abi/runtime.go
index b53fb6d3a..7394cadfc 100644
--- a/pkg/domain/infra/abi/runtime.go
+++ b/pkg/domain/infra/abi/runtime.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package abi
 
 import (
diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go
index 078f5404d..e5c109ee6 100644
--- a/pkg/domain/infra/abi/system.go
+++ b/pkg/domain/infra/abi/system.go
@@ -1,20 +1,15 @@
-// +build ABISupport
-
 package abi
 
 import (
 	"context"
 	"fmt"
 	"io/ioutil"
-	"net"
 	"os"
 	"strconv"
-	"strings"
 	"syscall"
 
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/libpod/libpod/define"
-	api "github.com/containers/libpod/pkg/api/server"
 	"github.com/containers/libpod/pkg/cgroups"
 	"github.com/containers/libpod/pkg/domain/entities"
 	"github.com/containers/libpod/pkg/rootless"
@@ -33,42 +28,6 @@ func (ic *ContainerEngine) Info(ctx context.Context) (*define.Info, error) {
 	return ic.Libpod.Info()
 }
 
-func (ic *ContainerEngine) RestService(_ context.Context, opts entities.ServiceOptions) error {
-	var (
-		listener *net.Listener
-		err      error
-	)
-
-	if opts.URI != "" {
-		fields := strings.Split(opts.URI, ":")
-		if len(fields) == 1 {
-			return errors.Errorf("%s is an invalid socket destination", opts.URI)
-		}
-		address := strings.Join(fields[1:], ":")
-		l, err := net.Listen(fields[0], address)
-		if err != nil {
-			return errors.Wrapf(err, "unable to create socket %s", opts.URI)
-		}
-		listener = &l
-	}
-
-	server, err := api.NewServerWithSettings(ic.Libpod, opts.Timeout, listener)
-	if err != nil {
-		return err
-	}
-	defer func() {
-		if err := server.Shutdown(); err != nil {
-			logrus.Warnf("Error when stopping API service: %s", err)
-		}
-	}()
-
-	err = server.Serve()
-	if listener != nil {
-		_ = (*listener).Close()
-	}
-	return err
-}
-
 func (ic *ContainerEngine) VarlinkService(_ context.Context, opts entities.ServiceOptions) error {
 	var varlinkInterfaces = []*iopodman.VarlinkInterface{
 		iopodmanAPI.New(opts.Command, ic.Libpod),
diff --git a/pkg/domain/infra/abi/terminal/sigproxy_linux.go b/pkg/domain/infra/abi/terminal/sigproxy_linux.go
index d7f5853d8..b422e549e 100644
--- a/pkg/domain/infra/abi/terminal/sigproxy_linux.go
+++ b/pkg/domain/infra/abi/terminal/sigproxy_linux.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package terminal
 
 import (
diff --git a/pkg/domain/infra/abi/terminal/terminal.go b/pkg/domain/infra/abi/terminal/terminal.go
index f187bdd6b..0fc3af511 100644
--- a/pkg/domain/infra/abi/terminal/terminal.go
+++ b/pkg/domain/infra/abi/terminal/terminal.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package terminal
 
 import (
diff --git a/pkg/domain/infra/abi/terminal/terminal_linux.go b/pkg/domain/infra/abi/terminal/terminal_linux.go
index 664205df1..15701342f 100644
--- a/pkg/domain/infra/abi/terminal/terminal_linux.go
+++ b/pkg/domain/infra/abi/terminal/terminal_linux.go
@@ -1,5 +1,3 @@
-// +build ABISupport
-
 package terminal
 
 import (
diff --git a/pkg/domain/infra/tunnel/events.go b/pkg/domain/infra/tunnel/events.go
index 46d88341a..93da3aeb4 100644
--- a/pkg/domain/infra/tunnel/events.go
+++ b/pkg/domain/infra/tunnel/events.go
@@ -4,7 +4,6 @@ import (
 	"context"
 	"strings"
 
-	"github.com/containers/libpod/pkg/api/handlers"
 	"github.com/containers/libpod/pkg/bindings/system"
 	"github.com/containers/libpod/pkg/domain/entities"
 	"github.com/pkg/errors"
@@ -21,10 +20,10 @@ func (ic *ContainerEngine) Events(ctx context.Context, opts entities.EventsOptio
 			filters[split[0]] = append(filters[split[0]], strings.Join(split[1:], "="))
 		}
 	}
-	binChan := make(chan handlers.Event)
+	binChan := make(chan entities.Event)
 	go func() {
 		for e := range binChan {
-			opts.EventChan <- e.ToLibpodEvent()
+			opts.EventChan <- entities.ConvertToLibpodEvent(e)
 		}
 	}()
 	return system.Events(ic.ClientCxt, binChan, nil, &opts.Since, &opts.Until, filters)
diff --git a/pkg/domain/infra/tunnel/images.go b/pkg/domain/infra/tunnel/images.go
index 2decd605d..822842936 100644
--- a/pkg/domain/infra/tunnel/images.go
+++ b/pkg/domain/infra/tunnel/images.go
@@ -19,25 +19,8 @@ func (ir *ImageEngine) Exists(_ context.Context, nameOrId string) (*entities.Boo
 	return &entities.BoolReport{Value: found}, err
 }
 
-func (ir *ImageEngine) Delete(ctx context.Context, nameOrId []string, opts entities.ImageDeleteOptions) (*entities.ImageDeleteReport, error) {
-	report := entities.ImageDeleteReport{}
-
-	for _, id := range nameOrId {
-		results, err := images.Remove(ir.ClientCxt, id, &opts.Force)
-		if err != nil {
-			return nil, err
-		}
-		for _, e := range results {
-			if a, ok := e["Deleted"]; ok {
-				report.Deleted = append(report.Deleted, a)
-			}
-
-			if a, ok := e["Untagged"]; ok {
-				report.Untagged = append(report.Untagged, a)
-			}
-		}
-	}
-	return &report, nil
+func (ir *ImageEngine) Remove(ctx context.Context, imagesArg []string, opts entities.ImageRemoveOptions) (*entities.ImageRemoveReport, error) {
+	return images.Remove(ir.ClientCxt, imagesArg, opts)
 }
 
 func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions) ([]*entities.ImageSummary, error) {
diff --git a/pkg/domain/infra/tunnel/system.go b/pkg/domain/infra/tunnel/system.go
index f373525c5..97bf885e7 100644
--- a/pkg/domain/infra/tunnel/system.go
+++ b/pkg/domain/infra/tunnel/system.go
@@ -14,10 +14,6 @@ func (ic *ContainerEngine) Info(ctx context.Context) (*define.Info, error) {
 	return system.Info(ic.ClientCxt)
 }
 
-func (ic *ContainerEngine) RestService(_ context.Context, _ entities.ServiceOptions) error {
-	panic(errors.New("rest service is not supported when tunneling"))
-}
-
 func (ic *ContainerEngine) VarlinkService(_ context.Context, _ entities.ServiceOptions) error {
 	panic(errors.New("varlink service is not supported when tunneling"))
 }