diff --git a/api/handlers.go b/api/handlers.go index 95e6a340b6..39ab133980 100644 --- a/api/handlers.go +++ b/api/handlers.go @@ -275,14 +275,13 @@ func postContainersCreate(c *context, w http.ResponseWriter, r *http.Request) { return } - if container := c.cluster.Container(name); container != nil { - httpError(w, fmt.Sprintf("Conflict, The name %s is already assigned to %s. You have to delete (or rename) that container to be able to assign %s to a container again.", name, container.Id, name), http.StatusConflict) - return - } - container, err := c.cluster.CreateContainer(cluster.BuildContainerConfig(config), name) if err != nil { - httpError(w, err.Error(), http.StatusInternalServerError) + if strings.HasPrefix(err.Error(), "Conflict") { + httpError(w, err.Error(), http.StatusConflict) + } else { + httpError(w, err.Error(), http.StatusInternalServerError) + } return } @@ -606,12 +605,12 @@ func postRenameContainer(c *context, w http.ResponseWriter, r *http.Request) { return } - newName := r.Form.Get("name") - - // call cluster rename container - err = c.cluster.RenameContainer(container, newName) - if err != nil { - httpError(w, err.Error(), http.StatusInternalServerError) + if err = c.cluster.RenameContainer(container, r.Form.Get("name")); err != nil { + if strings.HasPrefix(err.Error(), "Conflict") { + httpError(w, err.Error(), http.StatusConflict) + } else { + httpError(w, err.Error(), http.StatusInternalServerError) + } } } diff --git a/cluster/swarm/cluster.go b/cluster/swarm/cluster.go index b2934029d5..5f4f2254dc 100644 --- a/cluster/swarm/cluster.go +++ b/cluster/swarm/cluster.go @@ -85,6 +85,11 @@ func (c *Cluster) CreateContainer(config *cluster.ContainerConfig, name string) c.scheduler.Lock() defer c.scheduler.Unlock() + // check new name whether avaliable + if cID := c.getIDFromName(name); cID != "" { + return nil, fmt.Errorf("Conflict, The name %s is already assigned to %s. You have to delete (or rename) that container to be able to assign %s to a container again.", name, cID, name) + } + n, err := c.scheduler.SelectNodeForContainer(c.listNodes(), config) if err != nil { return nil, err @@ -304,6 +309,26 @@ func (c *Cluster) Containers() []*cluster.Container { return out } +func (c *Cluster) getIDFromName(name string) string { + // Abort immediately if the name is empty. + if len(name) == 0 { + return "" + } + + c.RLock() + defer c.RUnlock() + for _, n := range c.engines { + for _, c := range n.Containers() { + for _, cname := range c.Names { + if cname == name || cname == "/"+name { + return c.Id + } + } + } + } + return "" +} + // Container returns the container with IDOrName in the cluster func (c *Cluster) Container(IDOrName string) *cluster.Container { // Abort immediately if the name is empty. @@ -392,8 +417,8 @@ func (c *Cluster) RenameContainer(container *cluster.Container, newName string) 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) + if cID := c.getIDFromName(newName); cID != "" { + return fmt.Errorf("Conflict, The name %s is already assigned to %s. You have to delete (or rename) that container to be able to assign %s to a container again.", newName, cID, newName) } // call engine rename diff --git a/test/integration/api/create.bats b/test/integration/api/create.bats index 83136d70b9..f5eff61ee4 100644 --- a/test/integration/api/create.bats +++ b/test/integration/api/create.bats @@ -16,11 +16,21 @@ function teardown() { [ "${#lines[@]}" -eq 0 ] # create - run docker_swarm create --name test_container busybox sleep 1000 - [ "$status" -eq 0 ] + docker_swarm create --name test_container busybox sleep 1000 # verify, created container exists run docker_swarm ps -l [ "${#lines[@]}" -eq 2 ] [[ "${lines[1]}" == *"test_container"* ]] } + +@test "docker create conflict" { + start_docker_with_busybox 1 + swarm_manage + + id=$(docker_swarm create busybox) + prefix=$(printf "$id" | cut -c 1-4) + docker_swarm create --name "$prefix" busybox + docker_swarm create --name "$id" busybox + docker_swarm create --name test busybox +} \ No newline at end of file diff --git a/test/integration/api/rename.bats b/test/integration/api/rename.bats index 2bcc5dfcaa..225bc90747 100644 --- a/test/integration/api/rename.bats +++ b/test/integration/api/rename.bats @@ -11,11 +11,9 @@ function teardown() { start_docker_with_busybox 2 swarm_manage - run docker_swarm run -d --name test_container busybox sleep 500 - [ "$status" -eq 0 ] + docker_swarm run -d --name test_container busybox sleep 500 - run docker_swarm run -d --name another_container busybox sleep 500 - [ "$status" -eq 0 ] + docker_swarm run -d --name another_container busybox sleep 500 # make sure container exist run docker_swarm ps -a @@ -27,11 +25,10 @@ function teardown() { # rename container, conflict and fail run docker_swarm rename test_container another_container [ "$status" -ne 0 ] - [[ "${output}" == *"Error when allocating new name: Conflict."* ]] + [[ "${output}" == *"Conflict,"* ]] # rename container, sucessful - run docker_swarm rename test_container rename_container - [ "$status" -eq 0 ] + docker_swarm rename test_container rename_container # verify after, rename run docker_swarm ps -a @@ -40,3 +37,15 @@ function teardown() { [[ "${output}" == *"another_container"* ]] [[ "${output}" != *"test_container"* ]] } + +@test "docker rename conflict" { + start_docker_with_busybox 1 + swarm_manage + + id=$(docker_swarm create busybox) + prefix=$(printf "$id" | cut -c 1-4) + docker_swarm create --name test busybox + docker_swarm rename test "$prefix" + run docker_swarm rename test "$id" + [ "$status" -eq 1 ] +} \ No newline at end of file