podman untag: error if tag doesn't exist

Throw an error if a specified tag does not exist.  Also make sure that
the user input is normalized as we already do for `podman tag`.

To prevent regressions, add a set of end-to-end and systemd tests.

Last but not least, update the docs and add bash completions.

Signed-off-by: Valentin Rothberg <rothberg@redhat.com>

7 files changed, 127 insertions, 34 deletions

diff --git a/completions/bash/podman b/completions/bash/podman
index cb4e86156..c0d9560ed 100644
--- a/completions/bash/podman
+++ b/completions/bash/podman
@@ -1572,6 +1572,11 @@ _podman_image_tag() {
      _podman_tag
 }
 
+
+_podman_image_untag() {
+     _podman_untag
+}
+
 _podman_image() {
     local boolean_options="
 	--help
@@ -1593,6 +1598,7 @@ _podman_image() {
 	 sign
 	 tag
 	 trust
+	 untag
      "
      local aliases="
 	 list
@@ -2460,6 +2466,23 @@ _podman_tag() {
     esac
 }
 
+_podman_untag() {
+    local options_with_args="
+    "
+    local boolean_options="
+	--help
+	-h
+    "
+    case "$cur" in
+	-*)
+	    COMPREPLY=($(compgen -W "$boolean_options $options_with_args" -- "$cur"))
+	    ;;
+	*)
+	    __podman_complete_images
+	    ;;
+    esac
+}
+
 __podman_top_descriptors() {
 	podman top --list-descriptors
 }
@@ -3588,6 +3611,7 @@ _podman_podman() {
     umount
     unmount
     unpause
+    untag
     varlink
     version
     volume
diff --git a/docs/source/markdown/podman-untag.1.md b/docs/source/markdown/podman-untag.1.md
index 3a54283d6..c83a0544c 100644
--- a/docs/source/markdown/podman-untag.1.md
+++ b/docs/source/markdown/podman-untag.1.md
@@ -4,14 +4,12 @@
 podman\-untag - Removes one or more names from a locally-stored image
 
 ## SYNOPSIS
-**podman untag** *image*[:*tag*] [*target-names*[:*tag*]] [*options*]
+**podman untag** [*options*] *image* [*name*[:*tag*]...]
 
-**podman image untag** *image*[:*tag*] [target-names[:*tag*]] [*options*]
+**podman image untag** [*options*] *image* [*name*[:*tag*]...]
 
 ## DESCRIPTION
-Removes one or all names of an image. A name refers to the entire image name,
-including the optional *tag* after the `:`. If no target image names are
-specified, `untag` will remove all tags for the image at once.
+Remove one or more names from an image in the local storage.  The image can be referred to by ID or reference.  If a no name is specified, all names are removed the image.  If a specified name is a short name and does not include a registry `localhost/` will be prefixed (e.g., `fedora` -> `localhost/fedora`). If a specified name does not include a tag `:latest` will be appended (e.g., `localhost/fedora` -> `localhost/fedora:latest`).
 
 ## OPTIONS
 
diff --git a/libpod/define/errors.go b/libpod/define/errors.go
index e0c9811fe..98dc603d1 100644
--- a/libpod/define/errors.go
+++ b/libpod/define/errors.go
@@ -17,6 +17,9 @@ var (
 	// ErrNoSuchImage indicates the requested image does not exist
 	ErrNoSuchImage = image.ErrNoSuchImage
 
+	// ErrNoSuchTag indicates the requested image tag does not exist
+	ErrNoSuchTag = image.ErrNoSuchTag
+
 	// ErrNoSuchVolume indicates the requested volume does not exist
 	ErrNoSuchVolume = errors.New("no such volume")
 
diff --git a/libpod/image/errors.go b/libpod/image/errors.go
index 4088946cb..ddbf7be4b 100644
--- a/libpod/image/errors.go
+++ b/libpod/image/errors.go
@@ -12,4 +12,6 @@ var (
 	ErrNoSuchPod = errors.New("no such pod")
 	// ErrNoSuchImage indicates the requested image does not exist
 	ErrNoSuchImage = errors.New("no such image")
+	// ErrNoSuchTag indicates the requested image tag does not exist
+	ErrNoSuchTag = errors.New("no such tag")
 )
diff --git a/libpod/image/image.go b/libpod/image/image.go
index d81f7e911..83e7467e9 100644
--- a/libpod/image/image.go
+++ b/libpod/image/image.go
@@ -559,15 +559,24 @@ func (i *Image) TagImage(tag string) error {
 	return nil
 }
 
-// UntagImage removes a tag from the given image
+// UntagImage removes the specified tag from the image.
+// If the tag does not exist, ErrNoSuchTag is returned.
 func (i *Image) UntagImage(tag string) error {
 	if err := i.reloadImage(); err != nil {
 		return err
 	}
+
+	// Normalize the tag as we do with TagImage.
+	ref, err := NormalizedTag(tag)
+	if err != nil {
+		return err
+	}
+	tag = ref.String()
+
 	var newTags []string
 	tags := i.Names()
 	if !util.StringInSlice(tag, tags) {
-		return nil
+		return errors.Wrapf(ErrNoSuchTag, "%q", tag)
 	}
 	for _, t := range tags {
 		if tag != t {
diff --git a/test/e2e/untag_test.go b/test/e2e/untag_test.go
index dc1a6208e..8a1c8091d 100644
--- a/test/e2e/untag_test.go
+++ b/test/e2e/untag_test.go
@@ -23,13 +23,6 @@ var _ = Describe("Podman untag", func() {
 		podmanTest = PodmanTestCreate(tempdir)
 		podmanTest.Setup()
 		podmanTest.RestoreAllArtifacts()
-
-		for _, tag := range []string{"test", "foo", "bar"} {
-			session := podmanTest.PodmanNoCache([]string{"tag", ALPINE, tag})
-			session.WaitWithDefaultTimeout()
-			Expect(session.ExitCode()).To(Equal(0))
-		}
-
 	})
 
 	AfterEach(func() {
@@ -40,34 +33,63 @@ var _ = Describe("Podman untag", func() {
 	})
 
 	It("podman untag all", func() {
-		session := podmanTest.PodmanNoCache([]string{"untag", ALPINE})
+		tags := []string{ALPINE, "registry.com/foo:bar", "localhost/foo:bar"}
+
+		cmd := []string{"tag"}
+		cmd = append(cmd, tags...)
+		session := podmanTest.PodmanNoCache(cmd)
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		results := podmanTest.PodmanNoCache([]string{"images", ALPINE})
-		results.WaitWithDefaultTimeout()
-		Expect(results.ExitCode()).To(Equal(0))
-		Expect(results.OutputToStringArray()).To(HaveLen(1))
-	})
+		// Make sure that all tags exists.
+		for _, t := range tags {
+			session = podmanTest.PodmanNoCache([]string{"image", "exists", t})
+			session.WaitWithDefaultTimeout()
+			Expect(session.ExitCode()).To(Equal(0))
+		}
 
-	It("podman untag single", func() {
-		session := podmanTest.PodmanNoCache([]string{"untag", ALPINE, "localhost/test:latest"})
+		// No arguments -> remove all tags.
+		session = podmanTest.PodmanNoCache([]string{"untag", ALPINE})
 		session.WaitWithDefaultTimeout()
 		Expect(session.ExitCode()).To(Equal(0))
 
-		results := podmanTest.PodmanNoCache([]string{"images"})
-		results.WaitWithDefaultTimeout()
-		Expect(results.ExitCode()).To(Equal(0))
-		Expect(results.OutputToStringArray()).To(HaveLen(6))
-		Expect(results.LineInOuputStartsWith("docker.io/library/alpine")).To(BeTrue())
-		Expect(results.LineInOuputStartsWith("localhost/foo")).To(BeTrue())
-		Expect(results.LineInOuputStartsWith("localhost/bar")).To(BeTrue())
-		Expect(results.LineInOuputStartsWith("localhost/test")).To(BeFalse())
+		// Make sure that none of tags exists anymore.
+		for _, t := range tags {
+			session = podmanTest.PodmanNoCache([]string{"image", "exists", t})
+			session.WaitWithDefaultTimeout()
+			Expect(session.ExitCode()).To(Equal(1))
+		}
 	})
 
-	It("podman untag not enough arguments", func() {
-		session := podmanTest.PodmanNoCache([]string{"untag"})
-		session.WaitWithDefaultTimeout()
-		Expect(session.ExitCode()).NotTo(Equal(0))
+	It("podman tag/untag - tag normalization", func() {
+		tests := []struct {
+			tag, normalized string
+		}{
+			{"registry.com/image:latest", "registry.com/image:latest"},
+			{"registry.com/image", "registry.com/image:latest"},
+			{"image:latest", "localhost/image:latest"},
+			{"image", "localhost/image:latest"},
+		}
+
+		// Make sure that the user input is normalized correctly for
+		// `podman tag` and `podman untag`.
+		for _, tt := range tests {
+			session := podmanTest.PodmanNoCache([]string{"tag", ALPINE, tt.tag})
+			session.WaitWithDefaultTimeout()
+			Expect(session.ExitCode()).To(Equal(0))
+
+			session = podmanTest.PodmanNoCache([]string{"image", "exists", tt.normalized})
+			session.WaitWithDefaultTimeout()
+			Expect(session.ExitCode()).To(Equal(0))
+
+			session = podmanTest.PodmanNoCache([]string{"untag", ALPINE, tt.tag})
+			session.WaitWithDefaultTimeout()
+			Expect(session.ExitCode()).To(Equal(0))
+
+			session = podmanTest.PodmanNoCache([]string{"image", "exists", tt.normalized})
+			session.WaitWithDefaultTimeout()
+			Expect(session.ExitCode()).To(Equal(1))
+		}
 	})
+
 })
diff --git a/test/system/020-tag.bats b/test/system/020-tag.bats
new file mode 100644
index 000000000..7593ad68f
--- /dev/null
+++ b/test/system/020-tag.bats
@@ -0,0 +1,35 @@
+#!/usr/bin/env bats
+
+load helpers
+
+# helper function for "podman tag/untag" test
+function _tag_and_check() {
+    local tag_as="$1"
+    local check_as="$2"
+
+    run_podman tag $IMAGE $tag_as
+    run_podman image exists $check_as
+    run_podman untag $IMAGE $check_as
+    run_podman 1 image exists $check_as
+}
+
+@test "podman tag/untag" {
+	# Test a fully-qualified image reference.
+	_tag_and_check registry.com/image:latest registry.com/image:latest
+
+	# Test a reference without tag and make sure ":latest" is appended.
+	_tag_and_check registry.com/image registry.com/image:latest
+
+	# Test a tagged short image and make sure "localhost/" is prepended.
+	_tag_and_check image:latest localhost/image:latest
+
+	# Test a short image without tag and make sure "localhost/" is
+	# prepended and ":latest" is appended.
+	_tag_and_check image localhost/image:latest
+
+	# Test error case.
+	run_podman 125 untag $IMAGE registry.com/foo:bar
+	is "$output" "Error: \"registry.com/foo:bar\": no such tag"
+}
+
+# vim: filetype=sh