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

Fixed remove_link container action. #181

Merged
merged 1 commit into from
Jul 10, 2014

Conversation

jperville
Copy link
Contributor

This PR fixes the new docker_container remove_link action that was introduced in e298547. The feature was broken because the generated docker_cmd did not respect the syntax of docker rm --link.

Given the following containers:

$ docker run --rm -t -i --name myssh -p 22222:22 busybox:ubuntu-14.04 /bin/sh
$ docker run --rm -t -i --link myssh:ssh --name jovial_franklin busybox:ubuntu-14.04 /bin/sh

And the following chef resource:

docker_container 'jovial_franklin' do
  link 'ssh'
  action :remove_container
end

Before this PR, the generated docker_cmd would generate execute like this:

$ docker rm  --link="ssh" 821a290387c0b44032a09aa9b85d0247d72852c5275e49c83417ebd0a38028de
invalid boolean value "ssh" for  --link: strconv.ParseBool: parsing "ssh": invalid syntax

Usage: docker rm [OPTIONS] CONTAINER [CONTAINER...]

Remove one or more containers

  -f, --force=false      Force removal of running container
  -l, --link=false       Remove the specified link and not the underlying container
  -v, --volumes=false    Remove the volumes associated to the container

After this PR we now have:

$ docker rm --link=true jovial_franklin/ssh

Which should work as expected.

@bflad
Copy link
Contributor

bflad commented Jul 10, 2014

Great fix, thanks so much!

bflad added a commit that referenced this pull request Jul 10, 2014
@bflad bflad merged commit a82d2b1 into sous-chefs:master Jul 10, 2014
@jperville jperville deleted the fix-container-remove-link branch August 29, 2014 12:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants