From 3d2537eb848b3e68d43187fa8aabe586e9d2f117 Mon Sep 17 00:00:00 2001
From: Xian Chaobo <xianchaobo@huawei.com>
Date: Mon, 27 Apr 2015 08:20:55 -0400
Subject: [PATCH] move check into cluster and rename func

Signed-off-by: Xian Chaobo <xianchaobo@huawei.com>
---
 api/handlers.go           | 11 +++--------
 api/router.go             |  2 +-
 cluster/cluster.go        |  4 ++--
 cluster/engine.go         |  4 ++--
 cluster/swarm/cluster.go  | 11 ++++++++---
 test/integration/api.bats | 31 +++++++++++++++++++++----------
 6 files changed, 37 insertions(+), 26 deletions(-)

diff --git a/api/handlers.go b/api/handlers.go
index 082ffffe9b..1a80b13d29 100644
--- a/api/handlers.go
+++ b/api/handlers.go
@@ -490,7 +490,7 @@ func postCommit(c *context, w http.ResponseWriter, r *http.Request) {
 }
 
 // POST /containers/{name:.*}/rename
-func postRename(c *context, w http.ResponseWriter, r *http.Request) {
+func postRenameContainer(c *context, w http.ResponseWriter, r *http.Request) {
 	container, err := getContainerFromVars(c, mux.Vars(r))
 	if err != nil {
 		httpError(w, err.Error(), http.StatusNotFound)
@@ -502,15 +502,10 @@ func postRename(c *context, w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	// check new name whether avaliable
 	newName := r.Form.Get("name")
-	if conflictContainer := c.cluster.Container(newName); conflictContainer != nil {
-		httpError(w, fmt.Sprintf("Conflict, The new name %s is already assigned to %s. ", newName, conflictContainer.Id), http.StatusConflict)
-		return
-	}
 
-	// call cluster rename
-	err = c.cluster.Rename(container, newName)
+	// call cluster rename container
+	err = c.cluster.RenameContainer(container, newName)
 	if err != nil {
 		httpError(w, err.Error(), http.StatusInternalServerError)
 	}
diff --git a/api/router.go b/api/router.go
index 7038bdb212..50580f5672 100644
--- a/api/router.go
+++ b/api/router.go
@@ -55,7 +55,7 @@ var routes = map[string]map[string]handler{
 		"/containers/{name:.*}/kill":    proxyContainer,
 		"/containers/{name:.*}/pause":   proxyContainer,
 		"/containers/{name:.*}/unpause": proxyContainer,
-		"/containers/{name:.*}/rename":  postRename,
+		"/containers/{name:.*}/rename":  postRenameContainer,
 		"/containers/{name:.*}/restart": proxyContainer,
 		"/containers/{name:.*}/start":   proxyContainer,
 		"/containers/{name:.*}/stop":    proxyContainer,
diff --git a/cluster/cluster.go b/cluster/cluster.go
index cd64ef66b9..3edddbf34c 100644
--- a/cluster/cluster.go
+++ b/cluster/cluster.go
@@ -44,6 +44,6 @@ type Cluster interface {
 	// Return a random engine
 	RANDOMENGINE() (*Engine, error)
 
-	// Rename a container
-	Rename(container *Container, newName string) error
+	// RenameContainer rename a container
+	RenameContainer(container *Container, newName string) error
 }
diff --git a/cluster/engine.go b/cluster/engine.go
index ad22779ec1..1a5aa898eb 100644
--- a/cluster/engine.go
+++ b/cluster/engine.go
@@ -556,8 +556,8 @@ func (e *Engine) cleanupContainers() {
 	e.Unlock()
 }
 
-// Rename a container
-func (e *Engine) Rename(container *Container, newName string) error {
+// RenameContainer rename a container
+func (e *Engine) RenameContainer(container *Container, newName string) error {
 	// send rename request
 	err := e.client.RenameContainer(container.Id, newName)
 	if err != nil {
diff --git a/cluster/swarm/cluster.go b/cluster/swarm/cluster.go
index 218f95db32..78461cc88c 100644
--- a/cluster/swarm/cluster.go
+++ b/cluster/swarm/cluster.go
@@ -334,13 +334,18 @@ func (c *Cluster) RANDOMENGINE() (*cluster.Engine, error) {
 	return nil, nil
 }
 
-// Rename a container
-func (c *Cluster) Rename(container *cluster.Container, newName string) error {
+// RenameContainer rename a container
+func (c *Cluster) RenameContainer(container *cluster.Container, newName string) error {
 	c.RLock()
 	defer c.RUnlock()
 
+	// check new name whether avaliable
+	if conflictContainer := c.Container(newName); conflictContainer != nil {
+		return fmt.Errorf("Error when allocating new name: Conflict. The name %q is already in use by container %s. You have to delete (or rename) that container to be able to reuse that name.", newName, conflictContainer.Id)
+	}
+
 	// call engine rename
-	err := container.Engine.Rename(container, newName)
+	err := container.Engine.RenameContainer(container, newName)
 	if err != nil {
 		return err
 	}
diff --git a/test/integration/api.bats b/test/integration/api.bats
index 30c94c7e99..fe5ae52da2 100644
--- a/test/integration/api.bats
+++ b/test/integration/api.bats
@@ -359,20 +359,31 @@ function teardown() {
 	run docker_swarm run -d --name test_container busybox sleep 500
 	[ "$status" -eq 0 ]
 
-	# make sure container exist
-	run docker_swarm ps -l
-	[ "${#lines[@]}" -eq 2 ]
-	[[ "${lines[*]}" == *"test_container"* ]]
-	[[ "${lines[*]}" != *"rename_container"* ]]
+	run docker_swarm run -d --name another_container busybox sleep 500
+	[ "$status" -eq 0 ]
 
-	# rename container
+	# make sure container exist
+	run docker_swarm ps -a
+	[ "${#lines[@]}" -eq 3 ]
+	[[ "${output}" == *"test_container"* ]]
+	[[ "${output}" == *"another_container"* ]]
+	[[ "${output}" != *"rename_container"* ]]
+
+	# rename container, conflict and fail
+	run docker_swarm rename test_container another_container
+	[ "$status" -ne 0 ]
+	[[ "${output}" == *"Error when allocating new name: Conflict."* ]]
+
+	# rename container, sucessful
 	run docker_swarm rename test_container rename_container
 	[ "$status" -eq 0 ]
 
-	# verify after rename 
-	run docker_swarm ps -l
-	[ "${#lines[@]}" -eq 2 ]
-	[[ "${lines[*]}" == *"rename_container"* ]]
+	# verify after, rename 
+	run docker_swarm ps -a
+	[ "${#lines[@]}" -eq 3 ]
+	[[ "${output}" == *"rename_container"* ]]
+	[[ "${output}" == *"another_container"* ]]
+	[[ "${output}" != *"test_container"* ]]
 }
 
 @test "docker restart" {