Skip to content
This repository has been archived by the owner on Jul 6, 2018. It is now read-only.

leverage exec for transport and implements connect_to_machine and stop_machine #56

Merged
merged 2 commits into from Jul 28, 2015

Conversation

mwrock
Copy link
Contributor

@mwrock mwrock commented Jun 13, 2015

This PR simplifies reconverging and changing the state of running containers by levereging exec in the transport. It also implements connect_to_machine and thus allows one to use the machine_execute and machine_file resources on docker containers.

This ended up being bringing in some fairly deep changes so here are some implementation notes:

  • Removes the Gemfile.lock and adds basic entries to the empty .gitignore file to provide a better dev experience.
  • Replaces the apache based readme example with the ssh_server cookbook. Currently the apache example fails to start the container. The apache::default recipe is empty and the command /usr/sbin/httpd is not applicable to the ubuntu base image used. Even using /usr/sbin/apache2 and the apache2::default cookbook had some config errors. The ssh_server cookbook just worked and results in a running container.
  • There was a fair amount of "dead code" in the transport that I deleted. It was called by the execute_old code deleted over a year ago and I found it added confusion to the class.
  • Much of the initial container port, volume, etc. setup is moved to the driver and is now a one-time operation done in machine_allocate and simple runs a vanilla bash loop as suggested by @marc- . machine_allocate now creates the container but it is "unconverged" and not running the final command. However it does have the ports, volumes, etc properly configured.
  • So the transport now just delegates to Docker::Container.exec
  • Writes to the container writes to the container mount usually located in /var/lib/docker
  • This now streams stdout/err similar to most of the other drivers using stream_chunk
  • The docker_container_machine commits images after convergence and if the command has changed, starts a new container
  • Container and image lookups now prefer IDs and the implementation ensures these are kept in alignment with the machine/image spec.

One thing that is a definite hack and I think is due to a bug in chef-provisioning is this line in allocate_machine:

machine_spec.from_image = action_handler.provider.new_resource.from_image

from_image is not set in the provider until after the call to allocate_machine

I'll submit a separate PR to chef-provisioning to address tis unless someone thinks this is necessary.

if @command && transport.container.info['Config']['Cmd'].join(' ') != @command
transport.container.delete(:force => true)
container = image.run(Shellwords.split(@command))
container.rename(machine_spec.reference['container_name'])
Copy link
Contributor

Choose a reason for hiding this comment

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

I think

container = Docker::Container.create('Image' => image.id, 'Cmd' => Shellwords.split(@command), 'name' => container_name)

more convenient way to run container.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its kind of a wash since create doesn't start the container so either way you have to perform 2 operations.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it does https://github.com/swipely/docker-api/blob/master/lib/docker/container.rb#L227 . Anyways it's minor and totally up to you.

@mwrock
Copy link
Contributor Author

mwrock commented Jun 14, 2015

@marc- I found a different approach to writing to the container that works on both ubuntu and centos hosts. Just commited it but it writes to the /proc filesystem under the Pid of the container.

@marc-
Copy link
Contributor

marc- commented Jun 14, 2015

Yes, this will work (I actually didn't know, that one can access container's file system this way, good finding).
But what if we have a remote docker daemon? Please checkout following commands:

cat random_file | docker -H 192.168.34.7:2376 exec -i my_container /bin/sh -c 'cat > random_file'

This is for writing file. And following for reading:

docker -H 192.168.34.7:2376 exec -i my_container /bin/sh -c 'cat random_file' > random_file

This will work both for local docker daemon and remote.
I'm currently looking into how to make chef-provisioning-docker work with Docker Swarm (#55), which uses remote docker daemon to communicate with nodes, and I would appreciate if you consider approach above.

@mwrock mwrock changed the title leverage exec for transport and implement connect_to_machine leverage exec for transport and implements connect_to_machine and stop_machine Jun 16, 2015
@mwrock
Copy link
Contributor Author

mwrock commented Jun 20, 2015

squashed and rebased

end

args << image.id
args += Shellwords.split("/bin/sh -c 'while true;do sleep 1; done'")
Copy link
Contributor

Choose a reason for hiding this comment

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

I have concerns regarding to while loop here. What if we rather use /bin/sh -l with -i docker run flag instead. Check this:

docker run -i -d ubuntu /bin/sh -l

Copy link
Contributor Author

Choose a reason for hiding this comment

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

what are your concerns? changing this to running interactively fundamentally changes how this all flows. I don't think cmd.run_command would simply return and leave you with a running container.

@@ -70,66 +70,116 @@ def self.connection_url(driver_url)
end
end


def allocate_machine(action_handler, machine_spec, machine_options)
Copy link
Contributor

Choose a reason for hiding this comment

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

Hey @mwrock - now that chef-boneyard/chef-provisioning#366 is merged you should just be able to use from_image = machine_options.from_image correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's right . As long as this gem takes a dependency on the new version of chef-provisioning.

@tyler-ball
Copy link
Contributor

Hey @mwrock - do you need any help getting this across the finish line?

@mwrock
Copy link
Contributor Author

mwrock commented Jul 28, 2015

It has been a couple weeks, but last I checked this all worked well. The only outstanding issue would be refactoring the from_image to use the new code in master in chef-provisioning. We'd want to wait for a new chef-provisioning release and then tie this gem to that version otherwise the refactoring would break. I think that could be done in a separate PR.

Is there anything else here we think needs changing?

tyler-ball added a commit that referenced this pull request Jul 28, 2015
leverage exec for transport and implements connect_to_machine and stop_machine
@tyler-ball tyler-ball merged commit 06a352a into chef-boneyard:master Jul 28, 2015
@mwrock
Copy link
Contributor Author

mwrock commented Jul 28, 2015

ah sweet thanks!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

4 participants