Multiple fixes to container name detection #206
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This 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).
Handle case when container has multiple names
Containers 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.Example:
When a container has multiple names, the
new_resource.container_name
may not be found in the output ofdocker ps
because of exact string comparison with the raw value of the NAMES column.acc8296 should allow matching the container name with any of the different names of the container.
Made container matching more strict
When 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 followingdocker ps
output:Assuming
new_resource.container_name = 'my-backend-name'
, the current version ofcontainer_matches?
will match a container that has:such as
my-test-app
in the above example just becausemy-test-app
is beforemy-backend-app
in thedocker ps
output.As a consequence, chef tried to recreate the
my-backend-app
container (withdocker 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.My fix in cac59af 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.
Filter out technical names
When 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.In the case where
docker run --link
is used, it results in thecurrent_resource.container_name
being set tomy-backend-app,my-test-app/link
instead ofmy-backend-app
which was thecontainer_name
that I have set in my recipe.In 50b73cc 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.