Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix run docker rename <container-id> new_name concurrently, the container will have multi names #33940

Merged
merged 1 commit into from
Jul 7, 2017

Conversation

yaocaigen
Copy link
Contributor

@yaocaigen yaocaigen commented Jul 4, 2017

When run docker rename <container-id> new_name concurrently, every operation will release old
container name. So container will have multi new names reserve in nameIndex.

- What I did
Fix a bug:
Description:

  1. start a container
    docker run busybox
  2. run docker rename command concurrently
    #!/bin/bash
    for((i=0;i<10;i++))
    do
    docker rename efbf7ff32533 name${i} &
    done
    
  3. container has multi names
    # docker ps --no-trunc -a
    CONTAINER ID                                                       IMAGE           COMMAND             CREATED             STATUS                       PORTS               NAMES                                                           
    efbf7ff32533151babb878769e47fc5c33cd85b2afbfa52ef29f4cb16929e64c   busybox         "/bin/sh"           7 minutes ago       Exited (0) 7 minutes ago                         name2,name4,name1,name8,name5,name6,name3,name7,name9                                     
    
  4. run docker ps and docker inspect, container name is different
    # docker ps --format "table {{.Names}}" -a
    NAMES
    name2
    # docker inspect --format {{.Name}} name2
    /name9
    

- How I did it
Every docker rename operation should release newest container name, so container should get locked before get container name.
- How to verify it

  1. start a container
  2. run docker rename command concurrently
  3. run docker ps and docker inspect, container name is the same

Signed-off-by: Yang Pengfei yangpengfei4@huawei.com

@AkihiroSuda
Copy link
Member

cc @cpuguy83 @fabiokung PTAL?

Copy link
Member

@cpuguy83 cpuguy83 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@yaocaigen yaocaigen force-pushed the fix_container_multi_names branch 2 times, most recently from 0273b4b to cc23406 Compare July 5, 2017 00:31
…tainer will have multi names

When run `docker rename <container-id> new_name` concurrently, every operation will release
container's old name. So container will have multi new names reserve in nameIndex.

Signed-off-by: Yang Pengfei <yangpengfei4@huawei.com>
@yaocaigen
Copy link
Contributor Author

@fabiokung PTAL

@fabiokung
Copy link
Contributor

LGTM, thanks!

Copy link
Member

@AkihiroSuda AkihiroSuda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@AkihiroSuda AkihiroSuda merged commit 8e3555f into moby:master Jul 7, 2017
@yaocaigen yaocaigen deleted the fix_container_multi_names branch July 7, 2017 04:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants