diff options
-rw-r--r-- | cmd/podman/rmi.go | 4 | ||||
-rw-r--r-- | libpod/image/image.go | 57 | ||||
-rw-r--r-- | libpod/image/image_test.go | 49 | ||||
-rw-r--r-- | libpod/runtime_img.go | 38 | ||||
-rw-r--r-- | test/e2e/push_test.go | 8 | ||||
-rw-r--r-- | test/e2e/rmi_test.go | 36 |
6 files changed, 167 insertions, 25 deletions
diff --git a/cmd/podman/rmi.go b/cmd/podman/rmi.go index b38686a87..cc902cdbe 100644 --- a/cmd/podman/rmi.go +++ b/cmd/podman/rmi.go @@ -75,14 +75,14 @@ func rmiCmd(c *cli.Context) error { return errors.Errorf("no valid images to delete") } for _, img := range imagesToDelete { - err := runtime.RemoveImage(img, c.Bool("force")) + msg, err := runtime.RemoveImage(img, c.Bool("force")) if err != nil { if lastError != nil { fmt.Fprintln(os.Stderr, lastError) } lastError = err } else { - fmt.Println(img.ID()) + fmt.Println(msg) } } return lastError diff --git a/libpod/image/image.go b/libpod/image/image.go index 633a682cc..b6e15e361 100644 --- a/libpod/image/image.go +++ b/libpod/image/image.go @@ -685,3 +685,60 @@ func Import(path, reference string, writer io.Writer, signingOptions SigningOpti } return runtime.NewFromLocal(reference) } + +// MatchRepoTag takes a string and tries to match it against an +// image's repotags +func (i *Image) MatchRepoTag(input string) (string, error) { + results := make(map[int][]string) + var maxCount int + // first check if we have an exact match with the input + if util.StringInSlice(input, i.Names()) { + return input, nil + } + // next check if we are missing the tag + dcImage, err := decompose(input) + if err != nil { + return "", err + } + for _, repoName := range i.Names() { + count := 0 + dcRepoName, err := decompose(repoName) + if err != nil { + return "", err + } + if dcRepoName.registry == dcImage.registry && dcImage.registry != "" { + count++ + } + if dcRepoName.name == dcImage.name && dcImage.name != "" { + count++ + } else if splitString(dcRepoName.name) == splitString(dcImage.name) { + count++ + } + if dcRepoName.tag == dcImage.tag { + count++ + } + results[count] = append(results[count], repoName) + if count > maxCount { + maxCount = count + } + } + if maxCount == 0 { + return "", errors.Errorf("unable to match user input to any specific repotag") + } + if len(results[maxCount]) > 1 { + return "", errors.Errorf("user input matched multiple repotags for the image") + } + return results[maxCount][0], nil +} + +// splitString splits input string by / and returns the last array item +func splitString(input string) string { + split := strings.Split(input, "/") + return split[len(split)-1] +} + +// InputIsID returns a bool if the user input for an image +// is the image's partial or full id +func (i *Image) InputIsID() bool { + return strings.HasPrefix(i.ID(), i.InputName) +} diff --git a/libpod/image/image_test.go b/libpod/image/image_test.go index d9e8987a6..5c7cc191d 100644 --- a/libpod/image/image_test.go +++ b/libpod/image/image_test.go @@ -136,3 +136,52 @@ func TestImage_New(t *testing.T) { // Shutdown the runtime and remove the temporary storage cleanup(workdir, ir) } + +// TestImage_MatchRepoTag tests the various inputs we need to match +// against an image's reponames +func TestImage_MatchRepoTag(t *testing.T) { + //Set up + workdir, err := mkWorkDir() + assert.NoError(t, err) + + so := storage.StoreOptions{ + RunRoot: workdir, + GraphRoot: workdir, + } + ir, err := NewImageRuntimeFromOptions(so) + assert.NoError(t, err) + newImage, err := ir.New("busybox", "", "", os.Stdout, nil, SigningOptions{}) + assert.NoError(t, err) + err = newImage.TagImage("foo:latest") + assert.NoError(t, err) + err = newImage.TagImage("foo:bar") + assert.NoError(t, err) + + // Tests start here. + for _, name := range bbNames { + repoTag, err := newImage.MatchRepoTag(name) + assert.NoError(t, err) + assert.Equal(t, "docker.io/library/busybox:latest", repoTag) + } + + // Test against tagged images of busybox + // foo should resolve to foo:latest + repoTag, err := newImage.MatchRepoTag("foo") + assert.NoError(t, err) + assert.Equal(t, "foo:latest", repoTag) + + // foo:bar should resolve to foo:bar + repoTag, err = newImage.MatchRepoTag("foo:bar") + assert.NoError(t, err) + assert.Equal(t, "foo:bar", repoTag) + // Shutdown the runtime and remove the temporary storage + cleanup(workdir, ir) +} + +// Test_splitString tests the splitString function in image that +// takes input and splits on / and returns the last array item +func Test_splitString(t *testing.T) { + assert.Equal(t, splitString("foo/bar"), "bar") + assert.Equal(t, splitString("a/foo/bar"), "bar") + assert.Equal(t, splitString("bar"), "bar") +} diff --git a/libpod/runtime_img.go b/libpod/runtime_img.go index 248d1c0aa..c830304ae 100644 --- a/libpod/runtime_img.go +++ b/libpod/runtime_img.go @@ -834,18 +834,18 @@ func (r *Runtime) UntagImage(image *storage.Image, tag string) (string, error) { // RemoveImage deletes an image from local storage // Images being used by running containers can only be removed if force=true -func (r *Runtime) RemoveImage(image *image.Image, force bool) error { +func (r *Runtime) RemoveImage(image *image.Image, force bool) (string, error) { r.lock.Lock() defer r.lock.Unlock() if !r.valid { - return ErrRuntimeStopped + return "", ErrRuntimeStopped } // Get all containers, filter to only those using the image, and remove those containers ctrs, err := r.state.AllContainers() if err != nil { - return err + return "", err } imageCtrs := []*Container{} for _, ctr := range ctrs { @@ -857,29 +857,33 @@ func (r *Runtime) RemoveImage(image *image.Image, force bool) error { if force { for _, ctr := range imageCtrs { if err := r.removeContainer(ctr, true); err != nil { - return errors.Wrapf(err, "error removing image %s: container %s using image could not be removed", image.ID, ctr.ID()) + return "", errors.Wrapf(err, "error removing image %s: container %s using image could not be removed", image.ID, ctr.ID()) } } } else { - return fmt.Errorf("could not remove image %s as it is being used by %d containers", image.ID, len(imageCtrs)) + return "", fmt.Errorf("could not remove image %s as it is being used by %d containers", image.ID, len(imageCtrs)) } } - if len(image.Names()) > 1 && !force { - return fmt.Errorf("unable to delete %s (must force) - image is referred to in multiple tags", image.ID) - } - - if len(image.Names()) > 1 && force { - // If it is forced, we have to untag the image so that it can be deleted - if err = r.store.SetNames(image.ID(), image.Names()[:0]); err != nil { - return err + if len(image.Names()) > 1 && !image.InputIsID() { + // If the image has multiple reponames, we do not technically delete + // the image. we figure out which repotag the user is trying to refer + // to and untag it. + repoName, err := image.MatchRepoTag(image.InputName) + if err != nil { + return "", err + } + if err := image.UntagImage(repoName); err != nil { + return "", err } + return fmt.Sprintf("Untagged: %s", repoName), nil + } else if len(image.Names()) > 1 && image.InputIsID() && !force { + // If the user requests to delete an image by ID and the image has multiple + // reponames and no force is applied, we error out. + return "", fmt.Errorf("unable to delete %s (must force) - image is referred to in multiple tags", image.ID()) } - if err = image.Remove(force); err != nil { - return err - } - return nil + return image.ID(), image.Remove(force) } // GetImage retrieves an image matching the given name or hash from system diff --git a/test/e2e/push_test.go b/test/e2e/push_test.go index 028f96424..f783fe418 100644 --- a/test/e2e/push_test.go +++ b/test/e2e/push_test.go @@ -35,20 +35,16 @@ var _ = Describe("Podman push", func() { session = podmanTest.Podman([]string{"rmi", ALPINE}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Not(Equal(0))) + Expect(session.ExitCode()).To(Equal(0)) session = podmanTest.Podman([]string{"rmi", "busybox:test"}) session.WaitWithDefaultTimeout() - Expect(session.ExitCode()).To(Not(Equal(0))) - - session = podmanTest.Podman([]string{"rmi", "-f", "busybox:test"}) - session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) }) // push to oci-archive, docker-archive, and dir are tested in pull_test.go - It("podman push to containers/storage", func() { + It("podman push to dir", func() { session := podmanTest.Podman([]string{"push", "--remove-signatures", ALPINE, "dir:/tmp/busybox"}) session.WaitWithDefaultTimeout() Expect(session.ExitCode()).To(Equal(0)) diff --git a/test/e2e/rmi_test.go b/test/e2e/rmi_test.go index 04346f5ac..67ccc1b95 100644 --- a/test/e2e/rmi_test.go +++ b/test/e2e/rmi_test.go @@ -67,4 +67,40 @@ var _ = Describe("Podman rmi", func() { }) + It("podman rmi tagged image", func() { + setup := podmanTest.Podman([]string{"images", "-q", ALPINE}) + setup.WaitWithDefaultTimeout() + Expect(setup.ExitCode()).To(Equal(0)) + + session := podmanTest.Podman([]string{"tag", "alpine", "foo:bar", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + result := podmanTest.Podman([]string{"images", "-q", "foo"}) + result.WaitWithDefaultTimeout() + Expect(result.ExitCode()).To(Equal(0)) + + Expect(result.LineInOuputContains(setup.OutputToString())).To(BeTrue()) + }) + + It("podman rmi image with tags by ID cannot be done without force", func() { + setup := podmanTest.Podman([]string{"images", "-q", ALPINE}) + setup.WaitWithDefaultTimeout() + Expect(setup.ExitCode()).To(Equal(0)) + alpineId := setup.OutputToString() + + session := podmanTest.Podman([]string{"tag", "alpine", "foo:bar", "foo"}) + session.WaitWithDefaultTimeout() + Expect(session.ExitCode()).To(Equal(0)) + + // Trying without --force should fail + result := podmanTest.Podman([]string{"rmi", alpineId}) + result.WaitWithDefaultTimeout() + Expect(result.ExitCode()).ToNot(Equal(0)) + + // With --force it should work + resultForce := podmanTest.Podman([]string{"rmi", "-f", alpineId}) + resultForce.WaitWithDefaultTimeout() + Expect(resultForce.ExitCode()).To(Equal(0)) + }) }) |