Merge pull request #23305 from Luap99/auto-update-race

podman auto-update/system df: fix ErrNoSuchCtr/Volume race
containers · Jul 18, 2024 · 22cf2b4 · 22cf2b4
2 parents 599967b + 46f4044
commit 22cf2b4
Show file tree

Hide file tree

Showing 2 changed files with 27 additions and 16 deletions.
diff --git a/pkg/autoupdate/autoupdate.go b/pkg/autoupdate/autoupdate.go
@@ -4,6 +4,7 @@ package autoupdate
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"sort"
@@ -367,12 +368,15 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 
 	u.unitToTasks = make(map[string][]*task)
 
-	errors := []error{}
+	errs := []error{}
 	for _, c := range allContainers {
 		ctr := c
 		state, err := ctr.State()
 		if err != nil {
-			errors = append(errors, err)
+			// container may have been removed in the meantime ignore it and not print errors
+			if !errors.Is(err, define.ErrNoSuchCtr) {
+				errs = append(errs, err)
+			}
 			continue
 		}
 		// Only update running containers.
@@ -389,7 +393,7 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 		}
 		policy, err := LookupPolicy(value)
 		if err != nil {
-			errors = append(errors, err)
+			errs = append(errs, err)
 			continue
 		}
 		if policy == PolicyDefault {
@@ -400,25 +404,25 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 		// stored as a label at container creation.
 		unit, exists, err := u.systemdUnitForContainer(ctr, labels)
 		if err != nil {
-			errors = append(errors, err)
+			errs = append(errs, err)
 			continue
 		}
 		if !exists {
-			errors = append(errors, fmt.Errorf("auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable))
+			errs = append(errs, fmt.Errorf("auto-updating container %q: no %s label found", ctr.ID(), systemdDefine.EnvVariable))
 			continue
 		}
 
 		id, _ := ctr.Image()
 		image, exists := imageMap[id]
 		if !exists {
 			err := fmt.Errorf("internal error: no image found for ID %s", id)
-			errors = append(errors, err)
+			errs = append(errs, err)
 			continue
 		}
 
 		rawImageName := ctr.RawImageName()
 		if rawImageName == "" {
-			errors = append(errors, fmt.Errorf("locally auto-updating container %q: raw-image name is empty", ctr.ID()))
+			errs = append(errs, fmt.Errorf("locally auto-updating container %q: raw-image name is empty", ctr.ID()))
 			continue
 		}
 
@@ -443,7 +447,7 @@ func (u *updater) assembleTasks(ctx context.Context) []error {
 		u.unitToTasks[unit] = append(u.unitToTasks[unit], &t)
 	}
 
-	return errors
+	return errs
 }
 
 // systemdUnitForContainer returns the name of the container's systemd unit.

diff --git a/pkg/domain/infra/abi/system.go b/pkg/domain/infra/abi/system.go
@@ -198,23 +198,24 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
 		iid, _ := c.Image()
 		state, err := c.State()
 		if err != nil {
+			if errors.Is(err, define.ErrNoSuchCtr) {
+				continue
+			}
 			return nil, fmt.Errorf("failed to get state of container %s: %w", c.ID(), err)
 		}
 		conSize, err := c.RootFsSize()
 		if err != nil {
-			if errors.Is(err, storage.ErrContainerUnknown) {
-				logrus.Error(fmt.Errorf("failed to get root file system size of container %s: %w", c.ID(), err))
-			} else {
-				return nil, fmt.Errorf("failed to get root file system size of container %s: %w", c.ID(), err)
+			if errors.Is(err, storage.ErrContainerUnknown) || errors.Is(err, define.ErrNoSuchCtr) {
+				continue
 			}
+			return nil, fmt.Errorf("failed to get root file system size of container %s: %w", c.ID(), err)
 		}
 		rwsize, err := c.RWSize()
 		if err != nil {
-			if errors.Is(err, storage.ErrContainerUnknown) {
-				logrus.Error(fmt.Errorf("failed to get read/write size of container %s: %w", c.ID(), err))
-			} else {
-				return nil, fmt.Errorf("failed to get read/write size of container %s: %w", c.ID(), err)
+			if errors.Is(err, storage.ErrContainerUnknown) || errors.Is(err, define.ErrNoSuchCtr) {
+				continue
 			}
+			return nil, fmt.Errorf("failed to get read/write size of container %s: %w", c.ID(), err)
 		}
 		report := entities.SystemDfContainerReport{
 			ContainerID:  c.ID(),
@@ -241,6 +242,9 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
 		var reclaimableSize int64
 		mountPoint, err := v.MountPoint()
 		if err != nil {
+			if errors.Is(err, define.ErrNoSuchVolume) {
+				continue
+			}
 			return nil, err
 		}
 		if mountPoint == "" {
@@ -255,6 +259,9 @@ func (ic *ContainerEngine) SystemDf(ctx context.Context, options entities.System
 		}
 		inUse, err := v.VolumeInUse()
 		if err != nil {
+			if errors.Is(err, define.ErrNoSuchVolume) {
+				continue
+			}
 			return nil, err
 		}
 		if len(inUse) == 0 {