Merge pull request #815 from jimmyxian/rmi_by_name

Fix image match

api/handlers.go

@@ -62,6 +62,20 @@ func getVersion(c *context, w http.ResponseWriter, r *http.Request) {
 	json.NewEncoder(w).Encode(version)
 }
 
+// GET /images/{name:.*}/get
+func getImage(c *context, w http.ResponseWriter, r *http.Request) {
+	name := mux.Vars(r)["name"]
+
+	for _, image := range c.cluster.Images() {
+		if len(strings.SplitN(name, ":", 2)) == 2 && image.Match(name, true) ||
+			len(strings.SplitN(name, ":", 2)) == 1 && image.Match(name, false) {
+			proxy(c.tlsConfig, image.Engine.Addr, w, r)
+			return
+		}
+	}
+	httpError(w, fmt.Sprintf("No such image: %s", name), http.StatusNotFound)
+}
+
 // GET /images/get
 func getImages(c *context, w http.ResponseWriter, r *http.Request) {
 	if err := r.ParseForm(); err != nil {
@@ -83,7 +97,8 @@ func getImages(c *context, w http.ResponseWriter, r *http.Request) {
 		// Count how many images we need it has.
 		for _, name := range names {
 			for _, image := range images {
-				if image.Match(name) {
+				if len(strings.SplitN(name, ":", 2)) == 2 && image.Match(name, true) ||
+					len(strings.SplitN(name, ":", 2)) == 1 && image.Match(name, false) {
 					matchedImages = matchedImages + 1
 					break
 				}
@@ -477,36 +492,18 @@ func deleteImages(c *context, w http.ResponseWriter, r *http.Request) {
 	}
 	var name = mux.Vars(r)["name"]
 
-	matchedImages := []*cluster.Image{}
+	out, err := c.cluster.RemoveImages(name)
-	for _, image := range c.cluster.Images() {
+	if err != nil {
-		if image.Match(name) {
+		httpError(w, err.Error(), http.StatusInternalServerError)
-			matchedImages = append(matchedImages, image)
-		}
-	}
-
-	if len(matchedImages) == 0 {
-		httpError(w, fmt.Sprintf("No such image %s", name), http.StatusNotFound)
 		return
 	}
 
-	out := []*dockerclient.ImageDelete{}
+	if len(out) == 0 {
-	errs := []string{}
+		httpError(w, fmt.Sprintf("No such image %s", name), http.StatusNotFound)
-	for _, image := range matchedImages {
+		return
-		content, err := c.cluster.RemoveImage(image)
-		if err != nil {
-			errs = append(errs, fmt.Sprintf("%s: %s", image.Engine.Name, err.Error()))
-			continue
-		}
-		out = append(out, content...)
 	}
-
+	w.Header().Set("Content-Type", "application/json")
-	if len(errs) != 0 {
+	json.NewEncoder(NewWriteFlusher(w)).Encode(out)
-		httpError(w, strings.Join(errs, ""), http.StatusInternalServerError)
-	} else {
-		w.Header().Set("Content-Type", "application/json")
-		json.NewEncoder(NewWriteFlusher(w)).Encode(out)
-	}
-
 }
 
 // GET /_ping

api/router.go

@@ -29,7 +29,7 @@ var routes = map[string]map[string]handler{
 		"/images/viz":                     notImplementedHandler,
 		"/images/search":                  proxyRandom,
 		"/images/get":                     getImages,
-		"/images/{name:.*}/get":           proxyImage,
+		"/images/{name:.*}/get":           getImage,
 		"/images/{name:.*}/history":       proxyImage,
 		"/images/{name:.*}/json":          proxyImage,
 		"/containers/ps":                  getContainersJSON,

cluster/cluster.go

@@ -20,8 +20,8 @@ type Cluster interface {
 	// Return one image matching `IDOrName`
 	Image(IDOrName string) *Image
 
-	// Remove an image from the cluster
+	// Remove images from the cluster
-	RemoveImage(image *Image) ([]*dockerclient.ImageDelete, error)
+	RemoveImages(name string) ([]*dockerclient.ImageDelete, error)
 
 	// Return all containers
 	Containers() []*Container

cluster/engine.go

@@ -161,8 +161,8 @@ func (e *Engine) updateSpecs() error {
 }
 
 // RemoveImage deletes an image from the engine.
-func (e *Engine) RemoveImage(image *Image) ([]*dockerclient.ImageDelete, error) {
+func (e *Engine) RemoveImage(image *Image, name string) ([]*dockerclient.ImageDelete, error) {
-	return e.client.RemoveImage(image.Id)
+	return e.client.RemoveImage(name)
 }
 
 // RefreshImages refreshes the list of images on the engine.
@@ -480,7 +480,7 @@ func (e *Engine) Image(IDOrName string) *Image {
 	defer e.RUnlock()
 
 	for _, image := range e.images {
-		if image.Match(IDOrName) {
+		if image.Match(IDOrName, true) {
 			return image
 		}
 	}

cluster/image.go

@@ -14,15 +14,27 @@ type Image struct {
 }
 
 // Match is exported
-func (image *Image) Match(IDOrName string) bool {
+func (image *Image) Match(IDOrName string, matchTag bool) bool {
 	size := len(IDOrName)
 
 	if image.Id == IDOrName || (size > 2 && strings.HasPrefix(image.Id, IDOrName)) {
 		return true
 	}
+
+	name := IDOrName
+	if matchTag {
+		if len(strings.SplitN(IDOrName, ":", 2)) == 1 {
+			name = IDOrName + ":latest"
+		}
+	} else {
+		name = strings.SplitN(IDOrName, ":", 2)[0]
+	}
+
 	for _, repoTag := range image.RepoTags {
-		parts := strings.SplitN(repoTag, ":", 2)
+		if matchTag == false {
-		if repoTag == IDOrName || parts[0] == IDOrName {
+			repoTag = strings.SplitN(repoTag, ":", 2)[0]
+		}
+		if repoTag == name {
 			return true
 		}
 	}

cluster/image_test.go

@@ -12,13 +12,23 @@ func TestMatch(t *testing.T) {
 	img.Id = "378954456789"
 	img.RepoTags = []string{"name:latest"}
 
-	assert.True(t, img.Match("378954456789"))
+	assert.True(t, img.Match("378954456789", true))
-	assert.True(t, img.Match("3789"))
+	assert.True(t, img.Match("3789", true))
-	assert.True(t, img.Match("378"))
+	assert.True(t, img.Match("378", true))
-	assert.False(t, img.Match("37"))
+	assert.False(t, img.Match("37", true))
 
-	assert.True(t, img.Match("name:latest"))
+	assert.True(t, img.Match("name:latest", true))
-	assert.True(t, img.Match("name"))
+	assert.True(t, img.Match("name", true))
-	assert.False(t, img.Match("nam"))
+	assert.False(t, img.Match("nam", true))
-	assert.False(t, img.Match("na"))
+	assert.False(t, img.Match("na", true))
+
+	assert.True(t, img.Match("378954456789", false))
+	assert.True(t, img.Match("3789", false))
+	assert.True(t, img.Match("378", false))
+	assert.False(t, img.Match("37", false))
+
+	assert.True(t, img.Match("name:latest", false))
+	assert.True(t, img.Match("name", false))
+	assert.False(t, img.Match("nam", false))
+	assert.False(t, img.Match("na", false))
 }

cluster/swarm/cluster.go

@@ -269,11 +269,32 @@ func (c *Cluster) Image(IDOrName string) *cluster.Image {
 	return nil
 }
 
-// RemoveImage removes an image from the cluster
+// RemoveImages removes all the images that match `name` from the cluster
-func (c *Cluster) RemoveImage(image *cluster.Image) ([]*dockerclient.ImageDelete, error) {
+func (c *Cluster) RemoveImages(name string) ([]*dockerclient.ImageDelete, error) {
 	c.Lock()
 	defer c.Unlock()
-	return image.Engine.RemoveImage(image)
+
+	out := []*dockerclient.ImageDelete{}
+	errs := []string{}
+	var err error
+	for _, n := range c.engines {
+		for _, image := range n.Images() {
+			if image.Match(name, true) {
+				content, err := image.Engine.RemoveImage(image, name)
+				if err != nil {
+					errs = append(errs, fmt.Sprintf("%s: %s", image.Engine.Name, err.Error()))
+					continue
+				}
+				out = append(out, content...)
+			}
+		}
+	}
+
+	if len(errs) > 0 {
+		err = errors.New(strings.Join(errs, "\n"))
+	}
+
+	return out, err
 }
 
 // Pull is exported

test/integration/api/rmi.bats

@@ -56,3 +56,25 @@ function teardown() {
 	[ "$status" -ne 0 ]
 	[[ "${output}" == *"No such image"* ]]
 }
+
+@test "docker rmi without tag" {
+	start_docker_with_busybox 1
+	start_docker 1 
+	
+	docker -H ${HOSTS[0]} tag busybox:latest testimage:latest
+	swarm_manage
+
+	run docker_swarm images
+	[ "$status" -eq 0 ]
+	[[ "${output}" == *"busybox"* ]]
+	[[ "${output}" == *"testimage"* ]]
+
+	run docker_swarm rmi testimage
+	[ "$status" -eq 0 ]
+	[[ "${output}" == *"Untagged"* ]]
+
+	run docker_swarm images
+	[ "$status" -eq 0 ]
+	[[ "${output}" == *"busybox"* ]]
+	[[ "${output}" != *"testimage"* ]]
+}