diff --git a/go.mod b/go.mod index c3125f9ec6..f2e8d4bf5a 100644 --- a/go.mod +++ b/go.mod @@ -13,7 +13,7 @@ require ( github.com/containernetworking/cni v1.1.2 github.com/containernetworking/plugins v1.3.0 github.com/containers/buildah v1.30.1-0.20230504052500-e925b5852e07 - github.com/containers/common v0.53.1-0.20230621174116-586a3be4e1fc + github.com/containers/common v0.53.1-0.20230626115555-370c89881624 github.com/containers/conmon v2.0.20+incompatible github.com/containers/image/v5 v5.25.1-0.20230613183705-07ced6137083 github.com/containers/libhvee v0.0.5 diff --git a/go.sum b/go.sum index f10266a4ad..83189ae016 100644 --- a/go.sum +++ b/go.sum @@ -239,8 +239,8 @@ github.com/containernetworking/plugins v1.3.0 h1:QVNXMT6XloyMUoO2wUOqWTC1hWFV62Q github.com/containernetworking/plugins v1.3.0/go.mod h1:Pc2wcedTQQCVuROOOaLBPPxrEXqqXBFt3cZ+/yVg6l0= github.com/containers/buildah v1.30.1-0.20230504052500-e925b5852e07 h1:Bs2sNFh/fSYr4J6JJLFqzyn3dp6HhlA6ewFwRYUpeIE= github.com/containers/buildah v1.30.1-0.20230504052500-e925b5852e07/go.mod h1:6A/BK0YJLXL8+AqlbceKJrhUT+NtEgsvAc51F7TAllc= -github.com/containers/common v0.53.1-0.20230621174116-586a3be4e1fc h1:6yxDNgJGrddAWKeeAH7m0GUzCFRuvc2BqXund52Ui7k= -github.com/containers/common v0.53.1-0.20230621174116-586a3be4e1fc/go.mod h1:qE1MzGl69IoK7ZNCCH51+aLVjyQtnH0LiZe0wG32Jy0= +github.com/containers/common v0.53.1-0.20230626115555-370c89881624 h1:YBgjfoo0G3tR8vm225ghJnqOZOVv3tH1L1GbyRM9320= +github.com/containers/common v0.53.1-0.20230626115555-370c89881624/go.mod h1:qE1MzGl69IoK7ZNCCH51+aLVjyQtnH0LiZe0wG32Jy0= github.com/containers/conmon v2.0.20+incompatible h1:YbCVSFSCqFjjVwHTPINGdMX1F6JXHGTUje2ZYobNrkg= github.com/containers/conmon v2.0.20+incompatible/go.mod h1:hgwZ2mtuDrppv78a/cOBNiCm6O0UMWGx1mu7P00nu5I= github.com/containers/image/v5 v5.25.1-0.20230613183705-07ced6137083 h1:6Pbnll97ls6G0U3DSxaTqp7Sd8Fykc4gd7BUJm7Bpn8= diff --git a/libpod/container_internal.go b/libpod/container_internal.go index 136902d45a..dd1936ef44 100644 --- a/libpod/container_internal.go +++ b/libpod/container_internal.go @@ -2094,7 +2094,7 @@ func (c *Container) postDeleteHooks(ctx context.Context) error { hook := hook logrus.Debugf("container %s: invoke poststop hook %d, path %s", c.ID(), i, hook.Path) var stderr, stdout bytes.Buffer - hookErr, err := exec.Run(ctx, &hook, state, &stdout, &stderr, exec.DefaultPostKillTimeout) + hookErr, err := exec.Run(ctx, &hook, state, &stdout, &stderr, exec.DefaultPostKillTimeout) //nolint:staticcheck if err != nil { logrus.Warnf("Container %s: poststop hook %d: %v", c.ID(), i, err) if hookErr != err { @@ -2223,7 +2223,7 @@ func (c *Container) setupOCIHooks(ctx context.Context, config *spec.Spec) (map[s } } - hookErr, err := exec.RuntimeConfigFilter(ctx, allHooks["precreate"], config, exec.DefaultPostKillTimeout) + hookErr, err := exec.RuntimeConfigFilter(ctx, allHooks["precreate"], config, exec.DefaultPostKillTimeout) //nolint:staticcheck if err != nil { logrus.Warnf("Container %s: precreate hook: %v", c.ID(), err) if hookErr != nil && hookErr != err { diff --git a/pkg/domain/infra/abi/images_list.go b/pkg/domain/infra/abi/images_list.go index d9661721a3..b980dcc001 100644 --- a/pkg/domain/infra/abi/images_list.go +++ b/pkg/domain/infra/abi/images_list.go @@ -28,54 +28,63 @@ func (ir *ImageEngine) List(ctx context.Context, opts entities.ImageListOptions) summaries := []*entities.ImageSummary{} for _, img := range images { - repoDigests, err := img.RepoDigests() + summary, err := func() (*entities.ImageSummary, error) { + repoDigests, err := img.RepoDigests() + if err != nil { + return nil, fmt.Errorf("getting repoDigests from image %q: %w", img.ID(), err) + } + + if img.ListData.IsDangling == nil { // Sanity check + return nil, fmt.Errorf("%w: ListData.IsDangling is nil but should not be", define.ErrInternal) + } + isDangling := *img.ListData.IsDangling + parentID := "" + if img.ListData.Parent != nil { + parentID = img.ListData.Parent.ID() + } + + s := &entities.ImageSummary{ + ID: img.ID(), + Created: img.Created().Unix(), + Dangling: isDangling, + Digest: string(img.Digest()), + RepoDigests: repoDigests, + History: img.NamesHistory(), + Names: img.Names(), + ReadOnly: img.IsReadOnly(), + SharedSize: 0, + RepoTags: img.Names(), // may include tags and digests + ParentId: parentID, + } + s.Labels, err = img.Labels(ctx) + if err != nil { + return nil, fmt.Errorf("retrieving label for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) + } + + ctnrs, err := img.Containers() + if err != nil { + return nil, fmt.Errorf("retrieving containers for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) + } + s.Containers = len(ctnrs) + + sz, err := img.Size() + if err != nil { + return nil, fmt.Errorf("retrieving size of image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) + } + s.Size = sz + // This is good enough for now, but has to be + // replaced later with correct calculation logic + s.VirtualSize = sz + return s, nil + }() if err != nil { - return nil, fmt.Errorf("getting repoDigests from image %q: %w", img.ID(), err) + if libimage.ErrorIsImageUnknown(err) { + // The image may have been (partially) removed in the meantime + continue + } + return nil, err } - - if img.ListData.IsDangling == nil { // Sanity check - return nil, fmt.Errorf("%w: ListData.IsDangling is nil but should not", define.ErrInternal) - } - isDangling := *img.ListData.IsDangling - parentID := "" - if img.ListData.Parent != nil { - parentID = img.ListData.Parent.ID() - } - - e := entities.ImageSummary{ - ID: img.ID(), - Created: img.Created().Unix(), - Dangling: isDangling, - Digest: string(img.Digest()), - RepoDigests: repoDigests, - History: img.NamesHistory(), - Names: img.Names(), - ReadOnly: img.IsReadOnly(), - SharedSize: 0, - RepoTags: img.Names(), // may include tags and digests - ParentId: parentID, - } - e.Labels, err = img.Labels(ctx) - if err != nil { - return nil, fmt.Errorf("retrieving label for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) - } - - ctnrs, err := img.Containers() - if err != nil { - return nil, fmt.Errorf("retrieving containers for image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) - } - e.Containers = len(ctnrs) - - sz, err := img.Size() - if err != nil { - return nil, fmt.Errorf("retrieving size of image %q: you may need to remove the image to resolve the error: %w", img.ID(), err) - } - e.Size = sz - // This is good enough for now, but has to be - // replaced later with correct calculation logic - e.VirtualSize = sz - - summaries = append(summaries, &e) + summaries = append(summaries, summary) } return summaries, nil } diff --git a/test/system/010-images.bats b/test/system/010-images.bats index 33818194a9..cdb14088fd 100644 --- a/test/system/010-images.bats +++ b/test/system/010-images.bats @@ -363,4 +363,40 @@ EOF run_podman --root $imstore/root rmi --all } +@test "podman images with concurrent removal" { + skip_if_remote "following test is not supported for remote clients" + local count=5 + + # First build $count images + for i in $(seq --format '%02g' 1 $count); do + cat >$PODMAN_TMPDIR/Containerfile <<EOF +FROM $IMAGE +RUN echo $i +EOF + run_podman build -q -t i$i $PODMAN_TMPDIR + done + + run_podman images + # Now remove all images in parallel and in the background and make sure + # that listing all images does not fail (see BZ 2216700). + for i in $(seq --format '%02g' 1 $count); do + timeout --foreground -v --kill=10 60 \ + $PODMAN rmi i$i & + done + + tries=100 + while [[ ${#lines[*]} -gt 1 ]] && [[ $tries -gt 0 ]]; do + # Prior to #18980, 'podman images' during rmi could fail with 'image not known' + run_podman images --format "{{.ID}} {{.Names}}" + tries=$((tries - 1)) + done + + if [[ $tries -eq 0 ]]; then + die "Timed out waiting for images to be removed" + fi + + wait +} + + # vim: filetype=sh diff --git a/vendor/github.com/containers/common/libimage/image.go b/vendor/github.com/containers/common/libimage/image.go index 3897d2f00d..da4ff8b7a4 100644 --- a/vendor/github.com/containers/common/libimage/image.go +++ b/vendor/github.com/containers/common/libimage/image.go @@ -402,7 +402,7 @@ func (i *Image) removeRecursive(ctx context.Context, rmMap map[string]*RemoveIma // have a closer look at the errors. On top, image removal should be // tolerant toward corrupted images. handleError := func(err error) error { - if errors.Is(err, storage.ErrImageUnknown) || errors.Is(err, storage.ErrNotAnImage) || errors.Is(err, storage.ErrLayerUnknown) { + if ErrorIsImageUnknown(err) { // The image or layers of the image may already have been removed // in which case we consider the image to be removed. return nil diff --git a/vendor/github.com/containers/common/libimage/layer_tree.go b/vendor/github.com/containers/common/libimage/layer_tree.go index a7d2f8c588..e6b012f908 100644 --- a/vendor/github.com/containers/common/libimage/layer_tree.go +++ b/vendor/github.com/containers/common/libimage/layer_tree.go @@ -2,8 +2,10 @@ package libimage import ( "context" + "errors" "github.com/containers/storage" + storageTypes "github.com/containers/storage/types" ociv1 "github.com/opencontainers/image-spec/specs-go/v1" "github.com/sirupsen/logrus" ) @@ -30,7 +32,19 @@ func (t *layerTree) node(layerID string) *layerNode { return node } +// ErrorIsImageUnknown returns true if the specified error indicates that an +// image is unknown or has been partially removed (e.g., a missing layer). +func ErrorIsImageUnknown(err error) bool { + return errors.Is(err, storage.ErrImageUnknown) || + errors.Is(err, storageTypes.ErrLayerUnknown) || + errors.Is(err, storageTypes.ErrSizeUnknown) || + errors.Is(err, storage.ErrNotAnImage) +} + // toOCI returns an OCI image for the specified image. +// +// WARNING: callers are responsible for handling cases where the target image +// has been (partially) removed and can use `ErrorIsImageUnknown` to detect it. func (t *layerTree) toOCI(ctx context.Context, i *Image) (*ociv1.Image, error) { var err error oci, exists := t.ociCache[i.ID()] @@ -155,6 +169,9 @@ func (t *layerTree) children(ctx context.Context, parent *Image, all bool) ([]*I parentID := parent.ID() parentOCI, err := t.toOCI(ctx, parent) if err != nil { + if ErrorIsImageUnknown(err) { + return nil, nil + } return nil, err } @@ -165,6 +182,9 @@ func (t *layerTree) children(ctx context.Context, parent *Image, all bool) ([]*I } childOCI, err := t.toOCI(ctx, child) if err != nil { + if ErrorIsImageUnknown(err) { + return false, nil + } return false, err } // History check. @@ -255,6 +275,9 @@ func (t *layerTree) parent(ctx context.Context, child *Image) (*Image, error) { childID := child.ID() childOCI, err := t.toOCI(ctx, child) if err != nil { + if ErrorIsImageUnknown(err) { + return nil, nil + } return nil, err } @@ -268,6 +291,9 @@ func (t *layerTree) parent(ctx context.Context, child *Image) (*Image, error) { } emptyOCI, err := t.toOCI(ctx, empty) if err != nil { + if ErrorIsImageUnknown(err) { + return nil, nil + } return nil, err } // History check. @@ -300,6 +326,9 @@ func (t *layerTree) parent(ctx context.Context, child *Image) (*Image, error) { } parentOCI, err := t.toOCI(ctx, parent) if err != nil { + if ErrorIsImageUnknown(err) { + return nil, nil + } return nil, err } // History check. diff --git a/vendor/github.com/containers/common/libnetwork/slirp4netns/slirp4netns.go b/vendor/github.com/containers/common/libnetwork/slirp4netns/slirp4netns.go index 712abedbc3..0e53a922f6 100644 --- a/vendor/github.com/containers/common/libnetwork/slirp4netns/slirp4netns.go +++ b/vendor/github.com/containers/common/libnetwork/slirp4netns/slirp4netns.go @@ -69,8 +69,6 @@ type SetupOptions struct { ContainerID string // Netns path to the netns Netns string - // ContainerPID is the pid of container process - ContainerPID int // Ports the should be forwarded Ports []types.PortMapping // ExtraOptions for slirp4netns that were set on the cli @@ -84,6 +82,9 @@ type SetupOptions struct { // RootlessPortSyncPipe pipe used to exit the rootlessport process. // Same as Slirp4netnsExitPipeR, except this is only used when ports are given. RootlessPortExitPipeR *os.File + // Pdeathsig is the signal which is send to slirp4netns process if the calling thread + // exits. The caller is responsible for locking the thread with runtime.LockOSThread(). + Pdeathsig syscall.Signal } // SetupResult return type from Setup() @@ -309,7 +310,8 @@ func Setup(opts *SetupOptions) (*SetupResult, error) { cmd := exec.Command(path, cmdArgs...) logrus.Debugf("slirp4netns command: %s", strings.Join(cmd.Args, " ")) cmd.SysProcAttr = &syscall.SysProcAttr{ - Setpgid: true, + Setpgid: true, + Pdeathsig: opts.Pdeathsig, } // workaround for https://github.com/rootless-containers/slirp4netns/pull/153 diff --git a/vendor/github.com/containers/common/pkg/hooks/exec/exec.go b/vendor/github.com/containers/common/pkg/hooks/exec/exec.go index bc639245f2..2bd3a2adfc 100644 --- a/vendor/github.com/containers/common/pkg/hooks/exec/exec.go +++ b/vendor/github.com/containers/common/pkg/hooks/exec/exec.go @@ -16,16 +16,50 @@ import ( // DefaultPostKillTimeout is the recommended default post-kill timeout. const DefaultPostKillTimeout = time.Duration(10) * time.Second +type RunOptions struct { + // The hook to run + Hook *rspec.Hook + // The workdir to change when invoking the hook + Dir string + // The container state data to pass into the hook process + State []byte + // Stdout from the hook process + Stdout io.Writer + // Stderr from the hook process + Stderr io.Writer + // Timeout for waiting process killed + PostKillTimeout time.Duration +} + // Run executes the hook and waits for it to complete or for the // context or hook-specified timeout to expire. +// +// Deprecated: Too many arguments, has been refactored and replaced by RunWithOptions instead func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer, stderr io.Writer, postKillTimeout time.Duration) (hookErr, err error) { + return RunWithOptions( + ctx, + RunOptions{ + Hook: hook, + State: state, + Stdout: stdout, + Stderr: stderr, + PostKillTimeout: postKillTimeout, + }, + ) +} + +// RunWithOptions executes the hook and waits for it to complete or for the +// context or hook-specified timeout to expire. +func RunWithOptions(ctx context.Context, options RunOptions) (hookErr, err error) { + hook := options.Hook cmd := osexec.Cmd{ Path: hook.Path, Args: hook.Args, Env: hook.Env, - Stdin: bytes.NewReader(state), - Stdout: stdout, - Stderr: stderr, + Dir: options.Dir, + Stdin: bytes.NewReader(options.State), + Stdout: options.Stdout, + Stderr: options.Stderr, } if cmd.Env == nil { cmd.Env = []string{} @@ -57,11 +91,11 @@ func Run(ctx context.Context, hook *rspec.Hook, state []byte, stdout io.Writer, if err := cmd.Process.Kill(); err != nil { logrus.Errorf("Failed to kill pid %v", cmd.Process) } - timer := time.NewTimer(postKillTimeout) + timer := time.NewTimer(options.PostKillTimeout) defer timer.Stop() select { case <-timer.C: - err = fmt.Errorf("failed to reap process within %s of the kill signal", postKillTimeout) + err = fmt.Errorf("failed to reap process within %s of the kill signal", options.PostKillTimeout) case err = <-exit: } return err, ctx.Err() diff --git a/vendor/github.com/containers/common/pkg/hooks/exec/runtimeconfigfilter.go b/vendor/github.com/containers/common/pkg/hooks/exec/runtimeconfigfilter.go index 72d4b89792..ac17ac64da 100644 --- a/vendor/github.com/containers/common/pkg/hooks/exec/runtimeconfigfilter.go +++ b/vendor/github.com/containers/common/pkg/hooks/exec/runtimeconfigfilter.go @@ -21,19 +21,44 @@ var spewConfig = spew.ConfigState{ SortKeys: true, } +type RuntimeConfigFilterOptions struct { + // The hooks to run + Hooks []spec.Hook + // The workdir to change when invoking the hook + Dir string + // The container config spec to pass into the hook processes and potentially get modified by them + Config *spec.Spec + // Timeout for waiting process killed + PostKillTimeout time.Duration +} + // RuntimeConfigFilter calls a series of hooks. But instead of // passing container state on their standard input, // RuntimeConfigFilter passes the proposed runtime configuration (and // reads back a possibly-altered form from their standard output). +// +// Deprecated: Too many arguments, has been refactored and replaced by RuntimeConfigFilterWithOptions instead func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Spec, postKillTimeout time.Duration) (hookErr, err error) { - data, err := json.Marshal(config) + return RuntimeConfigFilterWithOptions(ctx, RuntimeConfigFilterOptions{ + Hooks: hooks, + Config: config, + PostKillTimeout: postKillTimeout, + }) +} + +// RuntimeConfigFilterWithOptions calls a series of hooks. But instead of +// passing container state on their standard input, +// RuntimeConfigFilterWithOptions passes the proposed runtime configuration (and +// reads back a possibly-altered form from their standard output). +func RuntimeConfigFilterWithOptions(ctx context.Context, options RuntimeConfigFilterOptions) (hookErr, err error) { + data, err := json.Marshal(options.Config) if err != nil { return nil, err } - for i, hook := range hooks { + for i, hook := range options.Hooks { hook := hook var stdout bytes.Buffer - hookErr, err = Run(ctx, &hook, data, &stdout, nil, postKillTimeout) + hookErr, err = RunWithOptions(ctx, RunOptions{Hook: &hook, Dir: options.Dir, State: data, Stdout: &stdout, PostKillTimeout: options.PostKillTimeout}) if err != nil { return hookErr, err } @@ -46,8 +71,8 @@ func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Sp return nil, fmt.Errorf("unmarshal output from config-filter hook %d: %w", i, err) } - if !reflect.DeepEqual(config, &newConfig) { - oldConfig := spewConfig.Sdump(config) + if !reflect.DeepEqual(options.Config, &newConfig) { + oldConfig := spewConfig.Sdump(options.Config) newConfig := spewConfig.Sdump(&newConfig) diff, err := difflib.GetUnifiedDiffString(difflib.UnifiedDiff{ A: difflib.SplitLines(oldConfig), @@ -65,7 +90,7 @@ func RuntimeConfigFilter(ctx context.Context, hooks []spec.Hook, config *spec.Sp } } - *config = newConfig + *options.Config = newConfig } return nil, nil diff --git a/vendor/modules.txt b/vendor/modules.txt index ae39d1148a..9c6aa8fae3 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -125,7 +125,7 @@ github.com/containers/buildah/pkg/rusage github.com/containers/buildah/pkg/sshagent github.com/containers/buildah/pkg/util github.com/containers/buildah/util -# github.com/containers/common v0.53.1-0.20230621174116-586a3be4e1fc +# github.com/containers/common v0.53.1-0.20230626115555-370c89881624 ## explicit; go 1.18 github.com/containers/common/libimage github.com/containers/common/libimage/define