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

address bflad/chef-docker #137 and support that trusty tahr. #138

Merged
merged 5 commits into from
May 7, 2014

Conversation

brandonmartin
Copy link
Contributor

I think I got most everything that was busted and didn't break anything new, ymmv of course.

fires up docker on 14.04 with the system packages.

@bflad
Copy link
Contributor

bflad commented Apr 28, 2014

First of all: awesome!

You'll need to fix the Upstart rspec test since you modified the start line:

ruby -S rspec ./spec/aufs_spec.rb ./spec/binary_spec.rb ./spec/cgroups_spec.rb ./spec/default_spec.rb ./spec/devicemapper_spec.rb ./spec/group_spec.rb ./spec/lxc_spec.rb ./spec/package_spec.rb ./spec/runit_spec.rb ./spec/source_spec.rb ./spec/systemd_spec.rb ./spec/sysv_spec.rb ./spec/upstart_spec.rb --color --format progress
..................................................................................................................F...................................

Failures:

  1) docker::upstart creates the docker Upstart template
     Failure/Error: expect(chef_run).to render_file('/etc/init/docker.conf').with_content(
       expected Chef run to render "/etc/init/docker.conf" matching:

       (?-mix:"\$DOCKER" -d \$DOCKER_OPTS)

       but got:

       description "Docker daemon"

       start on filesystem
       stop on runlevel [!2345]
       limit nofile 524288 1048576
       limit nproc 524288 1048576

       respawn

       pre-start script
         # see also https://github.com/tianon/cgroupfs-mount/blob/master/cgroupfs-mount
         if grep -v '^#' /etc/fstab | grep -q cgroup \
           || [ ! -e /proc/cgroups ] \
           || [ ! -d /sys/fs/cgroup ]; then
           exit 0
         fi
         if ! mountpoint -q /sys/fs/cgroup; then
           mount -t tmpfs -o uid=0,gid=0,mode=0755 cgroup /sys/fs/cgroup
         fi
         (
           cd /sys/fs/cgroup
           for sys in $(awk '!/^#/ { if ($4 == 1) print $1 }' /proc/cgroups); do
             mkdir -p $sys
             if ! mountpoint -q $sys; then
               if ! mount -n -t cgroup -o $sys cgroup $sys; then
                 rmdir $sys || true
               fi
             fi
           done
         )
       end script

       script
           DOCKER=/usr/bin/$UPSTART_JOB
           DOCKER_OPTS=
           if [ -f /etc/default/$UPSTART_JOB ]; then
               . /etc/default/$UPSTART_JOB
           fi
           "$UPSTART_JOB" -d $DOCKER_OPTS
       end script


     # ./spec/upstart_spec.rb:10:in `block (2 levels) in <top (required)>'

Finished in 1 minute 13.15 seconds
150 examples, 1 failure

I think there's also going to be some caveats here:

  • Does this implementation work with a binary install on 14.04?
  • Is it valid to still use the Docker PPA on 14.04 to get a more updated package than the distro version (e.g. currently 0.9.1 in the trusty repo and Docker PPA packages are at 0.10.0+)?

@brandonmartin
Copy link
Contributor Author

excellent points.

do you mean binary install from source?

i think it is still probably preferable to use the Docker PPA. given the delta that Docker is currently moving at, people want the features and fixes of the bleeding edge.

i'll get the spec file fixed later today.

@brandonmartin
Copy link
Contributor Author

doh! never mind the first question. working on fixes now.

@brandonmartin
Copy link
Contributor Author

fell into a black hole for a few days there, my apologies.

I changed the usage of the docker.io PPA to a new boolean attribute under node['docker']['package']. it defaults to true on all platforms and only has meaning on Ubuntu 14.04+. When set to false on that platform version it performs the aforementioned gymnastics. Otherwise it just carries on and installs from the docker.io package source.

I changed the spec as the change to the upstart template should be good either way.

I confirmed it works in both binary install mode and package install mode.

@bflad
Copy link
Contributor

bflad commented May 5, 2014

Implementation is looking good! One thing: could we change out the extra use_docker_ppa attribute for just setting node['docker']['repo']['url'] = nil when you want the Ubuntu repository? Will require minor changes to your helper method. I think after that we'll be good to merge this in. Thanks!

@bflad
Copy link
Contributor

bflad commented May 7, 2014

I'll go ahead and get this merged, switch to not using the extra attribute, and release in 0.35.0. Thanks for the hard work.

bflad added a commit that referenced this pull request May 7, 2014
address bflad/chef-docker #137 and support that trusty tahr.
@bflad bflad merged commit d8c189c into sous-chefs:master May 7, 2014
@bflad bflad mentioned this pull request May 7, 2014
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.

2 participants