GitHub PullRequestEvent at 2014-10-24 15:30:29
Repository: chef-docker
Actor: tduffield
Payload: {"action":"closed","number":206,"pull_request":{"url":"https://api.github.com/repos/bflad/chef-docker/pulls/206","id":20507632,"html_url":"https://github.com/bflad/chef-docker/pull/206","diff_url":"https://github.com/bflad/chef-docker/pull/206.diff","patch_url":"https://github.com/bflad/chef-docker/pull/206.patch","issue_url":"https://api.github.com/repos/bflad/chef-docker/issues/206","number":206,"state":"closed","locked":false,"title":"Multiple fixes to container name detection","user":{"login":"jperville","id":834791,"avatar_url":"https://avatars.githubusercontent.com/u/834791?v=2","gravatar_id":"","url":"https://api.github.com/users/jperville","html_url":"https://github.com/jperville","followers_url":"https://api.github.com/users/jperville/followers","following_url":"https://api.github.com/users/jperville/following{/other_user}","gists_url":"https://api.github.com/users/jperville/gists{/gist_id}","starred_url":"https://api.github.com/users/jperville/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jperville/subscriptions","organizations_url":"https://api.github.com/users/jperville/orgs","repos_url":"https://api.github.com/users/jperville/repos","events_url":"https://api.github.com/users/jperville/events{/privacy}","received_events_url":"https://api.github.com/users/jperville/received_events","type":"User","site_admin":false},"body":"# Summary\r\n\r\nThis PR fixes multiple issues with container names in the container provider; each of these issue has been encountered when playing with linked containers in #204. I have split each fix in a different changeset, feel free to cherry-pick (or merge everything if you like the PR).\r\n\r\n## Handle case when container has multiple names\r\n\r\nContainers may have multiple names assigned to them; even if the user only assigns one name to the container, the use of features such as 'links' may append technical names to the container. The different names are displayed in the `docker ps` output concatenated with a comma.\r\n\r\nExample:\r\n\r\n```\r\nvagrant@docker-ubuntu-1404:~$ docker ps | grep backend\r\nd605ff9bfbed        docker.local/sample-docker-app:latest   \"/sbin/my_init --ena   45 minutes ago      Up 45 minutes       0.0.0.0:6080->80/tcp     my-backend-app,my-test-app/link   \r\n```\r\n\r\nWhen a container has multiple names, the `new_resource.container_name` may not be found in the output of `docker ps` because of exact string comparison with the raw value of the NAMES column.\r\n\r\nacc8296916648a5187378200581fdaa2aabe8d0c should allow matching the container name with any of the different names of the container.\r\n\r\n## Made container matching more strict\r\n\r\nWhen loading the current_resource, the docker_container provider iterates over each container listed in `docker ps` output and stops when it finds a line that matches the new_resource. However, the method that does the matching (`container_matches?`) is too lax and will incorrectly select the wrong container with the following `docker ps` output:\r\n\r\n```\r\naad30ba67b24 docker.local/sample-docker-app:latest \"/sbin/my_init --ena About a minute ago Up About a minute 0.0.0.0:8080->80/tcp my-test-app\r\n26ba3c500199 docker.local/sample-docker-app:latest \"/sbin/my_init --ena 2 hours ago Up About an hour 0.0.0.0:6080->80/tcp my-backend-app,my-test-app/link\r\n```\r\n\r\nAssuming `new_resource.container_name = 'my-backend-name'`, the current version of `container_matches?` will match a container that has:\r\n - a different id\r\n - a different name\r\n - the same image\r\n - the same command\r\nsuch as `my-test-app` in the above example just because `my-test-app` is before `my-backend-app` in the `docker ps` output.\r\n\r\nAs a consequence, chef tried to recreate the `my-backend-app` container (with `docker run`) while the container already existed, and failed. This bug is very subtle because the incorrect loading of the current resource only shows up when log level is set at debug.\r\n\r\nMy fix in cac59af37714e53aa2786797580b5262cc824793 is to stop considering that being run from the same image with the same cmdline is enough to 'match' a container. @bflad any side effects you imagine by being stricter? Matching an incorrect container is worse than matching none IMO.\r\n\r\n## Filter out technical names\r\n\r\nWhen I was investigating #204, I found that the current_resource.container_name is initialized from the value of the raw NAMES colum in `docker ps` output.\r\n\r\nIn the case where `docker run --link` is used, it results in the `current_resource.container_name` being set to `my-backend-app,my-test-app/link` instead of `my-backend-app` which was the `container_name` that I have set in my recipe.\r\n\r\nIn 50b73cc7b21c5211a723f8cc6e132f8aec058eda I have decided to work around the issue by hiding the technical container names (such as `my-test-app/link`). This is not a full fix though, because if a container has multiple non-technical names the issue will surface again.","created_at":"2014-08-29T15:55:21Z","updated_at":"2014-10-24T15:30:29Z","closed_at":"2014-10-24T15:30:29Z","merged_at":null,"merge_commit_sha":"b7cf207792b4803067414d27d9800305b3de770b","assignee":null,"milestone":null,"commits_url":"https://api.github.com/repos/bflad/chef-docker/pulls/206/commits","review_comments_url":"https://api.github.com/repos/bflad/chef-docker/pulls/206/comments","review_comment_url":"https://api.github.com/repos/bflad/chef-docker/pulls/comments/{number}","comments_url":"https://api.github.com/repos/bflad/chef-docker/issues/206/comments","statuses_url":"https://api.github.com/repos/bflad/chef-docker/statuses/50b73cc7b21c5211a723f8cc6e132f8aec058eda","head":{"label":"jperville:fix-container-name-detection","ref":"fix-container-name-detection","sha":"50b73cc7b21c5211a723f8cc6e132f8aec058eda","user":{"login":"jperville","id":834791,"avatar_url":"https://avatars.githubusercontent.com/u/834791?v=2","gravatar_id":"","url":"https://api.github.com/users/jperville","html_url":"https://github.com/jperville","followers_url":"https://api.github.com/users/jperville/followers","following_url":"https://api.github.com/users/jperville/following{/other_user}","gists_url":"https://api.github.com/users/jperville/gists{/gist_id}","starred_url":"https://api.github.com/users/jperville/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jperville/subscriptions","organizations_url":"https://api.github.com/users/jperville/orgs","repos_url":"https://api.github.com/users/jperville/repos","events_url":"https://api.github.com/users/jperville/events{/privacy}","received_events_url":"https://api.github.com/users/jperville/received_events","type":"User","site_admin":false},"repo":{"id":20214556,"name":"chef-docker","full_name":"jperville/chef-docker","owner":{"login":"jperville","id":834791,"avatar_url":"https://avatars.githubusercontent.com/u/834791?v=2","gravatar_id":"","url":"https://api.github.com/users/jperville","html_url":"https://github.com/jperville","followers_url":"https://api.github.com/users/jperville/followers","following_url":"https://api.github.com/users/jperville/following{/other_user}","gists_url":"https://api.github.com/users/jperville/gists{/gist_id}","starred_url":"https://api.github.com/users/jperville/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jperville/subscriptions","organizations_url":"https://api.github.com/users/jperville/orgs","repos_url":"https://api.github.com/users/jperville/repos","events_url":"https://api.github.com/users/jperville/events{/privacy}","received_events_url":"https://api.github.com/users/jperville/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/jperville/chef-docker","description":"Chef Cookbook for Docker","fork":true,"url":"https://api.github.com/repos/jperville/chef-docker","forks_url":"https://api.github.com/repos/jperville/chef-docker/forks","keys_url":"https://api.github.com/repos/jperville/chef-docker/keys{/key_id}","collaborators_url":"https://api.github.com/repos/jperville/chef-docker/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/jperville/chef-docker/teams","hooks_url":"https://api.github.com/repos/jperville/chef-docker/hooks","issue_events_url":"https://api.github.com/repos/jperville/chef-docker/issues/events{/number}","events_url":"https://api.github.com/repos/jperville/chef-docker/events","assignees_url":"https://api.github.com/repos/jperville/chef-docker/assignees{/user}","branches_url":"https://api.github.com/repos/jperville/chef-docker/branches{/branch}","tags_url":"https://api.github.com/repos/jperville/chef-docker/tags","blobs_url":"https://api.github.com/repos/jperville/chef-docker/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/jperville/chef-docker/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/jperville/chef-docker/git/refs{/sha}","trees_url":"https://api.github.com/repos/jperville/chef-docker/git/trees{/sha}","statuses_url":"https://api.github.com/repos/jperville/chef-docker/statuses/{sha}","languages_url":"https://api.github.com/repos/jperville/chef-docker/languages","stargazers_url":"https://api.github.com/repos/jperville/chef-docker/stargazers","contributors_url":"https://api.github.com/repos/jperville/chef-docker/contributors","subscribers_url":"https://api.github.com/repos/jperville/chef-docker/subscribers","subscription_url":"https://api.github.com/repos/jperville/chef-docker/subscription","commits_url":"https://api.github.com/repos/jperville/chef-docker/commits{/sha}","git_commits_url":"https://api.github.com/repos/jperville/chef-docker/git/commits{/sha}","comments_url":"https://api.github.com/repos/jperville/chef-docker/comments{/number}","issue_comment_url":"https://api.github.com/repos/jperville/chef-docker/issues/comments/{number}","contents_url":"https://api.github.com/repos/jperville/chef-docker/contents/{+path}","compare_url":"https://api.github.com/repos/jperville/chef-docker/compare/{base}...{head}","merges_url":"https://api.github.com/repos/jperville/chef-docker/merges","archive_url":"https://api.github.com/repos/jperville/chef-docker/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/jperville/chef-docker/downloads","issues_url":"https://api.github.com/repos/jperville/chef-docker/issues{/number}","pulls_url":"https://api.github.com/repos/jperville/chef-docker/pulls{/number}","milestones_url":"https://api.github.com/repos/jperville/chef-docker/milestones{/number}","notifications_url":"https://api.github.com/repos/jperville/chef-docker/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/jperville/chef-docker/labels{/name}","releases_url":"https://api.github.com/repos/jperville/chef-docker/releases{/id}","created_at":"2014-05-27T09:33:44Z","updated_at":"2014-06-26T12:32:32Z","pushed_at":"2014-08-29T16:16:33Z","git_url":"git://github.com/jperville/chef-docker.git","ssh_url":"git@github.com:jperville/chef-docker.git","clone_url":"https://github.com/jperville/chef-docker.git","svn_url":"https://github.com/jperville/chef-docker","homepage":null,"size":845,"stargazers_count":0,"watchers_count":0,"language":"Ruby","has_issues":false,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"bflad:master","ref":"master","sha":"8339a64aba7618d30e00db9721bb8e3faf453511","user":{"login":"bflad","id":189114,"avatar_url":"https://avatars.githubusercontent.com/u/189114?v=2","gravatar_id":"","url":"https://api.github.com/users/bflad","html_url":"https://github.com/bflad","followers_url":"https://api.github.com/users/bflad/followers","following_url":"https://api.github.com/users/bflad/following{/other_user}","gists_url":"https://api.github.com/users/bflad/gists{/gist_id}","starred_url":"https://api.github.com/users/bflad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bflad/subscriptions","organizations_url":"https://api.github.com/users/bflad/orgs","repos_url":"https://api.github.com/users/bflad/repos","events_url":"https://api.github.com/users/bflad/events{/privacy}","received_events_url":"https://api.github.com/users/bflad/received_events","type":"User","site_admin":false},"repo":{"id":9992805,"name":"chef-docker","full_name":"bflad/chef-docker","owner":{"login":"bflad","id":189114,"avatar_url":"https://avatars.githubusercontent.com/u/189114?v=2","gravatar_id":"","url":"https://api.github.com/users/bflad","html_url":"https://github.com/bflad","followers_url":"https://api.github.com/users/bflad/followers","following_url":"https://api.github.com/users/bflad/following{/other_user}","gists_url":"https://api.github.com/users/bflad/gists{/gist_id}","starred_url":"https://api.github.com/users/bflad/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bflad/subscriptions","organizations_url":"https://api.github.com/users/bflad/orgs","repos_url":"https://api.github.com/users/bflad/repos","events_url":"https://api.github.com/users/bflad/events{/privacy}","received_events_url":"https://api.github.com/users/bflad/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/bflad/chef-docker","description":"Chef Cookbook for Docker","fork":false,"url":"https://api.github.com/repos/bflad/chef-docker","forks_url":"https://api.github.com/repos/bflad/chef-docker/forks","keys_url":"https://api.github.com/repos/bflad/chef-docker/keys{/key_id}","collaborators_url":"https://api.github.com/repos/bflad/chef-docker/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/bflad/chef-docker/teams","hooks_url":"https://api.github.com/repos/bflad/chef-docker/hooks","issue_events_url":"https://api.github.com/repos/bflad/chef-docker/issues/events{/number}","events_url":"https://api.github.com/repos/bflad/chef-docker/events","assignees_url":"https://api.github.com/repos/bflad/chef-docker/assignees{/user}","branches_url":"https://api.github.com/repos/bflad/chef-docker/branches{/branch}","tags_url":"https://api.github.com/repos/bflad/chef-docker/tags","blobs_url":"https://api.github.com/repos/bflad/chef-docker/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/bflad/chef-docker/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/bflad/chef-docker/git/refs{/sha}","trees_url":"https://api.github.com/repos/bflad/chef-docker/git/trees{/sha}","statuses_url":"https://api.github.com/repos/bflad/chef-docker/statuses/{sha}","languages_url":"https://api.github.com/repos/bflad/chef-docker/languages","stargazers_url":"https://api.github.com/repos/bflad/chef-docker/stargazers","contributors_url":"https://api.github.com/repos/bflad/chef-docker/contributors","subscribers_url":"https://api.github.com/repos/bflad/chef-docker/subscribers","subscription_url":"https://api.github.com/repos/bflad/chef-docker/subscription","commits_url":"https://api.github.com/repos/bflad/chef-docker/commits{/sha}","git_commits_url":"https://api.github.com/repos/bflad/chef-docker/git/commits{/sha}","comments_url":"https://api.github.com/repos/bflad/chef-docker/comments{/number}","issue_comment_url":"https://api.github.com/repos/bflad/chef-docker/issues/comments/{number}","contents_url":"https://api.github.com/repos/bflad/chef-docker/contents/{+path}","compare_url":"https://api.github.com/repos/bflad/chef-docker/compare/{base}...{head}","merges_url":"https://api.github.com/repos/bflad/chef-docker/merges","archive_url":"https://api.github.com/repos/bflad/chef-docker/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/bflad/chef-docker/downloads","issues_url":"https://api.github.com/repos/bflad/chef-docker/issues{/number}","pulls_url":"https://api.github.com/repos/bflad/chef-docker/pulls{/number}","milestones_url":"https://api.github.com/repos/bflad/chef-docker/milestones{/number}","notifications_url":"https://api.github.com/repos/bflad/chef-docker/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/bflad/chef-docker/labels{/name}","releases_url":"https://api.github.com/repos/bflad/chef-docker/releases{/id}","created_at":"2013-05-11T00:21:06Z","updated_at":"2014-10-23T01:13:26Z","pushed_at":"2014-10-24T15:30:00Z","git_url":"git://github.com/bflad/chef-docker.git","ssh_url":"git@github.com:bflad/chef-docker.git","clone_url":"https://github.com/bflad/chef-docker.git","svn_url":"https://github.com/bflad/chef-docker","homepage":null,"size":4018,"stargazers_count":289,"watchers_count":289,"language":"Ruby","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":124,"mirror_url":null,"open_issues_count":31,"forks":124,"open_issues":31,"watchers":289,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/bflad/chef-docker/pulls/206"},"html":{"href":"https://github.com/bflad/chef-docker/pull/206"},"issue":{"href":"https://api.github.com/repos/bflad/chef-docker/issues/206"},"comments":{"href":"https://api.github.com/repos/bflad/chef-docker/issues/206/comments"},"review_comments":{"href":"https://api.github.com/repos/bflad/chef-docker/pulls/206/comments"},"review_comment":{"href":"https://api.github.com/repos/bflad/chef-docker/pulls/comments/{number}"},"commits":{"href":"https://api.github.com/repos/bflad/chef-docker/pulls/206/commits"},"statuses":{"href":"https://api.github.com/repos/bflad/chef-docker/statuses/50b73cc7b21c5211a723f8cc6e132f8aec058eda"}},"merged":false,"mergeable":true,"mergeable_state":"clean","merged_by":null,"comments":3,"review_comments":0,"commits":3,"additions":7,"deletions":3,"changed_files":1}}