Merge pull request #10880 from vrothberg/auto-update

auto-update: make output more user friendly

5 files changed, 152 insertions, 110 deletions

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 0a13e7e74..fd95c319c 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -9,12 +9,13 @@ import (
 	"github.com/containers/common/pkg/config"
 	"github.com/containers/image/v5/docker"
 	"github.com/containers/image/v5/docker/reference"
-	"github.com/containers/image/v5/manifest"
 	"github.com/containers/image/v5/transports/alltransports"
 	"github.com/containers/podman/v3/libpod"
 	"github.com/containers/podman/v3/libpod/define"
+	"github.com/containers/podman/v3/pkg/domain/entities"
 	"github.com/containers/podman/v3/pkg/systemd"
 	systemdDefine "github.com/containers/podman/v3/pkg/systemd/define"
+	"github.com/coreos/go-systemd/v22/dbus"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
 )
@@ -119,7 +120,7 @@ func ValidateImageReference(imageName string) error {
 //
 // It returns a slice of successfully restarted systemd units and a slice of
 // errors encountered during auto update.
-func AutoUpdate(runtime *libpod.Runtime, options Options) ([]string, []error) {
+func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options Options) ([]*entities.AutoUpdateReport, []error) {
 	// Create a map from `image ID -> []*Container`.
 	containerMap, errs := imageContainersMap(runtime)
 	if len(containerMap) == 0 {
@@ -130,7 +131,7 @@ func AutoUpdate(runtime *libpod.Runtime, options Options) ([]string, []error) {
 	listOptions := &libimage.ListImagesOptions{
 		Filters: []string{"readonly=false"},
 	}
-	imagesSlice, err := runtime.LibimageRuntime().ListImages(context.Background(), nil, listOptions)
+	imagesSlice, err := runtime.LibimageRuntime().ListImages(ctx, nil, listOptions)
 	if err != nil {
 		return nil, []error{err}
 	}
@@ -147,8 +148,8 @@ func AutoUpdate(runtime *libpod.Runtime, options Options) ([]string, []error) {
 	}
 	defer conn.Close()
 
-	// Update images.
-	containersToRestart := []*libpod.Container{}
+	// Update all images/container according to their auto-update policy.
+	var allReports []*entities.AutoUpdateReport
 	updatedRawImages := make(map[string]bool)
 	for imageID, policyMapper := range containerMap {
 		image, exists := imageMap[imageID]
@@ -156,76 +157,139 @@ func AutoUpdate(runtime *libpod.Runtime, options Options) ([]string, []error) {
 			errs = append(errs, errors.Errorf("container image ID %q not found in local storage", imageID))
 			return nil, errs
 		}
-		// Now we have to check if the image of any containers must be updated.
-		// Note that the image ID is NOT enough for this check as a given image
-		// may have multiple tags.
-		for _, registryCtr := range policyMapper[PolicyRegistryImage] {
-			cid := registryCtr.ID()
-			rawImageName := registryCtr.RawImageName()
-			if rawImageName == "" {
-				errs = append(errs, errors.Errorf("error registry auto-updating container %q: raw-image name is empty", cid))
-			}
-			readAuthenticationPath(registryCtr, options)
-			needsUpdate, err := newerRemoteImageAvailable(runtime, image, rawImageName, options)
+
+		for _, ctr := range policyMapper[PolicyRegistryImage] {
+			report, err := autoUpdateRegistry(ctx, image, ctr, updatedRawImages, &options, conn, runtime)
 			if err != nil {
-				errs = append(errs, errors.Wrapf(err, "error registry auto-updating container %q: image check for %q failed", cid, rawImageName))
-				continue
+				errs = append(errs, err)
 			}
-
-			if needsUpdate {
-				logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-				if _, updated := updatedRawImages[rawImageName]; !updated {
-					_, err = updateImage(runtime, rawImageName, options)
-					if err != nil {
-						errs = append(errs, errors.Wrapf(err, "error registry auto-updating container %q: image update for %q failed", cid, rawImageName))
-						continue
-					}
-					updatedRawImages[rawImageName] = true
-				}
-				containersToRestart = append(containersToRestart, registryCtr)
+			if report != nil {
+				allReports = append(allReports, report)
 			}
 		}
 
-		for _, localCtr := range policyMapper[PolicyLocalImage] {
-			cid := localCtr.ID()
-			rawImageName := localCtr.RawImageName()
-			if rawImageName == "" {
-				errs = append(errs, errors.Errorf("error locally auto-updating container %q: raw-image name is empty", cid))
-			}
-			// This avoids restarting containers unnecessarily.
-			needsUpdate, err := newerLocalImageAvailable(runtime, image, rawImageName)
+		for _, ctr := range policyMapper[PolicyLocalImage] {
+			report, err := autoUpdateLocally(ctx, image, ctr, &options, conn, runtime)
 			if err != nil {
-				errs = append(errs, errors.Wrapf(err, "error locally auto-updating container %q: image check for %q failed", cid, rawImageName))
-				continue
+				errs = append(errs, err)
 			}
-
-			if needsUpdate {
-				logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName)
-				containersToRestart = append(containersToRestart, localCtr)
+			if report != nil {
+				allReports = append(allReports, report)
 			}
 		}
 	}
 
-	// Restart containers.
-	updatedUnits := []string{}
-	for _, ctr := range containersToRestart {
-		labels := ctr.Labels()
-		unit, exists := labels[systemdDefine.EnvVariable]
-		if !exists {
-			// Shouldn't happen but let's be sure of it.
-			errs = append(errs, errors.Errorf("error auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable))
-			continue
-		}
-		_, err := conn.RestartUnit(unit, "replace", nil)
-		if err != nil {
-			errs = append(errs, errors.Wrapf(err, "error auto-updating container %q: restarting systemd unit %q failed", ctr.ID(), unit))
-			continue
+	return allReports, errs
+}
+
+// autoUpdateRegistry updates the image/container according to the "registry" policy.
+func autoUpdateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.Container, updatedRawImages map[string]bool, options *Options, conn *dbus.Conn, runtime *libpod.Runtime) (*entities.AutoUpdateReport, error) {
+	cid := ctr.ID()
+	rawImageName := ctr.RawImageName()
+	if rawImageName == "" {
+		return nil, errors.Errorf("error registry auto-updating container %q: raw-image name is empty", cid)
+	}
+
+	labels := ctr.Labels()
+	unit, exists := labels[systemdDefine.EnvVariable]
+	if !exists {
+		return nil, errors.Errorf("error auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable)
+	}
+
+	report := &entities.AutoUpdateReport{
+		ContainerID:   cid,
+		ContainerName: ctr.Name(),
+		ImageName:     rawImageName,
+		Policy:        PolicyRegistryImage,
+		SystemdUnit:   unit,
+		Updated:       "failed",
+	}
+
+	if _, updated := updatedRawImages[rawImageName]; updated {
+		logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
+		if err := restartSystemdUnit(ctr, unit, conn); err != nil {
+			return report, err
 		}
-		logrus.Infof("Successfully restarted systemd unit %q", unit)
-		updatedUnits = append(updatedUnits, unit)
+		report.Updated = "true"
+		return report, nil
+	}
+
+	authfile := getAuthfilePath(ctr, options)
+	needsUpdate, err := newerRemoteImageAvailable(ctx, runtime, image, rawImageName, authfile)
+	if err != nil {
+		return report, errors.Wrapf(err, "error registry auto-updating container %q: image check for %q failed", cid, rawImageName)
+	}
+
+	if !needsUpdate {
+		report.Updated = "false"
+		return report, nil
+	}
+
+	if _, err := updateImage(ctx, runtime, rawImageName, options); err != nil {
+		return report, errors.Wrapf(err, "error registry auto-updating container %q: image update for %q failed", cid, rawImageName)
+	}
+	updatedRawImages[rawImageName] = true
+
+	logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
+	if err := restartSystemdUnit(ctr, unit, conn); err != nil {
+		return report, err
 	}
 
-	return updatedUnits, errs
+	report.Updated = "true"
+	return report, nil
+}
+
+// autoUpdateRegistry updates the image/container according to the "local" policy.
+func autoUpdateLocally(ctx context.Context, image *libimage.Image, ctr *libpod.Container, options *Options, conn *dbus.Conn, runtime *libpod.Runtime) (*entities.AutoUpdateReport, error) {
+	cid := ctr.ID()
+	rawImageName := ctr.RawImageName()
+	if rawImageName == "" {
+		return nil, errors.Errorf("error locally auto-updating container %q: raw-image name is empty", cid)
+	}
+
+	labels := ctr.Labels()
+	unit, exists := labels[systemdDefine.EnvVariable]
+	if !exists {
+		return nil, errors.Errorf("error auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable)
+	}
+
+	report := &entities.AutoUpdateReport{
+		ContainerID:   cid,
+		ContainerName: ctr.Name(),
+		ImageName:     rawImageName,
+		Policy:        PolicyLocalImage,
+		SystemdUnit:   unit,
+		Updated:       "failed",
+	}
+
+	needsUpdate, err := newerLocalImageAvailable(runtime, image, rawImageName)
+	if err != nil {
+		return report, errors.Wrapf(err, "error locally auto-updating container %q: image check for %q failed", cid, rawImageName)
+	}
+
+	if !needsUpdate {
+		report.Updated = "false"
+		return report, nil
+	}
+
+	logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName)
+	if err := restartSystemdUnit(ctr, unit, conn); err != nil {
+		return report, err
+	}
+
+	report.Updated = "true"
+	return report, nil
+}
+
+// restartSystemdUnit restarts the systemd unit the container is running in.
+func restartSystemdUnit(ctr *libpod.Container, unit string, conn *dbus.Conn) error {
+	_, err := conn.RestartUnit(unit, "replace", nil)
+	if err != nil {
+		return errors.Wrapf(err, "error auto-updating container %q: restarting systemd unit %q failed", ctr.ID(), unit)
+	}
+
+	logrus.Infof("Successfully restarted systemd unit %q of container %q", unit, ctr.ID())
+	return nil
 }
 
 // imageContainersMap generates a map[image ID] -> [containers using the image]
@@ -280,52 +344,25 @@ func imageContainersMap(runtime *libpod.Runtime) (map[string]policyMapper, []err
 	return containerMap, errors
 }
 
-// readAuthenticationPath reads a container's labels and reads authentication path into options
-func readAuthenticationPath(ctr *libpod.Container, options Options) {
+// getAuthfilePath returns an authfile path, if set. The authfile label in the
+// container, if set, as precedence over the one set in the options.
+func getAuthfilePath(ctr *libpod.Container, options *Options) string {
 	labels := ctr.Labels()
 	authFilePath, exists := labels[AuthfileLabel]
 	if exists {
-		options.Authfile = authFilePath
+		return authFilePath
 	}
+	return options.Authfile
 }
 
 // newerRemoteImageAvailable returns true if there corresponding image on the remote
 // registry is newer.
-func newerRemoteImageAvailable(runtime *libpod.Runtime, img *libimage.Image, origName string, options Options) (bool, error) {
+func newerRemoteImageAvailable(ctx context.Context, runtime *libpod.Runtime, img *libimage.Image, origName string, authfile string) (bool, error) {
 	remoteRef, err := docker.ParseReference("//" + origName)
 	if err != nil {
 		return false, err
 	}
-
-	data, err := img.Inspect(context.Background(), false)
-	if err != nil {
-		return false, err
-	}
-
-	sys := runtime.SystemContext()
-	sys.AuthFilePath = options.Authfile
-
-	// We need to account for the arch that the image uses.  It seems
-	// common on ARM to tweak this option to pull the correct image.  See
-	// github.com/containers/podman/issues/6613.
-	sys.ArchitectureChoice = data.Architecture
-
-	remoteImg, err := remoteRef.NewImage(context.Background(), sys)
-	if err != nil {
-		return false, err
-	}
-
-	rawManifest, _, err := remoteImg.Manifest(context.Background())
-	if err != nil {
-		return false, err
-	}
-
-	remoteDigest, err := manifest.Digest(rawManifest)
-	if err != nil {
-		return false, err
-	}
-
-	return img.Digest().String() != remoteDigest.String(), nil
+	return img.HasDifferentDigest(ctx, remoteRef)
 }
 
 // newerLocalImageAvailable returns true if the container and local image have different digests
@@ -334,21 +371,16 @@ func newerLocalImageAvailable(runtime *libpod.Runtime, img *libimage.Image, rawI
 	if err != nil {
 		return false, err
 	}
-
-	localDigest := localImg.Digest().String()
-
-	ctrDigest := img.Digest().String()
-
-	return localDigest != ctrDigest, nil
+	return localImg.Digest().String() != img.Digest().String(), nil
 }
 
 // updateImage pulls the specified image.
-func updateImage(runtime *libpod.Runtime, name string, options Options) (*libimage.Image, error) {
+func updateImage(ctx context.Context, runtime *libpod.Runtime, name string, options *Options) (*libimage.Image, error) {
 	pullOptions := &libimage.PullOptions{}
 	pullOptions.AuthFilePath = options.Authfile
 	pullOptions.Writer = os.Stderr
 
-	pulledImages, err := runtime.LibimageRuntime().Pull(context.Background(), name, config.PullPolicyAlways, pullOptions)
+	pulledImages, err := runtime.LibimageRuntime().Pull(ctx, name, config.PullPolicyAlways, pullOptions)
 	if err != nil {
 		return nil, err
 	}
diff --git a/pkg/domain/entities/auto-update.go b/pkg/domain/entities/auto-update.go
index c51158816..d74462b86 100644
--- a/pkg/domain/entities/auto-update.go
+++ b/pkg/domain/entities/auto-update.go
@@ -8,6 +8,17 @@ type AutoUpdateOptions struct {
 
 // AutoUpdateReport contains the results from running auto-update.
 type AutoUpdateReport struct {
-	// Units - the restarted systemd units during auto-update.
-	Units []string
+	// ID of the container *before* an update.
+	ContainerID string
+	// Name of the container *before* an update.
+	ContainerName string
+	// Name of the image.
+	ImageName string
+	// The configured auto-update policy.
+	Policy string
+	// SystemdUnit running a container configured for auto updates.
+	SystemdUnit string
+	// Indicates whether the image was updated and the container (and
+	// systemd unit) restarted.
+	Updated string
 }
diff --git a/pkg/domain/entities/engine_container.go b/pkg/domain/entities/engine_container.go
index 28e5160db..62e83fab3 100644
--- a/pkg/domain/entities/engine_container.go
+++ b/pkg/domain/entities/engine_container.go
@@ -14,7 +14,7 @@ import (
 type ContainerCopyFunc func() error
 
 type ContainerEngine interface {
-	AutoUpdate(ctx context.Context, options AutoUpdateOptions) (*AutoUpdateReport, []error)
+	AutoUpdate(ctx context.Context, options AutoUpdateOptions) ([]*AutoUpdateReport, []error)
 	Config(ctx context.Context) (*config.Config, error)
 	ContainerAttach(ctx context.Context, nameOrID string, options AttachOptions) error
 	ContainerCheckpoint(ctx context.Context, namesOrIds []string, options CheckpointOptions) ([]*CheckpointReport, error)
diff --git a/pkg/domain/infra/abi/auto-update.go b/pkg/domain/infra/abi/auto-update.go
index c9d7f2130..daa882ecf 100644
--- a/pkg/domain/infra/abi/auto-update.go
+++ b/pkg/domain/infra/abi/auto-update.go
@@ -7,11 +7,10 @@ import (
 	"github.com/containers/podman/v3/pkg/domain/entities"
 )
 
-func (ic *ContainerEngine) AutoUpdate(ctx context.Context, options entities.AutoUpdateOptions) (*entities.AutoUpdateReport, []error) {
+func (ic *ContainerEngine) AutoUpdate(ctx context.Context, options entities.AutoUpdateOptions) ([]*entities.AutoUpdateReport, []error) {
 	// Convert the entities options to the autoupdate ones.  We can't use
 	// them in the entities package as low-level packages must not leak
 	// into the remote client.
 	autoOpts := autoupdate.Options{Authfile: options.Authfile}
-	units, failures := autoupdate.AutoUpdate(ic.Libpod, autoOpts)
-	return &entities.AutoUpdateReport{Units: units}, failures
+	return autoupdate.AutoUpdate(ctx, ic.Libpod, autoOpts)
 }
diff --git a/pkg/domain/infra/tunnel/auto-update.go b/pkg/domain/infra/tunnel/auto-update.go
index 41165cc74..038c60537 100644
--- a/pkg/domain/infra/tunnel/auto-update.go
+++ b/pkg/domain/infra/tunnel/auto-update.go
@@ -7,6 +7,6 @@ import (
 	"github.com/pkg/errors"
 )
 
-func (ic *ContainerEngine) AutoUpdate(ctx context.Context, options entities.AutoUpdateOptions) (*entities.AutoUpdateReport, []error) {
+func (ic *ContainerEngine) AutoUpdate(ctx context.Context, options entities.AutoUpdateOptions) ([]*entities.AutoUpdateReport, []error) {
 	return nil, []error{errors.New("not implemented")}
 }