From d4760977392028f0228ba76e9c09575c97a0111e Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Thu, 28 Jul 2022 14:20:50 +0200
Subject: pkg/autoupdate: introduce `updater` for shared state

Introduce an `updater` type to allow for sharing state.
This will be more useful for future changes.

[NO NEW TESTS NEEDED] as it does not change behavior.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 60 +++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 23 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 8d9991622..c792ee4e9 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -145,9 +145,15 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 
 	runtime.NewSystemEvent(events.AutoUpdate)
 
+	auto := updater{
+		conn:             conn,
+		options:          &options,
+		runtime:          runtime,
+		updatedRawImages: make(map[string]bool),
+	}
+
 	// 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]
 		if !exists {
@@ -156,7 +162,7 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 		}
 
 		for _, ctr := range policyMapper[PolicyRegistryImage] {
-			report, err := autoUpdateRegistry(ctx, image, ctr, updatedRawImages, &options, conn, runtime)
+			report, err := auto.updateRegistry(ctx, image, ctr)
 			if err != nil {
 				errs = append(errs, err)
 			}
@@ -166,7 +172,7 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 		}
 
 		for _, ctr := range policyMapper[PolicyLocalImage] {
-			report, err := autoUpdateLocally(ctx, image, ctr, &options, conn, runtime)
+			report, err := auto.updateLocally(ctx, image, ctr)
 			if err != nil {
 				errs = append(errs, err)
 			}
@@ -179,8 +185,16 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 	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 *entities.AutoUpdateOptions, conn *dbus.Conn, runtime *libpod.Runtime) (*entities.AutoUpdateReport, error) {
+// updater includes shared state for auto-updating one or more containers.
+type updater struct {
+	conn             *dbus.Conn
+	options          *entities.AutoUpdateOptions
+	updatedRawImages map[string]bool
+	runtime          *libpod.Runtime
+}
+
+// updateRegistry updates the image/container according to the "registry" policy.
+func (u *updater) updateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.Container) (*entities.AutoUpdateReport, error) {
 	cid := ctr.ID()
 	rawImageName := ctr.RawImageName()
 	if rawImageName == "" {
@@ -202,16 +216,16 @@ func autoUpdateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.
 		Updated:       "failed",
 	}
 
-	if _, updated := updatedRawImages[rawImageName]; updated {
+	if _, updated := u.updatedRawImages[rawImageName]; updated {
 		logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-		if err := restartSystemdUnit(ctx, ctr, unit, conn); err != nil {
+		if err := u.restartSystemdUnit(ctx, ctr, unit); err != nil {
 			return report, err
 		}
 		report.Updated = "true"
 		return report, nil
 	}
 
-	authfile := getAuthfilePath(ctr, options)
+	authfile := getAuthfilePath(ctr, u.options)
 	needsUpdate, err := newerRemoteImageAvailable(ctx, image, rawImageName, authfile)
 	if err != nil {
 		return report, fmt.Errorf("registry auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
@@ -222,24 +236,24 @@ func autoUpdateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.
 		return report, nil
 	}
 
-	if options.DryRun {
+	if u.options.DryRun {
 		report.Updated = "pending"
 		return report, nil
 	}
 
-	if _, err := updateImage(ctx, runtime, rawImageName, authfile); err != nil {
+	if _, err := updateImage(ctx, u.runtime, rawImageName, authfile); err != nil {
 		return report, fmt.Errorf("registry auto-updating container %q: image update for %q failed: %w", cid, rawImageName, err)
 	}
-	updatedRawImages[rawImageName] = true
+	u.updatedRawImages[rawImageName] = true
 
 	logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-	updateErr := restartSystemdUnit(ctx, ctr, unit, conn)
+	updateErr := u.restartSystemdUnit(ctx, ctr, unit)
 	if updateErr == nil {
 		report.Updated = "true"
 		return report, nil
 	}
 
-	if !options.Rollback {
+	if !u.options.Rollback {
 		return report, updateErr
 	}
 
@@ -247,7 +261,7 @@ func autoUpdateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.
 	if err := image.Tag(rawImageName); err != nil {
 		return report, fmt.Errorf("falling back to previous image: %w", err)
 	}
-	if err := restartSystemdUnit(ctx, ctr, unit, conn); err != nil {
+	if err := u.restartSystemdUnit(ctx, ctr, unit); err != nil {
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
 
@@ -255,8 +269,8 @@ func autoUpdateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.
 	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 *entities.AutoUpdateOptions, conn *dbus.Conn, runtime *libpod.Runtime) (*entities.AutoUpdateReport, error) {
+// updateRegistry updates the image/container according to the "local" policy.
+func (u *updater) updateLocally(ctx context.Context, image *libimage.Image, ctr *libpod.Container) (*entities.AutoUpdateReport, error) {
 	cid := ctr.ID()
 	rawImageName := ctr.RawImageName()
 	if rawImageName == "" {
@@ -278,7 +292,7 @@ func autoUpdateLocally(ctx context.Context, image *libimage.Image, ctr *libpod.C
 		Updated:       "failed",
 	}
 
-	needsUpdate, err := newerLocalImageAvailable(runtime, image, rawImageName)
+	needsUpdate, err := newerLocalImageAvailable(u.runtime, image, rawImageName)
 	if err != nil {
 		return report, fmt.Errorf("locally auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
 	}
@@ -288,19 +302,19 @@ func autoUpdateLocally(ctx context.Context, image *libimage.Image, ctr *libpod.C
 		return report, nil
 	}
 
-	if options.DryRun {
+	if u.options.DryRun {
 		report.Updated = "pending"
 		return report, nil
 	}
 
 	logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName)
-	updateErr := restartSystemdUnit(ctx, ctr, unit, conn)
+	updateErr := u.restartSystemdUnit(ctx, ctr, unit)
 	if updateErr == nil {
 		report.Updated = "true"
 		return report, nil
 	}
 
-	if !options.Rollback {
+	if !u.options.Rollback {
 		return report, updateErr
 	}
 
@@ -308,7 +322,7 @@ func autoUpdateLocally(ctx context.Context, image *libimage.Image, ctr *libpod.C
 	if err := image.Tag(rawImageName); err != nil {
 		return report, fmt.Errorf("falling back to previous image: %w", err)
 	}
-	if err := restartSystemdUnit(ctx, ctr, unit, conn); err != nil {
+	if err := u.restartSystemdUnit(ctx, ctr, unit); err != nil {
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
 
@@ -317,9 +331,9 @@ func autoUpdateLocally(ctx context.Context, image *libimage.Image, ctr *libpod.C
 }
 
 // restartSystemdUnit restarts the systemd unit the container is running in.
-func restartSystemdUnit(ctx context.Context, ctr *libpod.Container, unit string, conn *dbus.Conn) error {
+func (u *updater) restartSystemdUnit(ctx context.Context, ctr *libpod.Container, unit string) error {
 	restartChan := make(chan string)
-	if _, err := conn.RestartUnitContext(ctx, unit, "replace", restartChan); err != nil {
+	if _, err := u.conn.RestartUnitContext(ctx, unit, "replace", restartChan); err != nil {
 		return fmt.Errorf("auto-updating container %q: restarting systemd unit %q failed: %w", ctr.ID(), unit, err)
 	}
 
-- 
cgit v1.2.3-54-g00ecf


From 328c8ba7b4567f69602cf6bc71f7a2f0efb5c969 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Mon, 1 Aug 2022 14:22:37 +0200
Subject: pkg/autoupdate: move policy map into updater

[NO NEW TESTS NEEDED] - should not change behavior.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 57 +++++++++++++++++++++++++-------------------
 1 file changed, 32 insertions(+), 25 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index c792ee4e9..7bda0ac27 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -53,6 +53,15 @@ var supportedPolicies = map[string]Policy{
 // policyMapper is used for tying a container to it's autoupdate policy
 type policyMapper map[Policy][]*libpod.Container
 
+// updater includes shared state for auto-updating one or more containers.
+type updater struct {
+	conn                *dbus.Conn
+	imageToPolicyMapper map[string]policyMapper
+	options             *entities.AutoUpdateOptions
+	updatedRawImages    map[string]bool
+	runtime             *libpod.Runtime
+}
+
 // LookupPolicy looks up the corresponding Policy for the specified
 // string. If none is found, an errors is returned including the list of
 // supported policies.
@@ -116,12 +125,22 @@ 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(ctx context.Context, runtime *libpod.Runtime, options entities.AutoUpdateOptions) ([]*entities.AutoUpdateReport, []error) {
+	auto := updater{
+		options:          &options,
+		runtime:          runtime,
+		updatedRawImages: make(map[string]bool),
+	}
+
 	// Create a map from `image ID -> []*Container`.
-	containerMap, errs := imageContainersMap(runtime)
-	if len(containerMap) == 0 {
+	if errs := auto.imageContainersMap(); len(errs) > 0 {
 		return nil, errs
 	}
 
+	// Nothing to do.
+	if len(auto.imageToPolicyMapper) == 0 {
+		return nil, nil
+	}
+
 	// Create a map from `image ID -> *libimage.Image` for image lookups.
 	listOptions := &libimage.ListImagesOptions{
 		Filters: []string{"readonly=false"},
@@ -142,19 +161,14 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 		return nil, []error{err}
 	}
 	defer conn.Close()
+	auto.conn = conn
 
 	runtime.NewSystemEvent(events.AutoUpdate)
 
-	auto := updater{
-		conn:             conn,
-		options:          &options,
-		runtime:          runtime,
-		updatedRawImages: make(map[string]bool),
-	}
-
 	// Update all images/container according to their auto-update policy.
 	var allReports []*entities.AutoUpdateReport
-	for imageID, policyMapper := range containerMap {
+	var errs []error
+	for imageID, policyMapper := range auto.imageToPolicyMapper {
 		image, exists := imageMap[imageID]
 		if !exists {
 			errs = append(errs, fmt.Errorf("container image ID %q not found in local storage", imageID))
@@ -185,14 +199,6 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 	return allReports, errs
 }
 
-// updater includes shared state for auto-updating one or more containers.
-type updater struct {
-	conn             *dbus.Conn
-	options          *entities.AutoUpdateOptions
-	updatedRawImages map[string]bool
-	runtime          *libpod.Runtime
-}
-
 // updateRegistry updates the image/container according to the "registry" policy.
 func (u *updater) updateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.Container) (*entities.AutoUpdateReport, error) {
 	cid := ctr.ID()
@@ -353,14 +359,15 @@ func (u *updater) restartSystemdUnit(ctx context.Context, ctr *libpod.Container,
 
 // imageContainersMap generates a map[image ID] -> [containers using the image]
 // of all containers with a valid auto-update policy.
-func imageContainersMap(runtime *libpod.Runtime) (map[string]policyMapper, []error) {
-	allContainers, err := runtime.GetAllContainers()
+func (u *updater) imageContainersMap() []error {
+	allContainers, err := u.runtime.GetAllContainers()
 	if err != nil {
-		return nil, []error{err}
+		return []error{err}
 	}
 
+	u.imageToPolicyMapper = make(map[string]policyMapper)
+
 	errors := []error{}
-	containerMap := make(map[string]policyMapper)
 	for _, ctr := range allContainers {
 		state, err := ctr.State()
 		if err != nil {
@@ -390,17 +397,17 @@ func imageContainersMap(runtime *libpod.Runtime) (map[string]policyMapper, []err
 			continue
 		} else {
 			id, _ := ctr.Image()
-			policyMap, exists := containerMap[id]
+			policyMap, exists := u.imageToPolicyMapper[id]
 			if !exists {
 				policyMap = make(map[Policy][]*libpod.Container)
 			}
 			policyMap[policy] = append(policyMap[policy], ctr)
-			containerMap[id] = policyMap
+			u.imageToPolicyMapper[id] = policyMap
 			// Now we know that `ctr` is configured for auto updates.
 		}
 	}
 
-	return containerMap, errors
+	return errors
 }
 
 // getAuthfilePath returns an authfile path, if set. The authfile label in the
-- 
cgit v1.2.3-54-g00ecf


From 033cc059fa712d930d631471b91f713a08ed4e45 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Mon, 1 Aug 2022 14:23:20 +0200
Subject: pkg/autoupdate: remove redundant branch

[NO NEW TESTS NEEDED] - should not change behavior.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 7bda0ac27..7586468a0 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -395,16 +395,16 @@ func (u *updater) imageContainersMap() []error {
 		// Skip labels not related to autoupdate
 		if policy == PolicyDefault {
 			continue
-		} else {
-			id, _ := ctr.Image()
-			policyMap, exists := u.imageToPolicyMapper[id]
-			if !exists {
-				policyMap = make(map[Policy][]*libpod.Container)
-			}
-			policyMap[policy] = append(policyMap[policy], ctr)
-			u.imageToPolicyMapper[id] = policyMap
-			// Now we know that `ctr` is configured for auto updates.
 		}
+
+		id, _ := ctr.Image()
+		policyMap, exists := u.imageToPolicyMapper[id]
+		if !exists {
+			policyMap = make(map[Policy][]*libpod.Container)
+		}
+		policyMap[policy] = append(policyMap[policy], ctr)
+		u.imageToPolicyMapper[id] = policyMap
+		// Now we know that `ctr` is configured for auto updates.
 	}
 
 	return errors
-- 
cgit v1.2.3-54-g00ecf


From 87c0c760ece41ae56ec3d99f3938a84b11366521 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 2 Aug 2022 14:28:18 +0200
Subject: pkg/autoupdate: introduce the notion of a `task`

A `task` includes data and state for updating a given container image.
It will come in handy in future changes, but we are going there in
baby steps to have smaller incremental changes.

[NO NEW TESTS NEEDED] - should not change behaviour.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 139 ++++++++++++++++++++++++++-----------------
 1 file changed, 84 insertions(+), 55 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 7586468a0..58e919493 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -50,18 +50,27 @@ var supportedPolicies = map[string]Policy{
 	"local":    PolicyLocalImage,
 }
 
-// policyMapper is used for tying a container to it's autoupdate policy
-type policyMapper map[Policy][]*libpod.Container
+// policyMappers assembles update tasks by policy
+type policyMapper map[Policy][]*task
 
 // updater includes shared state for auto-updating one or more containers.
 type updater struct {
 	conn                *dbus.Conn
+	idToImage           map[string]*libimage.Image
 	imageToPolicyMapper map[string]policyMapper
 	options             *entities.AutoUpdateOptions
 	updatedRawImages    map[string]bool
 	runtime             *libpod.Runtime
 }
 
+// task includes data and state for updating a container
+type task struct {
+	container *libpod.Container // Container to update
+	policy    Policy            // Update policy
+	image     *libimage.Image   // Original image before the update
+	unit      string            // Name of the systemd unit
+}
+
 // LookupPolicy looks up the corresponding Policy for the specified
 // string. If none is found, an errors is returned including the list of
 // supported policies.
@@ -110,6 +119,24 @@ func ValidateImageReference(imageName string) error {
 	return nil
 }
 
+func (u *updater) assembleImageMap(ctx context.Context) error {
+	// Create a map from `image ID -> *libimage.Image` for image lookups.
+	listOptions := &libimage.ListImagesOptions{
+		Filters: []string{"readonly=false"},
+	}
+	imagesSlice, err := u.runtime.LibimageRuntime().ListImages(ctx, nil, listOptions)
+	if err != nil {
+		return err
+	}
+	imageMap := make(map[string]*libimage.Image)
+	for i := range imagesSlice {
+		imageMap[imagesSlice[i].ID()] = imagesSlice[i]
+	}
+
+	u.idToImage = imageMap
+	return nil
+}
+
 // AutoUpdate looks up containers with a specified auto-update policy and acts
 // accordingly.
 //
@@ -131,6 +158,12 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 		updatedRawImages: make(map[string]bool),
 	}
 
+	// Assemble a map `image ID -> *libimage.Image` that we can consult
+	// later on for lookups.
+	if err := auto.assembleImageMap(ctx); err != nil {
+		return nil, []error{err}
+	}
+
 	// Create a map from `image ID -> []*Container`.
 	if errs := auto.imageContainersMap(); len(errs) > 0 {
 		return nil, errs
@@ -141,19 +174,6 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 		return nil, nil
 	}
 
-	// Create a map from `image ID -> *libimage.Image` for image lookups.
-	listOptions := &libimage.ListImagesOptions{
-		Filters: []string{"readonly=false"},
-	}
-	imagesSlice, err := runtime.LibimageRuntime().ListImages(ctx, nil, listOptions)
-	if err != nil {
-		return nil, []error{err}
-	}
-	imageMap := make(map[string]*libimage.Image)
-	for i := range imagesSlice {
-		imageMap[imagesSlice[i].ID()] = imagesSlice[i]
-	}
-
 	// Connect to DBUS.
 	conn, err := systemd.ConnectToDBUS()
 	if err != nil {
@@ -169,14 +189,13 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 	var allReports []*entities.AutoUpdateReport
 	var errs []error
 	for imageID, policyMapper := range auto.imageToPolicyMapper {
-		image, exists := imageMap[imageID]
-		if !exists {
+		if _, exists := auto.idToImage[imageID]; !exists {
 			errs = append(errs, fmt.Errorf("container image ID %q not found in local storage", imageID))
 			return nil, errs
 		}
 
-		for _, ctr := range policyMapper[PolicyRegistryImage] {
-			report, err := auto.updateRegistry(ctx, image, ctr)
+		for _, task := range policyMapper[PolicyRegistryImage] {
+			report, err := auto.updateRegistry(ctx, task)
 			if err != nil {
 				errs = append(errs, err)
 			}
@@ -185,8 +204,8 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 			}
 		}
 
-		for _, ctr := range policyMapper[PolicyLocalImage] {
-			report, err := auto.updateLocally(ctx, image, ctr)
+		for _, task := range policyMapper[PolicyLocalImage] {
+			report, err := auto.updateLocally(ctx, task)
 			if err != nil {
 				errs = append(errs, err)
 			}
@@ -200,39 +219,37 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 }
 
 // updateRegistry updates the image/container according to the "registry" policy.
-func (u *updater) updateRegistry(ctx context.Context, image *libimage.Image, ctr *libpod.Container) (*entities.AutoUpdateReport, error) {
-	cid := ctr.ID()
-	rawImageName := ctr.RawImageName()
+func (u *updater) updateRegistry(ctx context.Context, task *task) (*entities.AutoUpdateReport, error) {
+	cid := task.container.ID()
+	rawImageName := task.container.RawImageName()
 	if rawImageName == "" {
 		return nil, fmt.Errorf("registry auto-updating container %q: raw-image name is empty", cid)
 	}
 
-	labels := ctr.Labels()
-	unit, exists := labels[systemdDefine.EnvVariable]
-	if !exists {
-		return nil, fmt.Errorf("auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable)
+	if task.unit == "" {
+		return nil, fmt.Errorf("auto-updating container %q: no %s label found", task.container.ID(), systemdDefine.EnvVariable)
 	}
 
 	report := &entities.AutoUpdateReport{
 		ContainerID:   cid,
-		ContainerName: ctr.Name(),
+		ContainerName: task.container.Name(),
 		ImageName:     rawImageName,
 		Policy:        PolicyRegistryImage,
-		SystemdUnit:   unit,
+		SystemdUnit:   task.unit,
 		Updated:       "failed",
 	}
 
 	if _, updated := u.updatedRawImages[rawImageName]; updated {
 		logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-		if err := u.restartSystemdUnit(ctx, ctr, unit); err != nil {
+		if err := u.restartSystemdUnit(ctx, task.container, task.unit); err != nil {
 			return report, err
 		}
 		report.Updated = "true"
 		return report, nil
 	}
 
-	authfile := getAuthfilePath(ctr, u.options)
-	needsUpdate, err := newerRemoteImageAvailable(ctx, image, rawImageName, authfile)
+	authfile := getAuthfilePath(task.container, u.options)
+	needsUpdate, err := newerRemoteImageAvailable(ctx, task.image, rawImageName, authfile)
 	if err != nil {
 		return report, fmt.Errorf("registry auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
 	}
@@ -253,7 +270,7 @@ func (u *updater) updateRegistry(ctx context.Context, image *libimage.Image, ctr
 	u.updatedRawImages[rawImageName] = true
 
 	logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-	updateErr := u.restartSystemdUnit(ctx, ctr, unit)
+	updateErr := u.restartSystemdUnit(ctx, task.container, task.unit)
 	if updateErr == nil {
 		report.Updated = "true"
 		return report, nil
@@ -264,10 +281,10 @@ func (u *updater) updateRegistry(ctx context.Context, image *libimage.Image, ctr
 	}
 
 	// To fallback, simply retag the old image and restart the service.
-	if err := image.Tag(rawImageName); err != nil {
+	if err := task.image.Tag(rawImageName); err != nil {
 		return report, fmt.Errorf("falling back to previous image: %w", err)
 	}
-	if err := u.restartSystemdUnit(ctx, ctr, unit); err != nil {
+	if err := u.restartSystemdUnit(ctx, task.container, task.unit); err != nil {
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
 
@@ -276,29 +293,27 @@ func (u *updater) updateRegistry(ctx context.Context, image *libimage.Image, ctr
 }
 
 // updateRegistry updates the image/container according to the "local" policy.
-func (u *updater) updateLocally(ctx context.Context, image *libimage.Image, ctr *libpod.Container) (*entities.AutoUpdateReport, error) {
-	cid := ctr.ID()
-	rawImageName := ctr.RawImageName()
+func (u *updater) updateLocally(ctx context.Context, task *task) (*entities.AutoUpdateReport, error) {
+	cid := task.container.ID()
+	rawImageName := task.container.RawImageName()
 	if rawImageName == "" {
 		return nil, fmt.Errorf("locally auto-updating container %q: raw-image name is empty", cid)
 	}
 
-	labels := ctr.Labels()
-	unit, exists := labels[systemdDefine.EnvVariable]
-	if !exists {
-		return nil, fmt.Errorf("auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable)
+	if task.unit == "" {
+		return nil, fmt.Errorf("auto-updating container %q: no %s label found", task.container.ID(), systemdDefine.EnvVariable)
 	}
 
 	report := &entities.AutoUpdateReport{
 		ContainerID:   cid,
-		ContainerName: ctr.Name(),
+		ContainerName: task.container.Name(),
 		ImageName:     rawImageName,
 		Policy:        PolicyLocalImage,
-		SystemdUnit:   unit,
+		SystemdUnit:   task.unit,
 		Updated:       "failed",
 	}
 
-	needsUpdate, err := newerLocalImageAvailable(u.runtime, image, rawImageName)
+	needsUpdate, err := newerLocalImageAvailable(u.runtime, task.image, rawImageName)
 	if err != nil {
 		return report, fmt.Errorf("locally auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
 	}
@@ -314,7 +329,7 @@ func (u *updater) updateLocally(ctx context.Context, image *libimage.Image, ctr
 	}
 
 	logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName)
-	updateErr := u.restartSystemdUnit(ctx, ctr, unit)
+	updateErr := u.restartSystemdUnit(ctx, task.container, task.unit)
 	if updateErr == nil {
 		report.Updated = "true"
 		return report, nil
@@ -325,10 +340,10 @@ func (u *updater) updateLocally(ctx context.Context, image *libimage.Image, ctr
 	}
 
 	// To fallback, simply retag the old image and restart the service.
-	if err := image.Tag(rawImageName); err != nil {
+	if err := task.image.Tag(rawImageName); err != nil {
 		return report, fmt.Errorf("falling back to previous image: %w", err)
 	}
-	if err := u.restartSystemdUnit(ctx, ctr, unit); err != nil {
+	if err := u.restartSystemdUnit(ctx, task.container, task.unit); err != nil {
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
 
@@ -368,7 +383,8 @@ func (u *updater) imageContainersMap() []error {
 	u.imageToPolicyMapper = make(map[string]policyMapper)
 
 	errors := []error{}
-	for _, ctr := range allContainers {
+	for _, c := range allContainers {
+		ctr := c
 		state, err := ctr.State()
 		if err != nil {
 			errors = append(errors, err)
@@ -379,13 +395,11 @@ func (u *updater) imageContainersMap() []error {
 			continue
 		}
 
-		// Only update containers with the specific label/policy set.
 		labels := ctr.Labels()
 		value, exists := labels[Label]
 		if !exists {
 			continue
 		}
-
 		policy, err := LookupPolicy(value)
 		if err != nil {
 			errors = append(errors, err)
@@ -400,11 +414,26 @@ func (u *updater) imageContainersMap() []error {
 		id, _ := ctr.Image()
 		policyMap, exists := u.imageToPolicyMapper[id]
 		if !exists {
-			policyMap = make(map[Policy][]*libpod.Container)
+			policyMap = make(map[Policy][]*task)
+		}
+
+		image, exists := u.idToImage[id]
+		if !exists {
+			err := fmt.Errorf("internal error: no image found for ID %s", id)
+			errors = append(errors, err)
+			continue
+		}
+
+		unit, _ := labels[systemdDefine.EnvVariable]
+
+		t := task{
+			container: ctr,
+			policy:    policy,
+			image:     image,
+			unit:      unit,
 		}
-		policyMap[policy] = append(policyMap[policy], ctr)
+		policyMap[policy] = append(policyMap[policy], &t)
 		u.imageToPolicyMapper[id] = policyMap
-		// Now we know that `ctr` is configured for auto updates.
 	}
 
 	return errors
-- 
cgit v1.2.3-54-g00ecf


From f8b6a81ae4762e6ed67af254611b0a32a7d69ea9 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 2 Aug 2022 15:37:34 +0200
Subject: test/system/255-auto-update.bats: add an SELinux comment

Drop a comment on using `chcon` to let the local rollback test pass.
It took me a while to understand why the test failed and future souls
may appreciated the extra breadcrumb.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 test/system/255-auto-update.bats | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/test/system/255-auto-update.bats b/test/system/255-auto-update.bats
index 6cee939fb..c6f9600b6 100644
--- a/test/system/255-auto-update.bats
+++ b/test/system/255-auto-update.bats
@@ -234,6 +234,8 @@ function _confirm_update() {
     _confirm_update $cname $ori_image
 }
 
+# This test can fail in dev. environment because of SELinux.
+# quick fix: chcon -t container_runtime_exec_t ./bin/podman
 @test "podman auto-update - label io.containers.autoupdate=local with rollback" {
     # sdnotify fails with runc 1.0.0-3-dev2 on Ubuntu. Let's just
     # assume that we work only with crun, nothing else.
-- 
cgit v1.2.3-54-g00ecf


From 2c999f1ecb87a6688c19502f0f594e002104e951 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 2 Aug 2022 16:18:57 +0200
Subject: pkg/autoupdate: update unit-by-unit

Change the auto-update logic to update unit-by-unit rather by policy.
This allows for, in theory now and in practice later, to have mutliple
containers run in a single systemd unit and update them in sequence
before restarting the unit.

[NO NEW TESTS NEEDED] - should not change behavior.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 106 ++++++++++++++++++++++---------------------
 1 file changed, 54 insertions(+), 52 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 58e919493..0f9df9327 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -50,17 +50,14 @@ var supportedPolicies = map[string]Policy{
 	"local":    PolicyLocalImage,
 }
 
-// policyMappers assembles update tasks by policy
-type policyMapper map[Policy][]*task
-
 // updater includes shared state for auto-updating one or more containers.
 type updater struct {
-	conn                *dbus.Conn
-	idToImage           map[string]*libimage.Image
-	imageToPolicyMapper map[string]policyMapper
-	options             *entities.AutoUpdateOptions
-	updatedRawImages    map[string]bool
-	runtime             *libpod.Runtime
+	conn             *dbus.Conn
+	idToImage        map[string]*libimage.Image
+	unitToTasks      map[string][]*task
+	options          *entities.AutoUpdateOptions
+	updatedRawImages map[string]bool
+	runtime          *libpod.Runtime
 }
 
 // task includes data and state for updating a container
@@ -152,26 +149,22 @@ func (u *updater) assembleImageMap(ctx context.Context) error {
 // It returns a slice of successfully restarted systemd units and a slice of
 // errors encountered during auto update.
 func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.AutoUpdateOptions) ([]*entities.AutoUpdateReport, []error) {
+	// Note that (most) errors are non-fatal such that a single
+	// misconfigured container does not prevent others from being updated
+	// (which could be a security threat).
+
 	auto := updater{
 		options:          &options,
 		runtime:          runtime,
 		updatedRawImages: make(map[string]bool),
 	}
 
-	// Assemble a map `image ID -> *libimage.Image` that we can consult
-	// later on for lookups.
-	if err := auto.assembleImageMap(ctx); err != nil {
-		return nil, []error{err}
-	}
-
-	// Create a map from `image ID -> []*Container`.
-	if errs := auto.imageContainersMap(); len(errs) > 0 {
-		return nil, errs
-	}
+	// Find auto-update tasks and assemble them by unit.
+	errors := auto.assembleTasks(ctx)
 
 	// Nothing to do.
-	if len(auto.imageToPolicyMapper) == 0 {
-		return nil, nil
+	if len(auto.unitToTasks) == 0 {
+		return nil, errors
 	}
 
 	// Connect to DBUS.
@@ -187,27 +180,28 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 
 	// Update all images/container according to their auto-update policy.
 	var allReports []*entities.AutoUpdateReport
-	var errs []error
-	for imageID, policyMapper := range auto.imageToPolicyMapper {
-		if _, exists := auto.idToImage[imageID]; !exists {
-			errs = append(errs, fmt.Errorf("container image ID %q not found in local storage", imageID))
-			return nil, errs
+	for unit, tasks := range auto.unitToTasks {
+		// Sanity check: we'll support that in the future.
+		if len(tasks) != 1 {
+			errors = append(errors, fmt.Errorf("only 1 task per unit supported but unit %s has %d", unit, len(tasks)))
+			return nil, errors
 		}
 
-		for _, task := range policyMapper[PolicyRegistryImage] {
-			report, err := auto.updateRegistry(ctx, task)
-			if err != nil {
-				errs = append(errs, err)
-			}
-			if report != nil {
-				allReports = append(allReports, report)
+		for _, task := range tasks {
+			var report *entities.AutoUpdateReport
+			var reportError error
+
+			switch task.policy {
+			case PolicyRegistryImage:
+				report, reportError = auto.updateRegistry(ctx, task)
+			case PolicyLocalImage:
+				report, reportError = auto.updateLocally(ctx, task)
+			default:
+				reportError = fmt.Errorf("unexpected auto-update policy %s for container %s", task.policy, task.container.ID())
 			}
-		}
 
-		for _, task := range policyMapper[PolicyLocalImage] {
-			report, err := auto.updateLocally(ctx, task)
-			if err != nil {
-				errs = append(errs, err)
+			if reportError != nil {
+				errors = append(errors, reportError)
 			}
 			if report != nil {
 				allReports = append(allReports, report)
@@ -215,7 +209,7 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 		}
 	}
 
-	return allReports, errs
+	return allReports, errors
 }
 
 // updateRegistry updates the image/container according to the "registry" policy.
@@ -372,15 +366,21 @@ func (u *updater) restartSystemdUnit(ctx context.Context, ctr *libpod.Container,
 	}
 }
 
-// imageContainersMap generates a map[image ID] -> [containers using the image]
-// of all containers with a valid auto-update policy.
-func (u *updater) imageContainersMap() []error {
+// assembleTasks assembles update tasks per unit and populates a mapping from
+// `unit -> []*task` such that multiple containers _can_ run in a single unit.
+func (u *updater) assembleTasks(ctx context.Context) []error {
+	// Assemble a map `image ID -> *libimage.Image` that we can consult
+	// later on for lookups.
+	if err := u.assembleImageMap(ctx); err != nil {
+		return []error{err}
+	}
+
 	allContainers, err := u.runtime.GetAllContainers()
 	if err != nil {
 		return []error{err}
 	}
 
-	u.imageToPolicyMapper = make(map[string]policyMapper)
+	u.unitToTasks = make(map[string][]*task)
 
 	errors := []error{}
 	for _, c := range allContainers {
@@ -395,6 +395,8 @@ func (u *updater) imageContainersMap() []error {
 			continue
 		}
 
+		// Check the container's auto-update policy which is configured
+		// as a label.
 		labels := ctr.Labels()
 		value, exists := labels[Label]
 		if !exists {
@@ -405,18 +407,19 @@ func (u *updater) imageContainersMap() []error {
 			errors = append(errors, err)
 			continue
 		}
-
-		// Skip labels not related to autoupdate
 		if policy == PolicyDefault {
 			continue
 		}
 
-		id, _ := ctr.Image()
-		policyMap, exists := u.imageToPolicyMapper[id]
+		// Make sure the container runs in a systemd unit which is
+		// stored as a label at container creation.
+		unit, exists := labels[systemdDefine.EnvVariable]
 		if !exists {
-			policyMap = make(map[Policy][]*task)
+			errors = append(errors, fmt.Errorf("auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable))
+			continue
 		}
 
+		id, _ := ctr.Image()
 		image, exists := u.idToImage[id]
 		if !exists {
 			err := fmt.Errorf("internal error: no image found for ID %s", id)
@@ -424,16 +427,15 @@ func (u *updater) imageContainersMap() []error {
 			continue
 		}
 
-		unit, _ := labels[systemdDefine.EnvVariable]
-
 		t := task{
 			container: ctr,
 			policy:    policy,
 			image:     image,
 			unit:      unit,
 		}
-		policyMap[policy] = append(policyMap[policy], &t)
-		u.imageToPolicyMapper[id] = policyMap
+
+		// Add the task to the unit.
+		u.unitToTasks[unit] = append(u.unitToTasks[unit], &t)
 	}
 
 	return errors
-- 
cgit v1.2.3-54-g00ecf


From 13a8ebd09f7d76d43f1b8af86c7ca2bbc1acfbca Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Tue, 2 Aug 2022 17:32:38 +0200
Subject: pkg/autoupdate: repull image if other containers failed

If two containers use the same image and one rolled back (i.e., tagged
the old image again), make sure to repull the image for the other
container.

Once an image has caused a rollback, it may be worth marking this image
as broken and not update any other container using it but that is
outside of the scope.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 0f9df9327..dfdbcc956 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -54,8 +54,8 @@ var supportedPolicies = map[string]Policy{
 type updater struct {
 	conn             *dbus.Conn
 	idToImage        map[string]*libimage.Image
-	unitToTasks      map[string][]*task
 	options          *entities.AutoUpdateOptions
+	unitToTasks      map[string][]*task
 	updatedRawImages map[string]bool
 	runtime          *libpod.Runtime
 }
@@ -278,6 +278,8 @@ func (u *updater) updateRegistry(ctx context.Context, task *task) (*entities.Aut
 	if err := task.image.Tag(rawImageName); err != nil {
 		return report, fmt.Errorf("falling back to previous image: %w", err)
 	}
+	u.updatedRawImages[rawImageName] = false
+
 	if err := u.restartSystemdUnit(ctx, task.container, task.unit); err != nil {
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
-- 
cgit v1.2.3-54-g00ecf


From cfa089c361c697932f0cd5506a1a43077253d6c8 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 13:00:27 +0200
Subject: pkg/autoupdate: s/updateImage/pullImage/

"pull" is more expressive.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index dfdbcc956..d01695912 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -258,7 +258,7 @@ func (u *updater) updateRegistry(ctx context.Context, task *task) (*entities.Aut
 		return report, nil
 	}
 
-	if _, err := updateImage(ctx, u.runtime, rawImageName, authfile); err != nil {
+	if _, err := pullImage(ctx, u.runtime, rawImageName, authfile); err != nil {
 		return report, fmt.Errorf("registry auto-updating container %q: image update for %q failed: %w", cid, rawImageName, err)
 	}
 	u.updatedRawImages[rawImageName] = true
@@ -474,8 +474,8 @@ func newerLocalImageAvailable(runtime *libpod.Runtime, img *libimage.Image, rawI
 	return localImg.Digest().String() != img.Digest().String(), nil
 }
 
-// updateImage pulls the specified image.
-func updateImage(ctx context.Context, runtime *libpod.Runtime, name, authfile string) (*libimage.Image, error) {
+// pullImage pulls the specified image.
+func pullImage(ctx context.Context, runtime *libpod.Runtime, name, authfile string) (*libimage.Image, error) {
 	pullOptions := &libimage.PullOptions{}
 	pullOptions.AuthFilePath = authfile
 	pullOptions.Writer = os.Stderr
-- 
cgit v1.2.3-54-g00ecf


From 3f1928d76714e6cd11a4036b2b99b897937e0586 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 13:30:07 +0200
Subject: pkg/autoupdate: move more logic under `task`

This will simplify the logic and pave the way for abstracting the
auto-update policies to a certain degree that allows us to better
control _when_ the updates and rollbacks happen and will ultimately
reduce redundant code.

[NO NEW TESTS NEEDED] - should not change behavior.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 95 +++++++++++++++++++++++---------------------
 1 file changed, 49 insertions(+), 46 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index d01695912..e892ad301 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -62,6 +62,7 @@ type updater struct {
 
 // task includes data and state for updating a container
 type task struct {
+	auto      *updater          // Reverse pointer to the updater
 	container *libpod.Container // Container to update
 	policy    Policy            // Update policy
 	image     *libimage.Image   // Original image before the update
@@ -188,20 +189,9 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 		}
 
 		for _, task := range tasks {
-			var report *entities.AutoUpdateReport
-			var reportError error
-
-			switch task.policy {
-			case PolicyRegistryImage:
-				report, reportError = auto.updateRegistry(ctx, task)
-			case PolicyLocalImage:
-				report, reportError = auto.updateLocally(ctx, task)
-			default:
-				reportError = fmt.Errorf("unexpected auto-update policy %s for container %s", task.policy, task.container.ID())
-			}
-
-			if reportError != nil {
-				errors = append(errors, reportError)
+			report, err := task.update(ctx)
+			if err != nil {
+				errors = append(errors, err)
 			}
 			if report != nil {
 				allReports = append(allReports, report)
@@ -212,38 +202,50 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 	return allReports, errors
 }
 
+// update the task according to its auto-update policy.
+func (t *task) update(ctx context.Context) (*entities.AutoUpdateReport, error) {
+	switch t.policy {
+	case PolicyRegistryImage:
+		return t.updateRegistry(ctx)
+	case PolicyLocalImage:
+		return t.updateLocally(ctx)
+	default:
+		return nil, fmt.Errorf("unexpected auto-update policy %s for container %s", t.policy, t.container.ID())
+	}
+}
+
 // updateRegistry updates the image/container according to the "registry" policy.
-func (u *updater) updateRegistry(ctx context.Context, task *task) (*entities.AutoUpdateReport, error) {
-	cid := task.container.ID()
-	rawImageName := task.container.RawImageName()
+func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport, error) {
+	cid := t.container.ID()
+	rawImageName := t.container.RawImageName()
 	if rawImageName == "" {
 		return nil, fmt.Errorf("registry auto-updating container %q: raw-image name is empty", cid)
 	}
 
-	if task.unit == "" {
-		return nil, fmt.Errorf("auto-updating container %q: no %s label found", task.container.ID(), systemdDefine.EnvVariable)
+	if t.unit == "" {
+		return nil, fmt.Errorf("auto-updating container %q: no %s label found", t.container.ID(), systemdDefine.EnvVariable)
 	}
 
 	report := &entities.AutoUpdateReport{
 		ContainerID:   cid,
-		ContainerName: task.container.Name(),
+		ContainerName: t.container.Name(),
 		ImageName:     rawImageName,
 		Policy:        PolicyRegistryImage,
-		SystemdUnit:   task.unit,
+		SystemdUnit:   t.unit,
 		Updated:       "failed",
 	}
 
-	if _, updated := u.updatedRawImages[rawImageName]; updated {
+	if _, updated := t.auto.updatedRawImages[rawImageName]; updated {
 		logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-		if err := u.restartSystemdUnit(ctx, task.container, task.unit); err != nil {
+		if err := t.auto.restartSystemdUnit(ctx, t.container, t.unit); err != nil {
 			return report, err
 		}
 		report.Updated = "true"
 		return report, nil
 	}
 
-	authfile := getAuthfilePath(task.container, u.options)
-	needsUpdate, err := newerRemoteImageAvailable(ctx, task.image, rawImageName, authfile)
+	authfile := getAuthfilePath(t.container, t.auto.options)
+	needsUpdate, err := newerRemoteImageAvailable(ctx, t.image, rawImageName, authfile)
 	if err != nil {
 		return report, fmt.Errorf("registry auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
 	}
@@ -253,34 +255,34 @@ func (u *updater) updateRegistry(ctx context.Context, task *task) (*entities.Aut
 		return report, nil
 	}
 
-	if u.options.DryRun {
+	if t.auto.options.DryRun {
 		report.Updated = "pending"
 		return report, nil
 	}
 
-	if _, err := pullImage(ctx, u.runtime, rawImageName, authfile); err != nil {
+	if _, err := pullImage(ctx, t.auto.runtime, rawImageName, authfile); err != nil {
 		return report, fmt.Errorf("registry auto-updating container %q: image update for %q failed: %w", cid, rawImageName, err)
 	}
-	u.updatedRawImages[rawImageName] = true
+	t.auto.updatedRawImages[rawImageName] = true
 
 	logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-	updateErr := u.restartSystemdUnit(ctx, task.container, task.unit)
+	updateErr := t.auto.restartSystemdUnit(ctx, t.container, t.unit)
 	if updateErr == nil {
 		report.Updated = "true"
 		return report, nil
 	}
 
-	if !u.options.Rollback {
+	if !t.auto.options.Rollback {
 		return report, updateErr
 	}
 
 	// To fallback, simply retag the old image and restart the service.
-	if err := task.image.Tag(rawImageName); err != nil {
+	if err := t.image.Tag(rawImageName); err != nil {
 		return report, fmt.Errorf("falling back to previous image: %w", err)
 	}
-	u.updatedRawImages[rawImageName] = false
+	t.auto.updatedRawImages[rawImageName] = false
 
-	if err := u.restartSystemdUnit(ctx, task.container, task.unit); err != nil {
+	if err := t.auto.restartSystemdUnit(ctx, t.container, t.unit); err != nil {
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
 
@@ -289,27 +291,27 @@ func (u *updater) updateRegistry(ctx context.Context, task *task) (*entities.Aut
 }
 
 // updateRegistry updates the image/container according to the "local" policy.
-func (u *updater) updateLocally(ctx context.Context, task *task) (*entities.AutoUpdateReport, error) {
-	cid := task.container.ID()
-	rawImageName := task.container.RawImageName()
+func (t *task) updateLocally(ctx context.Context) (*entities.AutoUpdateReport, error) {
+	cid := t.container.ID()
+	rawImageName := t.container.RawImageName()
 	if rawImageName == "" {
 		return nil, fmt.Errorf("locally auto-updating container %q: raw-image name is empty", cid)
 	}
 
-	if task.unit == "" {
-		return nil, fmt.Errorf("auto-updating container %q: no %s label found", task.container.ID(), systemdDefine.EnvVariable)
+	if t.unit == "" {
+		return nil, fmt.Errorf("auto-updating container %q: no %s label found", t.container.ID(), systemdDefine.EnvVariable)
 	}
 
 	report := &entities.AutoUpdateReport{
 		ContainerID:   cid,
-		ContainerName: task.container.Name(),
+		ContainerName: t.container.Name(),
 		ImageName:     rawImageName,
 		Policy:        PolicyLocalImage,
-		SystemdUnit:   task.unit,
+		SystemdUnit:   t.unit,
 		Updated:       "failed",
 	}
 
-	needsUpdate, err := newerLocalImageAvailable(u.runtime, task.image, rawImageName)
+	needsUpdate, err := newerLocalImageAvailable(t.auto.runtime, t.image, rawImageName)
 	if err != nil {
 		return report, fmt.Errorf("locally auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
 	}
@@ -319,27 +321,27 @@ func (u *updater) updateLocally(ctx context.Context, task *task) (*entities.Auto
 		return report, nil
 	}
 
-	if u.options.DryRun {
+	if t.auto.options.DryRun {
 		report.Updated = "pending"
 		return report, nil
 	}
 
 	logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName)
-	updateErr := u.restartSystemdUnit(ctx, task.container, task.unit)
+	updateErr := t.auto.restartSystemdUnit(ctx, t.container, t.unit)
 	if updateErr == nil {
 		report.Updated = "true"
 		return report, nil
 	}
 
-	if !u.options.Rollback {
+	if !t.auto.options.Rollback {
 		return report, updateErr
 	}
 
 	// To fallback, simply retag the old image and restart the service.
-	if err := task.image.Tag(rawImageName); err != nil {
+	if err := t.image.Tag(rawImageName); err != nil {
 		return report, fmt.Errorf("falling back to previous image: %w", err)
 	}
-	if err := u.restartSystemdUnit(ctx, task.container, task.unit); err != nil {
+	if err := t.auto.restartSystemdUnit(ctx, t.container, t.unit); err != nil {
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
 
@@ -430,6 +432,7 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 		}
 
 		t := task{
+			auto:      u,
 			container: ctr,
 			policy:    policy,
 			image:     image,
-- 
cgit v1.2.3-54-g00ecf


From 42c4c17c01af746cd1424878a397cb6eeb9c65dd Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 13:37:06 +0200
Subject: pkg/autoupdate: move authfile into `tasks`

Will simplify the code and speed up things as we do not consult a
container's labels multiple times.

[NO NEW TESTS NEEDED] - should not change behavior.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 18 ++++--------------
 1 file changed, 4 insertions(+), 14 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index e892ad301..89d0a1ac3 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -62,6 +62,7 @@ type updater struct {
 
 // task includes data and state for updating a container
 type task struct {
+	authfile  string            // Container-specific authfile
 	auto      *updater          // Reverse pointer to the updater
 	container *libpod.Container // Container to update
 	policy    Policy            // Update policy
@@ -244,8 +245,7 @@ func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport,
 		return report, nil
 	}
 
-	authfile := getAuthfilePath(t.container, t.auto.options)
-	needsUpdate, err := newerRemoteImageAvailable(ctx, t.image, rawImageName, authfile)
+	needsUpdate, err := newerRemoteImageAvailable(ctx, t.image, rawImageName, t.authfile)
 	if err != nil {
 		return report, fmt.Errorf("registry auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
 	}
@@ -260,7 +260,7 @@ func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport,
 		return report, nil
 	}
 
-	if _, err := pullImage(ctx, t.auto.runtime, rawImageName, authfile); err != nil {
+	if _, err := pullImage(ctx, t.auto.runtime, rawImageName, t.authfile); err != nil {
 		return report, fmt.Errorf("registry auto-updating container %q: image update for %q failed: %w", cid, rawImageName, err)
 	}
 	t.auto.updatedRawImages[rawImageName] = true
@@ -432,6 +432,7 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 		}
 
 		t := task{
+			authfile:  labels[AuthfileLabel],
 			auto:      u,
 			container: ctr,
 			policy:    policy,
@@ -446,17 +447,6 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 	return errors
 }
 
-// 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 *entities.AutoUpdateOptions) string {
-	labels := ctr.Labels()
-	authFilePath, exists := labels[AuthfileLabel]
-	if exists {
-		return authFilePath
-	}
-	return options.Authfile
-}
-
 // newerRemoteImageAvailable returns true if there corresponding image on the remote
 // registry is newer.
 func newerRemoteImageAvailable(ctx context.Context, img *libimage.Image, origName string, authfile string) (bool, error) {
-- 
cgit v1.2.3-54-g00ecf


From 1cc933c6bb1dbbab56bc1918db4ae358c56bba8b Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 13:49:04 +0200
Subject: pkg/autoupdate: introduce status constants

To replace redundant string scattered across the code with proper
constants.  The "status" will further be useful in a future change
as it can be moved into a `task`.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 30 +++++++++++++++++++-----------
 1 file changed, 19 insertions(+), 11 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 89d0a1ac3..2db85df5d 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -60,6 +60,14 @@ type updater struct {
 	runtime          *libpod.Runtime
 }
 
+const (
+	statusFailed     = "failed"      // The update has failed
+	statusUpdated    = "true"        // The update succeeded
+	statusNotUpdated = "false"       // No update was needed
+	statusPending    = "pending"     // The update is pending (see options.DryRun)
+	statusRolledBack = "rolled back" // Rollback after a failed update
+)
+
 // task includes data and state for updating a container
 type task struct {
 	authfile  string            // Container-specific authfile
@@ -233,7 +241,7 @@ func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport,
 		ImageName:     rawImageName,
 		Policy:        PolicyRegistryImage,
 		SystemdUnit:   t.unit,
-		Updated:       "failed",
+		Updated:       statusFailed,
 	}
 
 	if _, updated := t.auto.updatedRawImages[rawImageName]; updated {
@@ -241,7 +249,7 @@ func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport,
 		if err := t.auto.restartSystemdUnit(ctx, t.container, t.unit); err != nil {
 			return report, err
 		}
-		report.Updated = "true"
+		report.Updated = statusUpdated
 		return report, nil
 	}
 
@@ -251,12 +259,12 @@ func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport,
 	}
 
 	if !needsUpdate {
-		report.Updated = "false"
+		report.Updated = statusNotUpdated
 		return report, nil
 	}
 
 	if t.auto.options.DryRun {
-		report.Updated = "pending"
+		report.Updated = statusPending
 		return report, nil
 	}
 
@@ -268,7 +276,7 @@ func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport,
 	logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
 	updateErr := t.auto.restartSystemdUnit(ctx, t.container, t.unit)
 	if updateErr == nil {
-		report.Updated = "true"
+		report.Updated = statusUpdated
 		return report, nil
 	}
 
@@ -286,7 +294,7 @@ func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport,
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
 
-	report.Updated = "rolled back"
+	report.Updated = statusRolledBack
 	return report, nil
 }
 
@@ -308,7 +316,7 @@ func (t *task) updateLocally(ctx context.Context) (*entities.AutoUpdateReport, e
 		ImageName:     rawImageName,
 		Policy:        PolicyLocalImage,
 		SystemdUnit:   t.unit,
-		Updated:       "failed",
+		Updated:       statusFailed,
 	}
 
 	needsUpdate, err := newerLocalImageAvailable(t.auto.runtime, t.image, rawImageName)
@@ -317,19 +325,19 @@ func (t *task) updateLocally(ctx context.Context) (*entities.AutoUpdateReport, e
 	}
 
 	if !needsUpdate {
-		report.Updated = "false"
+		report.Updated = statusNotUpdated
 		return report, nil
 	}
 
 	if t.auto.options.DryRun {
-		report.Updated = "pending"
+		report.Updated = statusPending
 		return report, nil
 	}
 
 	logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName)
 	updateErr := t.auto.restartSystemdUnit(ctx, t.container, t.unit)
 	if updateErr == nil {
-		report.Updated = "true"
+		report.Updated = statusUpdated
 		return report, nil
 	}
 
@@ -345,7 +353,7 @@ func (t *task) updateLocally(ctx context.Context) (*entities.AutoUpdateReport, e
 		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
 	}
 
-	report.Updated = "rolled back"
+	report.Updated = statusRolledBack
 	return report, nil
 }
 
-- 
cgit v1.2.3-54-g00ecf


From 82d18a86f395657424f24e86140bb1ed15229141 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 14:52:38 +0200
Subject: pkg/autoupdate: use policy consts were possible

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 2db85df5d..6748ca351 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -43,11 +43,11 @@ const (
 
 // Map for easy lookups of supported policies.
 var supportedPolicies = map[string]Policy{
-	"":         PolicyDefault,
-	"disabled": PolicyDefault,
-	"image":    PolicyRegistryImage,
-	"registry": PolicyRegistryImage,
-	"local":    PolicyLocalImage,
+	"":                          PolicyDefault,
+	string(PolicyDefault):       PolicyDefault,
+	"image":                     PolicyRegistryImage,
+	string(PolicyRegistryImage): PolicyRegistryImage,
+	string(PolicyLocalImage):    PolicyLocalImage,
 }
 
 // updater includes shared state for auto-updating one or more containers.
-- 
cgit v1.2.3-54-g00ecf


From 3fdd3b1ae332a26e6bba696d8bd49d29a0299b3b Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 14:56:01 +0200
Subject: pkg/autoupdate: remove image map from updater

It is not state needed after assembling the tasks, so remove it to keep
the task struct simpler.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 41 ++++++++++++++++++++---------------------
 1 file changed, 20 insertions(+), 21 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 6748ca351..794f31d1f 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -53,7 +53,6 @@ var supportedPolicies = map[string]Policy{
 // updater includes shared state for auto-updating one or more containers.
 type updater struct {
 	conn             *dbus.Conn
-	idToImage        map[string]*libimage.Image
 	options          *entities.AutoUpdateOptions
 	unitToTasks      map[string][]*task
 	updatedRawImages map[string]bool
@@ -126,24 +125,6 @@ func ValidateImageReference(imageName string) error {
 	return nil
 }
 
-func (u *updater) assembleImageMap(ctx context.Context) error {
-	// Create a map from `image ID -> *libimage.Image` for image lookups.
-	listOptions := &libimage.ListImagesOptions{
-		Filters: []string{"readonly=false"},
-	}
-	imagesSlice, err := u.runtime.LibimageRuntime().ListImages(ctx, nil, listOptions)
-	if err != nil {
-		return err
-	}
-	imageMap := make(map[string]*libimage.Image)
-	for i := range imagesSlice {
-		imageMap[imagesSlice[i].ID()] = imagesSlice[i]
-	}
-
-	u.idToImage = imageMap
-	return nil
-}
-
 // AutoUpdate looks up containers with a specified auto-update policy and acts
 // accordingly.
 //
@@ -383,7 +364,8 @@ func (u *updater) restartSystemdUnit(ctx context.Context, ctr *libpod.Container,
 func (u *updater) assembleTasks(ctx context.Context) []error {
 	// Assemble a map `image ID -> *libimage.Image` that we can consult
 	// later on for lookups.
-	if err := u.assembleImageMap(ctx); err != nil {
+	imageMap, err := u.assembleImageMap(ctx)
+	if err != nil {
 		return []error{err}
 	}
 
@@ -432,7 +414,7 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 		}
 
 		id, _ := ctr.Image()
-		image, exists := u.idToImage[id]
+		image, exists := imageMap[id]
 		if !exists {
 			err := fmt.Errorf("internal error: no image found for ID %s", id)
 			errors = append(errors, err)
@@ -455,6 +437,23 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 	return errors
 }
 
+// assembleImageMap creates a map from `image ID -> *libimage.Image` for image lookups.
+func (u *updater) assembleImageMap(ctx context.Context) (map[string]*libimage.Image, error) {
+	listOptions := &libimage.ListImagesOptions{
+		Filters: []string{"readonly=false"},
+	}
+	imagesSlice, err := u.runtime.LibimageRuntime().ListImages(ctx, nil, listOptions)
+	if err != nil {
+		return nil, err
+	}
+	imageMap := make(map[string]*libimage.Image)
+	for i := range imagesSlice {
+		imageMap[imagesSlice[i].ID()] = imagesSlice[i]
+	}
+
+	return imageMap, nil
+}
+
 // newerRemoteImageAvailable returns true if there corresponding image on the remote
 // registry is newer.
 func newerRemoteImageAvailable(ctx context.Context, img *libimage.Image, origName string, authfile string) (bool, error) {
-- 
cgit v1.2.3-54-g00ecf


From af3ce70844faaeece8d7a8dedab7065a13ceba2a Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 14:57:51 +0200
Subject: pkg/autoupdate: document fields of `updater`

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 794f31d1f..69a25cb3b 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -52,11 +52,11 @@ var supportedPolicies = map[string]Policy{
 
 // updater includes shared state for auto-updating one or more containers.
 type updater struct {
-	conn             *dbus.Conn
-	options          *entities.AutoUpdateOptions
-	unitToTasks      map[string][]*task
-	updatedRawImages map[string]bool
-	runtime          *libpod.Runtime
+	conn             *dbus.Conn                  // DBUS connection
+	options          *entities.AutoUpdateOptions // User-specified options
+	unitToTasks      map[string][]*task          // Keeps track of tasks per unit
+	updatedRawImages map[string]bool             // Keeps track of updated images
+	runtime          *libpod.Runtime             // The libpod runtime
 }
 
 const (
-- 
cgit v1.2.3-54-g00ecf


From 0df51bb6bccc4e3598db59ea1bf389a97d7019e4 Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 15:07:08 +0200
Subject: pkg/autoupdate: move status into `task`

As state should be kept in a single `task`.  This will allow for
separating updates from rollbacks which will be needed to support
multiple containers/tasks in a single unit.

[NO NEW TESTS NEEDED] - should not change behavior.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 302 ++++++++++++++++++++-----------------------
 1 file changed, 137 insertions(+), 165 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index 69a25cb3b..f450a4233 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -69,12 +69,14 @@ const (
 
 // task includes data and state for updating a container
 type task struct {
-	authfile  string            // Container-specific authfile
-	auto      *updater          // Reverse pointer to the updater
-	container *libpod.Container // Container to update
-	policy    Policy            // Update policy
-	image     *libimage.Image   // Original image before the update
-	unit      string            // Name of the systemd unit
+	authfile     string            // Container-specific authfile
+	auto         *updater          // Reverse pointer to the updater
+	container    *libpod.Container // Container to update
+	policy       Policy            // Update policy
+	image        *libimage.Image   // Original image before the update
+	rawImageName string            // The container's raw image name
+	status       string            // Auto-update status
+	unit         string            // Name of the systemd unit
 }
 
 // LookupPolicy looks up the corresponding Policy for the specified
@@ -179,170 +181,155 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 		}
 
 		for _, task := range tasks {
-			report, err := task.update(ctx)
+			err := func() error {
+				updateAvailable, err := task.updateAvailable(ctx)
+				if err != nil {
+					task.status = statusFailed
+					return fmt.Errorf("checking image updates for container %s: %w", task.container.ID(), err)
+				}
+
+				if !updateAvailable {
+					task.status = statusNotUpdated
+					return nil
+				}
+
+				if options.DryRun {
+					task.status = statusPending
+					return nil
+				}
+
+				if err := task.update(ctx); err != nil {
+					task.status = statusFailed
+					return fmt.Errorf("updating image for container %s: %w", task.container.ID(), err)
+				}
+
+				updateError := auto.restartSystemdUnit(ctx, unit)
+				if updateError == nil {
+					task.status = statusUpdated
+					return nil
+				}
+
+				if !options.Rollback {
+					task.status = statusFailed
+					return fmt.Errorf("restarting unit %s for container %s: %w", task.unit, task.container.ID(), err)
+				}
+
+				if err := task.rollbackImage(); err != nil {
+					task.status = statusFailed
+					return fmt.Errorf("rolling back image for container %s: %w", task.container.ID(), err)
+				}
+
+				if err := auto.restartSystemdUnit(ctx, unit); err != nil {
+					task.status = statusFailed
+					return fmt.Errorf("restarting unit %s for container %s during rollback: %w", task.unit, task.container.ID(), err)
+				}
+
+				task.status = statusRolledBack
+				return nil
+			}()
+
 			if err != nil {
 				errors = append(errors, err)
 			}
-			if report != nil {
-				allReports = append(allReports, report)
-			}
+			allReports = append(allReports, task.report())
 		}
 	}
 
 	return allReports, errors
 }
 
-// update the task according to its auto-update policy.
-func (t *task) update(ctx context.Context) (*entities.AutoUpdateReport, error) {
+// report creates an auto-update report for the task.
+func (t *task) report() *entities.AutoUpdateReport {
+	return &entities.AutoUpdateReport{
+		ContainerID:   t.container.ID(),
+		ContainerName: t.container.Name(),
+		ImageName:     t.container.RawImageName(),
+		Policy:        string(t.policy),
+		SystemdUnit:   t.unit,
+		Updated:       t.status,
+	}
+}
+
+// updateAvailable returns whether an update for the task is available.
+func (t *task) updateAvailable(ctx context.Context) (bool, error) {
 	switch t.policy {
 	case PolicyRegistryImage:
-		return t.updateRegistry(ctx)
+		return t.registryUpdateAvailable(ctx)
 	case PolicyLocalImage:
-		return t.updateLocally(ctx)
+		return t.localUpdateAvailable()
 	default:
-		return nil, fmt.Errorf("unexpected auto-update policy %s for container %s", t.policy, t.container.ID())
+		return false, fmt.Errorf("unexpected auto-update policy %s for container %s", t.policy, t.container.ID())
 	}
 }
 
-// updateRegistry updates the image/container according to the "registry" policy.
-func (t *task) updateRegistry(ctx context.Context) (*entities.AutoUpdateReport, error) {
-	cid := t.container.ID()
-	rawImageName := t.container.RawImageName()
-	if rawImageName == "" {
-		return nil, fmt.Errorf("registry auto-updating container %q: raw-image name is empty", cid)
-	}
-
-	if t.unit == "" {
-		return nil, fmt.Errorf("auto-updating container %q: no %s label found", t.container.ID(), systemdDefine.EnvVariable)
-	}
-
-	report := &entities.AutoUpdateReport{
-		ContainerID:   cid,
-		ContainerName: t.container.Name(),
-		ImageName:     rawImageName,
-		Policy:        PolicyRegistryImage,
-		SystemdUnit:   t.unit,
-		Updated:       statusFailed,
+// update the task according to its auto-update policy.
+func (t *task) update(ctx context.Context) error {
+	switch t.policy {
+	case PolicyRegistryImage:
+		return t.registryUpdate(ctx)
+	case PolicyLocalImage:
+		// Nothing to do as the image is already available in the local storage.
+		return nil
+	default:
+		return fmt.Errorf("unexpected auto-update policy %s for container %s", t.policy, t.container.ID())
 	}
+}
 
-	if _, updated := t.auto.updatedRawImages[rawImageName]; updated {
-		logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-		if err := t.auto.restartSystemdUnit(ctx, t.container, t.unit); err != nil {
-			return report, err
-		}
-		report.Updated = statusUpdated
-		return report, nil
+// registryUpdateAvailable returns whether a new image on the registry is available.
+func (t *task) registryUpdateAvailable(ctx context.Context) (bool, error) {
+	// The newer image has already been pulled for another task, so we know
+	// there's a newer one available.
+	if _, exists := t.auto.updatedRawImages[t.rawImageName]; exists {
+		return true, nil
 	}
 
-	needsUpdate, err := newerRemoteImageAvailable(ctx, t.image, rawImageName, t.authfile)
+	remoteRef, err := docker.ParseReference("//" + t.rawImageName)
 	if err != nil {
-		return report, fmt.Errorf("registry auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
-	}
-
-	if !needsUpdate {
-		report.Updated = statusNotUpdated
-		return report, nil
-	}
-
-	if t.auto.options.DryRun {
-		report.Updated = statusPending
-		return report, nil
-	}
-
-	if _, err := pullImage(ctx, t.auto.runtime, rawImageName, t.authfile); err != nil {
-		return report, fmt.Errorf("registry auto-updating container %q: image update for %q failed: %w", cid, rawImageName, err)
-	}
-	t.auto.updatedRawImages[rawImageName] = true
-
-	logrus.Infof("Auto-updating container %q using registry image %q", cid, rawImageName)
-	updateErr := t.auto.restartSystemdUnit(ctx, t.container, t.unit)
-	if updateErr == nil {
-		report.Updated = statusUpdated
-		return report, nil
-	}
-
-	if !t.auto.options.Rollback {
-		return report, updateErr
-	}
-
-	// To fallback, simply retag the old image and restart the service.
-	if err := t.image.Tag(rawImageName); err != nil {
-		return report, fmt.Errorf("falling back to previous image: %w", err)
-	}
-	t.auto.updatedRawImages[rawImageName] = false
-
-	if err := t.auto.restartSystemdUnit(ctx, t.container, t.unit); err != nil {
-		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
+		return false, err
 	}
-
-	report.Updated = statusRolledBack
-	return report, nil
+	options := &libimage.HasDifferentDigestOptions{AuthFilePath: t.authfile}
+	return t.image.HasDifferentDigest(ctx, remoteRef, options)
 }
 
-// updateRegistry updates the image/container according to the "local" policy.
-func (t *task) updateLocally(ctx context.Context) (*entities.AutoUpdateReport, error) {
-	cid := t.container.ID()
-	rawImageName := t.container.RawImageName()
-	if rawImageName == "" {
-		return nil, fmt.Errorf("locally auto-updating container %q: raw-image name is empty", cid)
+// registryUpdate pulls down the image from the registry.
+func (t *task) registryUpdate(ctx context.Context) error {
+	// The newer image has already been pulled for another task.
+	if _, exists := t.auto.updatedRawImages[t.rawImageName]; exists {
+		return nil
 	}
 
-	if t.unit == "" {
-		return nil, fmt.Errorf("auto-updating container %q: no %s label found", t.container.ID(), systemdDefine.EnvVariable)
+	if err := pullImage(ctx, t.auto.runtime, t.rawImageName, t.authfile); err != nil {
+		return fmt.Errorf("registry auto-updating container %q: image update for %q failed: %w", t.container.ID(), t.rawImageName, err)
 	}
 
-	report := &entities.AutoUpdateReport{
-		ContainerID:   cid,
-		ContainerName: t.container.Name(),
-		ImageName:     rawImageName,
-		Policy:        PolicyLocalImage,
-		SystemdUnit:   t.unit,
-		Updated:       statusFailed,
-	}
+	t.auto.updatedRawImages[t.rawImageName] = true
+	return nil
+}
 
-	needsUpdate, err := newerLocalImageAvailable(t.auto.runtime, t.image, rawImageName)
+// localUpdateAvailable returns whether a new image in the local storage is available.
+func (t *task) localUpdateAvailable() (bool, error) {
+	localImg, _, err := t.auto.runtime.LibimageRuntime().LookupImage(t.rawImageName, nil)
 	if err != nil {
-		return report, fmt.Errorf("locally auto-updating container %q: image check for %q failed: %w", cid, rawImageName, err)
-	}
-
-	if !needsUpdate {
-		report.Updated = statusNotUpdated
-		return report, nil
-	}
-
-	if t.auto.options.DryRun {
-		report.Updated = statusPending
-		return report, nil
-	}
-
-	logrus.Infof("Auto-updating container %q using local image %q", cid, rawImageName)
-	updateErr := t.auto.restartSystemdUnit(ctx, t.container, t.unit)
-	if updateErr == nil {
-		report.Updated = statusUpdated
-		return report, nil
-	}
-
-	if !t.auto.options.Rollback {
-		return report, updateErr
+		return false, err
 	}
+	return localImg.Digest().String() != t.image.Digest().String(), nil
+}
 
+// rollbackImage rolls back the task's image to the previous version before the update.
+func (t *task) rollbackImage() error {
 	// To fallback, simply retag the old image and restart the service.
-	if err := t.image.Tag(rawImageName); err != nil {
-		return report, fmt.Errorf("falling back to previous image: %w", err)
-	}
-	if err := t.auto.restartSystemdUnit(ctx, t.container, t.unit); err != nil {
-		return report, fmt.Errorf("restarting unit with old image during fallback: %w", err)
+	if err := t.image.Tag(t.rawImageName); err != nil {
+		return err
 	}
-
-	report.Updated = statusRolledBack
-	return report, nil
+	t.auto.updatedRawImages[t.rawImageName] = false
+	return nil
 }
 
 // restartSystemdUnit restarts the systemd unit the container is running in.
-func (u *updater) restartSystemdUnit(ctx context.Context, ctr *libpod.Container, unit string) error {
+func (u *updater) restartSystemdUnit(ctx context.Context, unit string) error {
 	restartChan := make(chan string)
 	if _, err := u.conn.RestartUnitContext(ctx, unit, "replace", restartChan); err != nil {
-		return fmt.Errorf("auto-updating container %q: restarting systemd unit %q failed: %w", ctr.ID(), unit, err)
+		return err
 	}
 
 	// Wait for the restart to finish and actually check if it was
@@ -351,11 +338,11 @@ func (u *updater) restartSystemdUnit(ctx context.Context, ctr *libpod.Container,
 
 	switch result {
 	case "done":
-		logrus.Infof("Successfully restarted systemd unit %q of container %q", unit, ctr.ID())
+		logrus.Infof("Successfully restarted systemd unit %q", unit)
 		return nil
 
 	default:
-		return fmt.Errorf("auto-updating container %q: restarting systemd unit %q failed: expected %q but received %q", ctr.ID(), unit, "done", result)
+		return fmt.Errorf("expected %q but received %q", "done", result)
 	}
 }
 
@@ -421,13 +408,21 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 			continue
 		}
 
+		rawImageName := ctr.RawImageName()
+		if rawImageName == "" {
+			errors = append(errors, fmt.Errorf("locally auto-updating container %q: raw-image name is empty", ctr.ID()))
+			continue
+		}
+
 		t := task{
-			authfile:  labels[AuthfileLabel],
-			auto:      u,
-			container: ctr,
-			policy:    policy,
-			image:     image,
-			unit:      unit,
+			authfile:     labels[AuthfileLabel],
+			auto:         u,
+			container:    ctr,
+			policy:       policy,
+			image:        image,
+			unit:         unit,
+			rawImageName: rawImageName,
+			status:       statusFailed, // must be updated later on
 		}
 
 		// Add the task to the unit.
@@ -454,35 +449,12 @@ func (u *updater) assembleImageMap(ctx context.Context) (map[string]*libimage.Im
 	return imageMap, nil
 }
 
-// newerRemoteImageAvailable returns true if there corresponding image on the remote
-// registry is newer.
-func newerRemoteImageAvailable(ctx context.Context, img *libimage.Image, origName string, authfile string) (bool, error) {
-	remoteRef, err := docker.ParseReference("//" + origName)
-	if err != nil {
-		return false, err
-	}
-	options := &libimage.HasDifferentDigestOptions{AuthFilePath: authfile}
-	return img.HasDifferentDigest(ctx, remoteRef, options)
-}
-
-// newerLocalImageAvailable returns true if the container and local image have different digests
-func newerLocalImageAvailable(runtime *libpod.Runtime, img *libimage.Image, rawImageName string) (bool, error) {
-	localImg, _, err := runtime.LibimageRuntime().LookupImage(rawImageName, nil)
-	if err != nil {
-		return false, err
-	}
-	return localImg.Digest().String() != img.Digest().String(), nil
-}
-
 // pullImage pulls the specified image.
-func pullImage(ctx context.Context, runtime *libpod.Runtime, name, authfile string) (*libimage.Image, error) {
+func pullImage(ctx context.Context, runtime *libpod.Runtime, name, authfile string) error {
 	pullOptions := &libimage.PullOptions{}
 	pullOptions.AuthFilePath = authfile
 	pullOptions.Writer = os.Stderr
 
-	pulledImages, err := runtime.LibimageRuntime().Pull(ctx, name, config.PullPolicyAlways, pullOptions)
-	if err != nil {
-		return nil, err
-	}
-	return pulledImages[0], nil
+	_, err := runtime.LibimageRuntime().Pull(ctx, name, config.PullPolicyAlways, pullOptions)
+	return err
 }
-- 
cgit v1.2.3-54-g00ecf


From 43cca5d97a2c971f3386099d6b66250a1ebad96f Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Wed, 3 Aug 2022 16:12:48 +0200
Subject: pkg/autoupdate: decompose the update logic

Decompose the update logic into smaller steps (update check, update,
rollback, etc.) and move the implementation into the `task` API.
This allows to transition a task from state to state, independent of its
underlying auto-update policy.

Supporting more than one container per unit is now really close.

[NO NEW TESTS NEEDED] - should not change behavior.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 21 +++++++++------------
 1 file changed, 9 insertions(+), 12 deletions(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index f450a4233..b3f0c53eb 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -182,6 +182,10 @@ func AutoUpdate(ctx context.Context, runtime *libpod.Runtime, options entities.A
 
 		for _, task := range tasks {
 			err := func() error {
+				// Transition from state to state.  Will be
+				// split into multiple loops in the future to
+				// support more than one container/task per
+				// unit.
 				updateAvailable, err := task.updateAvailable(ctx)
 				if err != nil {
 					task.status = statusFailed
@@ -298,8 +302,11 @@ func (t *task) registryUpdate(ctx context.Context) error {
 		return nil
 	}
 
-	if err := pullImage(ctx, t.auto.runtime, t.rawImageName, t.authfile); err != nil {
-		return fmt.Errorf("registry auto-updating container %q: image update for %q failed: %w", t.container.ID(), t.rawImageName, err)
+	pullOptions := &libimage.PullOptions{}
+	pullOptions.AuthFilePath = t.authfile
+	pullOptions.Writer = os.Stderr
+	if _, err := t.auto.runtime.LibimageRuntime().Pull(ctx, t.rawImageName, config.PullPolicyAlways, pullOptions); err != nil {
+		return err
 	}
 
 	t.auto.updatedRawImages[t.rawImageName] = true
@@ -448,13 +455,3 @@ func (u *updater) assembleImageMap(ctx context.Context) (map[string]*libimage.Im
 
 	return imageMap, nil
 }
-
-// pullImage pulls the specified image.
-func pullImage(ctx context.Context, runtime *libpod.Runtime, name, authfile string) error {
-	pullOptions := &libimage.PullOptions{}
-	pullOptions.AuthFilePath = authfile
-	pullOptions.Writer = os.Stderr
-
-	_, err := runtime.LibimageRuntime().Pull(ctx, name, config.PullPolicyAlways, pullOptions)
-	return err
-}
-- 
cgit v1.2.3-54-g00ecf


From 81a1ea18c8426134b9985469fc02092078912cfe Mon Sep 17 00:00:00 2001
From: Valentin Rothberg <vrothberg@redhat.com>
Date: Thu, 4 Aug 2022 08:22:37 +0200
Subject: pkg/autoupdate: "image" policy: add deprecation comment

The "image" policy has been deprecated in favor of the more precise
"registry" policy.  Add a code comment to leave some breadcrumbs for
future generations.

Signed-off-by: Valentin Rothberg <vrothberg@redhat.com>
---
 pkg/autoupdate/autoupdate.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
index b3f0c53eb..297d6640e 100644
--- a/pkg/autoupdate/autoupdate.go
+++ b/pkg/autoupdate/autoupdate.go
@@ -45,7 +45,7 @@ const (
 var supportedPolicies = map[string]Policy{
 	"":                          PolicyDefault,
 	string(PolicyDefault):       PolicyDefault,
-	"image":                     PolicyRegistryImage,
+	"image":                     PolicyRegistryImage, // Deprecated in favor of PolicyRegistryImage
 	string(PolicyRegistryImage): PolicyRegistryImage,
 	string(PolicyLocalImage):    PolicyLocalImage,
 }
-- 
cgit v1.2.3-54-g00ecf