opensource/podman
1
0
Fork 0
mirror of https://github.com/containers/podman.git synced 2025-06-20 17:13:43 +08:00
Code Issues Packages Projects Releases Wiki Activity

Merge pull request #18636 from mtrmac/cleanupStorage-error

Browse Source 
Fix, and reduce repetitiveness, in container cleanup error handling
This commit is contained in:
OpenShift Merge Robot
2023-05-23 07:43:01 -04:00
committed by GitHub
parent d7b811677e 032d4a95f0
commit fe64f79469
2 changed files with 32 additions and 76 deletions
Download Patch File Download Diff File Expand all files Collapse all files

56
libpod/container_internal.go
View File

@ -1816,6 +1816,14 @@ func (c *Container) cleanupStorage() error {
}
var cleanupErr error
reportErrorf := func(msg string, args ...any) {
err := fmt.Errorf(msg, args...) // Always use fmt.Errorf instead of just logrus.Errorf(…) because the format string probably contains %w
if cleanupErr == nil {
cleanupErr = err
} else {
logrus.Errorf("%s", err.Error())
}
}
markUnmounted := func() {
c.state.Mountpoint = ""
@ -1823,11 +1831,7 @@ func (c *Container) cleanupStorage() error {
if c.valid {
if err := c.save(); err != nil {
if cleanupErr != nil {
logrus.Errorf("Unmounting container %s: %v", c.ID(), err)
} else {
cleanupErr = err
}
reportErrorf("unmounting container %s: %w", c.ID(), err)
}
}
}
@ -1836,40 +1840,24 @@ func (c *Container) cleanupStorage() error {
if c.config.RootfsOverlay {
overlayBasePath := filepath.Dir(c.state.Mountpoint)
if err := overlay.Unmount(overlayBasePath); err != nil {
if cleanupErr != nil {
logrus.Errorf("Failed to clean up overlay mounts for %s: %v", c.ID(), err)
} else {
cleanupErr = fmt.Errorf("failed to clean up overlay mounts for %s: %w", c.ID(), err)
}
reportErrorf("failed to clean up overlay mounts for %s: %w", c.ID(), err)
}
}
if c.config.RootfsMapping != nil {
if err := unix.Unmount(c.config.Rootfs, 0); err != nil && err != unix.EINVAL {
if cleanupErr != nil {
logrus.Errorf("Unmounting idmapped rootfs for container %s after mount error: %v", c.ID(), err)
} else {
cleanupErr = fmt.Errorf("unmounting idmapped rootfs for container %s after mount error: %w", c.ID(), err)
}
reportErrorf("unmounting idmapped rootfs for container %s after mount error: %w", c.ID(), err)
}
}
for _, containerMount := range c.config.Mounts {
if err := c.unmountSHM(containerMount); err != nil {
if cleanupErr != nil {
logrus.Errorf("Unmounting container %s: %v", c.ID(), err)
} else {
cleanupErr = fmt.Errorf("unmounting container %s: %w", c.ID(), err)
}
reportErrorf("unmounting container %s: %w", c.ID(), err)
}
}
if err := c.cleanupOverlayMounts(); err != nil {
// If the container can't remove content report the error
if cleanupErr != nil {
logrus.Errorf("Failed to clean up overlay mounts for %s: %v", c.ID(), err)
} else {
cleanupErr = fmt.Errorf("failed to clean up overlay mounts for %s: %w", c.ID(), err)
}
reportErrorf("failed to clean up overlay mounts for %s: %w", c.ID(), err)
}
if c.config.Rootfs != "" {
@ -1885,11 +1873,7 @@ func (c *Container) cleanupStorage() error {
if errors.Is(err, storage.ErrNotAContainer) || errors.Is(err, storage.ErrContainerUnknown) || errors.Is(err, storage.ErrLayerNotMounted) {
logrus.Errorf("Storage for container %s has been removed", c.ID())
} else {
if cleanupErr != nil {
logrus.Errorf("Cleaning up container %s storage: %v", c.ID(), cleanupErr)
} else {
cleanupErr = fmt.Errorf("cleaning up container %s storage: %w", c.ID(), cleanupErr)
}
reportErrorf("cleaning up container %s storage: %w", c.ID(), err)
}
}
@ -1897,11 +1881,7 @@ func (c *Container) cleanupStorage() error {
for _, v := range c.config.NamedVolumes {
vol, err := c.runtime.state.Volume(v.Name)
if err != nil {
if cleanupErr != nil {
logrus.Errorf("Retrieving named volume %s for container %s: %v", v.Name, c.ID(), err)
} else {
cleanupErr = fmt.Errorf("retrieving named volume %s for container %s: %w", v.Name, c.ID(), err)
}
reportErrorf("retrieving named volume %s for container %s: %w", v.Name, c.ID(), err)
// We need to try and unmount every volume, so continue
// if they fail.
@ -1911,11 +1891,7 @@ func (c *Container) cleanupStorage() error {
if vol.needsMount() {
vol.lock.Lock()
if err := vol.unmount(false); err != nil {
if cleanupErr != nil {
logrus.Errorf("Unmounting volume %s for container %s: %v", vol.Name(), c.ID(), err)
} else {
cleanupErr = fmt.Errorf("unmounting volume %s for container %s: %w", vol.Name(), c.ID(), err)
}
reportErrorf("unmounting volume %s for container %s: %w", vol.Name(), c.ID(), err)
}
vol.lock.Unlock()
}

52
libpod/runtime_ctr.go
View File

@ -766,12 +766,20 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo
}
var cleanupErr error
reportErrorf := func(msg string, args ...any) {
err := fmt.Errorf(msg, args...) // Always use fmt.Errorf instead of just logrus.Errorf(…) because the format string probably contains %w
if cleanupErr == nil {
cleanupErr = err
} else {
logrus.Errorf("%s", err.Error())
}
}
// Clean up network namespace, cgroups, mounts.
// Do this before we set ContainerStateRemoving, to ensure that we can
// actually remove from the OCI runtime.
if err := c.cleanup(ctx); err != nil {
cleanupErr = fmt.Errorf("cleaning up container %s: %w", c.ID(), err)
reportErrorf("cleaning up container %s: %w", c.ID(), err)
}
// Remove all active exec sessions
@ -779,11 +787,7 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo
// after setting the state to ContainerStateRemoving will prevent that the container is
// restarted
if err := c.removeAllExecSessions(); err != nil {
if cleanupErr == nil {
cleanupErr = err
} else {
logrus.Errorf("Remove exec sessions: %v", err)
}
reportErrorf("removing exec sessions: %w", err)
}
// Set ContainerStateRemoving as an intermediate state (we may get
@ -792,31 +796,19 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo
if err := c.save(); err != nil {
if !errors.Is(err, define.ErrCtrRemoved) {
if cleanupErr == nil {
cleanupErr = err
} else {
logrus.Errorf("Saving container: %v", err)
}
reportErrorf("saving container: %w", err)
}
}
// Stop the container's storage
if err := c.teardownStorage(); err != nil {
if cleanupErr == nil {
cleanupErr = err
} else {
logrus.Errorf("Cleaning up storage: %v", err)
}
reportErrorf("cleaning up storage: %w", err)
}
// Remove the container's CID file on container removal.
if cidFile, ok := c.config.Spec.Annotations[define.InspectAnnotationCIDFile]; ok {
if err := os.Remove(cidFile); err != nil && !errors.Is(err, os.ErrNotExist) {
if cleanupErr == nil {
cleanupErr = err
} else {
logrus.Errorf("Cleaning up CID file: %v", err)
}
reportErrorf("cleaning up CID file: %w", err)
}
}
// Remove the container from the state
@ -824,29 +816,17 @@ func (r *Runtime) removeContainer(ctx context.Context, c *Container, force, remo
// If we're removing the pod, the container will be evicted
// from the state elsewhere
if err := r.state.RemoveContainerFromPod(pod, c); err != nil {
if cleanupErr == nil {
cleanupErr = err
} else {
logrus.Errorf("Removing container %s from database: %v", c.ID(), err)
}
reportErrorf("removing container %s from database: %w", c.ID(), err)
}
} else {
if err := r.state.RemoveContainer(c); err != nil {
if cleanupErr == nil {
cleanupErr = err
} else {
logrus.Errorf("Removing container %s from database: %v", c.ID(), err)
}
reportErrorf("removing container %s from database: %w", c.ID(), err)
}
}
// Deallocate the container's lock
if err := c.lock.Free(); err != nil {
if cleanupErr == nil && !os.IsNotExist(err) {
cleanupErr = fmt.Errorf("freeing lock for container %s: %w", c.ID(), err)
} else {
logrus.Errorf("Free container lock: %v", err)
}
reportErrorf("freeing lock for container %s: %w", c.ID(), err)
}
// Set container as invalid so it can no longer be used