diff options
author | Toshiki Sonoda <sonoda.toshiki@fujitsu.com> | 2022-07-26 09:56:24 +0900 |
---|---|---|
committer | Toshiki Sonoda <sonoda.toshiki@fujitsu.com> | 2022-07-27 10:33:15 +0900 |
commit | 8628c01f5e085335afc5ef3b4a019d09d2147858 (patch) | |
tree | 835c7d1da4f6fbf6aa0a0f17e45e13979c2c8576 | |
parent | da1f47921685f40f1b26405278cbf9cb2d06fe09 (diff) | |
download | podman-8628c01f5e085335afc5ef3b4a019d09d2147858.tar.gz podman-8628c01f5e085335afc5ef3b4a019d09d2147858.tar.bz2 podman-8628c01f5e085335afc5ef3b4a019d09d2147858.zip |
Fix: manifest push --rm removes a correct manifest list
This bug is reproduced when we execute the following command:
1. podman manifest add <manifest list> <images exist on local storage>
2. podman manifest push --rm <manifest list> dir:<directory>
If pushing succeeds, it is expected to remove only a manifest list.
However, manifest list remains on local storage and images are removed.
This commit fixes `podman manifest push --rm` to remove only a manifest list.
And, supports `manifest push --rm option` in remote environment,
like host environment.
Fixes: https://github.com/containers/podman/issues/15033
Signed-off-by: Toshiki Sonoda <sonoda.toshiki@fujitsu.com>
-rw-r--r-- | pkg/domain/infra/abi/manifest.go | 3 | ||||
-rw-r--r-- | pkg/domain/infra/tunnel/manifest.go | 10 | ||||
-rw-r--r-- | test/e2e/manifest_test.go | 44 |
3 files changed, 50 insertions, 7 deletions
diff --git a/pkg/domain/infra/abi/manifest.go b/pkg/domain/infra/abi/manifest.go index d20744d76..bdc3d9513 100644 --- a/pkg/domain/infra/abi/manifest.go +++ b/pkg/domain/infra/abi/manifest.go @@ -331,7 +331,8 @@ func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination strin } if opts.Rm { - if _, rmErrors := ir.Libpod.LibimageRuntime().RemoveImages(ctx, []string{manifestList.ID()}, nil); len(rmErrors) > 0 { + rmOpts := &libimage.RemoveImagesOptions{LookupManifest: true} + if _, rmErrors := ir.Libpod.LibimageRuntime().RemoveImages(ctx, []string{manifestList.ID()}, rmOpts); len(rmErrors) > 0 { return "", fmt.Errorf("error removing manifest after push: %w", rmErrors[0]) } } diff --git a/pkg/domain/infra/tunnel/manifest.go b/pkg/domain/infra/tunnel/manifest.go index d2554f198..4a3148fac 100644 --- a/pkg/domain/infra/tunnel/manifest.go +++ b/pkg/domain/infra/tunnel/manifest.go @@ -110,5 +110,15 @@ func (ir *ImageEngine) ManifestPush(ctx context.Context, name, destination strin } } digest, err := manifests.Push(ir.ClientCtx, name, destination, options) + if err != nil { + return "", fmt.Errorf("error adding to manifest list %s: %w", name, err) + } + + if opts.Rm { + if _, rmErrors := ir.Remove(ctx, []string{name}, entities.ImageRemoveOptions{LookupManifest: true}); len(rmErrors) > 0 { + return "", fmt.Errorf("error removing manifest after push: %w", rmErrors[0]) + } + } + return digest, err } diff --git a/test/e2e/manifest_test.go b/test/e2e/manifest_test.go index 1f58419a1..a7fcd1559 100644 --- a/test/e2e/manifest_test.go +++ b/test/e2e/manifest_test.go @@ -339,6 +339,16 @@ var _ = Describe("Podman manifest", func() { push = podmanTest.Podman([]string{"manifest", "push", "--tls-verify=false", "--creds=podmantest:wrongpasswd", "foo", "localhost:" + registry.Port + "/credstest"}) push.WaitWithDefaultTimeout() Expect(push).To(ExitWithError()) + + // push --rm after pull image (#15033) + push = podmanTest.Podman([]string{"manifest", "push", "--rm", "--tls-verify=false", "--creds=" + registry.User + ":" + registry.Password, "foo", "localhost:" + registry.Port + "/rmtest"}) + push.WaitWithDefaultTimeout() + Expect(push).Should(Exit(0)) + + session = podmanTest.Podman([]string{"images", "-q", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) }) It("push with error", func() { @@ -348,8 +358,8 @@ var _ = Describe("Podman manifest", func() { Expect(session.ErrorToString()).NotTo(BeEmpty()) }) - It("push --rm", func() { - SkipIfRemote("remote does not support --rm") + It("push --rm to local directory", func() { + SkipIfRemote("manifest push to dir not supported in remote mode") session := podmanTest.Podman([]string{"manifest", "create", "foo"}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) @@ -365,13 +375,35 @@ var _ = Describe("Podman manifest", func() { session = podmanTest.Podman([]string{"manifest", "push", "--purge", "foo", "dir:" + dest}) session.WaitWithDefaultTimeout() Expect(session).Should(Exit(0)) - session = podmanTest.Podman([]string{"manifest", "inspect", "foo"}) + session = podmanTest.Podman([]string{"images", "-q", "foo"}) session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) + Expect(session).Should(Exit(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) - session = podmanTest.Podman([]string{"manifest", "rm", "foo1", "foo2"}) + // push --rm after pull image (#15033) + session = podmanTest.Podman([]string{"pull", "quay.io/libpod/testdigest_v2s2"}) session.WaitWithDefaultTimeout() - Expect(session).To(ExitWithError()) + Expect(session).Should(Exit(0)) + + session = podmanTest.Podman([]string{"manifest", "create", "bar"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + session = podmanTest.Podman([]string{"manifest", "add", "bar", "quay.io/libpod/testdigest_v2s2"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + session = podmanTest.Podman([]string{"manifest", "push", "--rm", "bar", "dir:" + dest}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + session = podmanTest.Podman([]string{"images", "-q", "bar"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(Exit(0)) + Expect(podmanTest.NumberOfContainersRunning()).To(Equal(0)) + + session = podmanTest.Podman([]string{"manifest", "rm", "foo", "bar"}) + session.WaitWithDefaultTimeout() + Expect(session).Should(ExitWithError()) + Expect(session.ErrorToString()).To(ContainSubstring("foo: image not known")) + Expect(session.ErrorToString()).To(ContainSubstring("bar: image not known")) }) It("exists", func() { |