From 4a4797cefe321b20da5561ddf1ebae44f0dd48e2 Mon Sep 17 00:00:00 2001 From: Joe Crobak Date: Mon, 3 Feb 2014 21:45:48 +0000 Subject: [PATCH 1/4] Update all LWRPs to check that docker is alive. Before issuing any commands that talk to the local docker daemon, make sure it is running (since its service starts in the background). --- README.md | 1 + attributes/default.rb | 1 + libraries/helpers.rb | 49 ++++++++++++++++++++++++++++++++++++++++++ providers/container.rb | 1 + providers/image.rb | 1 + providers/registry.rb | 1 + 6 files changed, 54 insertions(+) diff --git a/README.md b/README.md index 958dec8a99..c6bf387fed 100644 --- a/README.md +++ b/README.md @@ -58,6 +58,7 @@ install_dir | Installation directory for docker binary | String | auto-detected install_type | Installation type for docker ("binary", "package" or "source") | String | "package" options | Additional options to pass to docker. These could be flags like "-api-enable-cors". | String | nil registry_cmd_timeout | registry LWRP default cmd_timeout seconds | Fixnum | 60 +docker_daemon_timeout | Timeout to wait for the docker daemon to start in seconds | Fixnum | 10 storage_type | Storage driver for docker (nil, "aufs", or "devmapper") | String | auto-detected (see attributes/default.rb) version | Version of docker | String | nil virtualization_type | Virtualization driver for docker (nil or "lxc") | String | auto-detected (see attributes/default.rb) diff --git a/attributes/default.rb b/attributes/default.rb index 94acdb2b7e..8e84963af4 100644 --- a/attributes/default.rb +++ b/attributes/default.rb @@ -14,6 +14,7 @@ default['docker']['http_proxy'] = nil default['docker']['image_cmd_timeout'] = 300 default['docker']['registry_cmd_timeout'] = 60 +default['docker']['docker_daemon_timeout'] = 10 default['docker']['init_type'] = value_for_platform( %w{ centos debian oracle redhat } => { diff --git a/libraries/helpers.rb b/libraries/helpers.rb index c1f1a608d3..0766df84fa 100644 --- a/libraries/helpers.rb +++ b/libraries/helpers.rb @@ -1,7 +1,27 @@ +require 'chef/mixin/shell_out' +include Chef::Mixin::ShellOut + # Helpers module module Helpers # Helpers::Docker module module Docker + # Exception to signify that the Docker daemon is not yet ready to handle + # docker commands. + class DockerNotReady < StandardError + def initialize(timeout) + super <<-EOH +The Docker daemon did not become ready within #{timeout} seconds. +This most likely means that Docker failed to start. +Docker can fail to start if: + + - a configuration file is invalid + - permissions are incorrect for the root directory of the docker runtime. + +If this problem persists, check your service log files. +EOH + end + end + def cli_args(spec) cli_line = '' spec.each_pair do |arg, value| @@ -28,5 +48,34 @@ def docker_inspect_id(id) inspect = docker_inspect(id) inspect['id'] if inspect end + + def timeout + node['docker']['docker_daemon_timeout'] + end + + # This is based upon wait_until_ready! from the opscode jenkins cookbook. + # + # Since the docker service returns immediately and the actual docker + # process is started as a daemon, we block the Chef Client run until the + # daemon is actually ready. + # + # This method will effectively "block" the current thread until the docker + # daemon is ready + # + # @raise [DockerNotReady] + # if the Docker master does not respond within (+timeout+) seconds + # + def wait_until_ready! + Timeout.timeout(timeout) do + loop do + result = shell_out('docker info') + break if Array(result.valid_exit_codes).include?(result.exitstatus) + Chef::Log.debug("Docker daemon is not running - #{result.stdout}\n#{result.stderr}") + sleep(0.5) + end + end + rescue Timeout::Error + raise DockerNotReady.new(timeout), 'docker timeout exceeded' + end end end diff --git a/providers/container.rb b/providers/container.rb index 1f8d7ef278..6c3e593076 100644 --- a/providers/container.rb +++ b/providers/container.rb @@ -6,6 +6,7 @@ class CommandTimeout < RuntimeError; end def load_current_resource @current_resource = Chef::Resource::DockerContainer.new(new_resource) + wait_until_ready! dps = docker_cmd('ps -a -notrunc') dps.stdout.each_line do |dps_line| ps = dps(dps_line) diff --git a/providers/image.rb b/providers/image.rb index caaa227816..ed1fce14e6 100644 --- a/providers/image.rb +++ b/providers/image.rb @@ -5,6 +5,7 @@ class CommandTimeout < RuntimeError; end def load_current_resource + wait_until_ready! @current_resource = Chef::Resource::DockerImage.new(new_resource) dimages = docker_cmd('images -a -notrunc') if dimages.stdout.include?(new_resource.image_name) diff --git a/providers/registry.rb b/providers/registry.rb index b5d71014f2..0414c90f6a 100644 --- a/providers/registry.rb +++ b/providers/registry.rb @@ -6,6 +6,7 @@ class CommandTimeout < RuntimeError; end def load_current_resource @current_resource = Chef::Resource::DockerRegistry.new(new_resource) + wait_until_ready! # TODO: load current resource? @current_resource end From 8c6cb6fd8ded744593cc56c87ac9914dab3c2ed0 Mon Sep 17 00:00:00 2001 From: Joe Crobak Date: Mon, 3 Feb 2014 22:27:04 +0000 Subject: [PATCH 2/4] Move docker_cmd and execute_cmd methods to helper Move docker_cmd and execute_cmd into the helper mixin to remove some repeated code. --- libraries/helpers.rb | 27 +++++++++++++++++++++++++++ providers/container.rb | 9 ++------- providers/image.rb | 13 ++----------- providers/registry.rb | 13 ++----------- 4 files changed, 33 insertions(+), 29 deletions(-) diff --git a/libraries/helpers.rb b/libraries/helpers.rb index 0766df84fa..47f59c6a65 100644 --- a/libraries/helpers.rb +++ b/libraries/helpers.rb @@ -77,5 +77,32 @@ def wait_until_ready! rescue Timeout::Error raise DockerNotReady.new(timeout), 'docker timeout exceeded' end + + # the Error message to display if a command times out. Subclasses + # may want to override this to provide more details on the timeout. + def command_timeout_error_message + <<-EOM + +Command timed out: +#{cmd} + +EOM + end + + # Runs a docker command. Does not raise exception on non-zero exit code. + def docker_cmd(cmd, timeout = new_resource.cmd_timeout) + execute_cmd('docker ' + cmd, timeout) + end + + # Executes the given command with the specified timeout. Does not raise an + # exception on a non-zero exit code. + def execute_cmd(cmd, timeout = new_resource.cmd_timeout) + Chef::Log.debug('Executing: ' + cmd) + begin + shell_out(cmd, :timeout => timeout) + rescue Mixlib::ShellOut::CommandTimeout + raise CommandTimeout, command_timeout_error_message + end + end end end diff --git a/providers/container.rb b/providers/container.rb index 6c3e593076..36d1cd1ab5 100644 --- a/providers/container.rb +++ b/providers/container.rb @@ -176,19 +176,14 @@ def dps(dps_line) ps end -def execute_cmd(cmd, timeout = new_resource.cmd_timeout) - Chef::Log.debug('Executing: ' + cmd) - begin - shell_out(cmd, :timeout => timeout) - rescue Mixlib::ShellOut::CommandTimeout - raise CommandTimeout, <<-EOM +def command_timeout_error_message + <<-EOM Command timed out: #{cmd} Please adjust node container_cmd_timeout attribute or this docker_container cmd_timeout attribute if necessary. EOM - end end def exists? diff --git a/providers/image.rb b/providers/image.rb index ed1fce14e6..8a50e0d8a3 100644 --- a/providers/image.rb +++ b/providers/image.rb @@ -137,23 +137,14 @@ def di(di_line) image end -def docker_cmd(cmd, timeout = new_resource.cmd_timeout) - execute_cmd('docker ' + cmd, timeout) -end - -def execute_cmd(cmd, timeout = new_resource.cmd_timeout) - Chef::Log.debug('Executing: ' + cmd) - begin - shell_out(cmd, :timeout => timeout) - rescue Mixlib::ShellOut::CommandTimeout - raise CommandTimeout, <<-EOM +def command_timeout_error_message + <<-EOM Command timed out: #{cmd} Please adjust node image_cmd_timeout attribute or this docker_image cmd_timeout attribute if necessary. EOM - end end def image_id_matches?(id) diff --git a/providers/registry.rb b/providers/registry.rb index 0414c90f6a..dd5cdd35a4 100644 --- a/providers/registry.rb +++ b/providers/registry.rb @@ -18,23 +18,14 @@ def load_current_resource end end -def docker_cmd(cmd, timeout = new_resource.cmd_timeout) - execute_cmd('docker ' + cmd, timeout) -end - -def execute_cmd(cmd, timeout = new_resource.cmd_timeout) - Chef::Log.debug('Executing: ' + cmd) - begin - shell_out(cmd, :timeout => timeout) - rescue Mixlib::ShellOut::CommandTimeout - raise CommandTimeout, <<-EOM +def command_timeout_error_message + <<-EOM Command timed out: #{cmd} Please adjust node registry_cmd_timeout attribute or this docker_registry cmd_timeout attribute if necessary. EOM - end end def logged_in? From 7f81e12c4766a809a5f5a95622d634ab5aa455d1 Mon Sep 17 00:00:00 2001 From: Joe Crobak Date: Mon, 3 Feb 2014 22:29:27 +0000 Subject: [PATCH 3/4] Add docker_cmd! to raise exceptions on failure. Update all call sites to use the failing version of docker_cmd. --- libraries/helpers.rb | 14 ++++++++++++++ providers/container.rb | 22 +++++++++++----------- providers/image.rb | 18 +++++++++--------- providers/registry.rb | 2 +- 4 files changed, 35 insertions(+), 21 deletions(-) diff --git a/libraries/helpers.rb b/libraries/helpers.rb index 47f59c6a65..0693d9c361 100644 --- a/libraries/helpers.rb +++ b/libraries/helpers.rb @@ -104,5 +104,19 @@ def execute_cmd(cmd, timeout = new_resource.cmd_timeout) raise CommandTimeout, command_timeout_error_message end end + + # Executes the given docker command with the specified timeout. Raises an + # exception if the command returns a non-zero exit code. + def docker_cmd!(cmd, timeout = new_resource.cmd_timeout) + execute_cmd!('docker ' + cmd, timeout) + end + + # Executes teh given command with the specified timeout. Raises an + # exception if the command returns a non-zero exit code. + def execute_cmd!(cmd, timeout = new_resource.cmd_timeout) + cmd = execute_cmd(cmd, timeout) + cmd.error! + cmd + end end end diff --git a/providers/container.rb b/providers/container.rb index 36d1cd1ab5..fe08807641 100644 --- a/providers/container.rb +++ b/providers/container.rb @@ -7,7 +7,7 @@ class CommandTimeout < RuntimeError; end def load_current_resource @current_resource = Chef::Resource::DockerContainer.new(new_resource) wait_until_ready! - dps = docker_cmd('ps -a -notrunc') + dps = docker_cmd!('ps -a -notrunc') dps.stdout.each_line do |dps_line| ps = dps(dps_line) unless container_id_matches?(ps['id']) @@ -122,7 +122,7 @@ def commit commit_end_args = new_resource.repository commit_end_args += ":#{new_resource.tag}" if new_resource.tag end - docker_cmd("commit #{commit_args} #{current_resource.id} #{commit_end_args}") + docker_cmd!("commit #{commit_args} #{current_resource.id} #{commit_end_args}") end def container_command_matches_if_exists?(command) @@ -152,7 +152,7 @@ def container_name end def cp - docker_cmd("cp #{current_resource.id}:#{new_resource.source} #{new_resource.destination}") + docker_cmd!("cp #{current_resource.id}:#{new_resource.source} #{new_resource.destination}") end def docker_cmd(cmd, timeout = new_resource.cmd_timeout) @@ -191,14 +191,14 @@ def exists? end def export - docker_cmd("export #{current_resource.id} > #{new_resource.destination}") + docker_cmd!("export #{current_resource.id} > #{new_resource.destination}") end def kill if service? service_stop else - docker_cmd("kill #{current_resource.id}") + docker_cmd!("kill #{current_resource.id}") end end @@ -217,7 +217,7 @@ def remove rm_args = cli_args( 'link' => new_resource.link ) - docker_cmd("rm #{rm_args} #{current_resource.id}") + docker_cmd!("rm #{rm_args} #{current_resource.id}") service_remove if service? end @@ -225,7 +225,7 @@ def restart if service? service_restart else - docker_cmd("restart #{current_resource.id}") + docker_cmd!("restart #{current_resource.id}") end end @@ -254,7 +254,7 @@ def run 'volumes-from' => new_resource.volumes_from, 'w' => new_resource.working_directory ) - dr = docker_cmd("run #{run_args} #{new_resource.image} #{new_resource.command}") + dr = docker_cmd!("run #{run_args} #{new_resource.image} #{new_resource.command}") dr.error! new_resource.id(dr.stdout.chomp) service_create if service? @@ -438,7 +438,7 @@ def start if service? service_create else - docker_cmd("start #{start_args} #{current_resource.id}") + docker_cmd!("start #{start_args} #{current_resource.id}") end end @@ -449,10 +449,10 @@ def stop if service? service_stop else - docker_cmd("stop #{stop_args} #{current_resource.id}", (new_resource.cmd_timeout + 15)) + docker_cmd!("stop #{stop_args} #{current_resource.id}", (new_resource.cmd_timeout + 15)) end end def wait - docker_cmd("wait #{current_resource.id}") + docker_cmd!("wait #{current_resource.id}") end diff --git a/providers/image.rb b/providers/image.rb index 8a50e0d8a3..6962eacfe8 100644 --- a/providers/image.rb +++ b/providers/image.rb @@ -123,7 +123,7 @@ def build command = new_resource.source end - docker_cmd("build #{build_args} #{command}") + docker_cmd!("build #{build_args} #{command}") end def di(di_line) @@ -175,12 +175,12 @@ def import import_args += new_resource.source import_args += " #{new_resource.image_name}" end - docker_cmd("import #{import_args} #{repository_and_tag_args}") + docker_cmd!("import #{import_args} #{repository_and_tag_args}") end end def insert - docker_cmd("insert #{new_resource.image_name} #{new_resource.source} #{new_resource.destination}") + docker_cmd!("insert #{new_resource.image_name} #{new_resource.source} #{new_resource.destination}") end def installed? @@ -188,7 +188,7 @@ def installed? end def load - docker_cmd("load < #{new_resource.source}") + docker_cmd!("load < #{new_resource.source}") end def pull @@ -196,15 +196,15 @@ def pull 'registry' => new_resource.registry, 't' => new_resource.tag ) - docker_cmd("pull #{new_resource.image_name} #{pull_args}") + docker_cmd!("pull #{new_resource.image_name} #{pull_args}") end def push - docker_cmd("push #{new_resource.image_name}") + docker_cmd!("push #{new_resource.image_name}") end def remove - docker_cmd("rmi #{new_resource.image_name}") + docker_cmd!("rmi #{new_resource.image_name}") end def repository_and_tag_args @@ -217,12 +217,12 @@ def repository_and_tag_args end def save - docker_cmd("save #{new_resource.image_name} > #{new_resource.destination}") + docker_cmd!("save #{new_resource.image_name} > #{new_resource.destination}") end def tag tag_args = cli_args( 'f' => new_resource.force ) - docker_cmd("tag #{tag_args} #{new_resource.image_name} #{repository_and_tag_args}") + docker_cmd!("tag #{tag_args} #{new_resource.image_name} #{repository_and_tag_args}") end diff --git a/providers/registry.rb b/providers/registry.rb index dd5cdd35a4..77979f4079 100644 --- a/providers/registry.rb +++ b/providers/registry.rb @@ -38,5 +38,5 @@ def login 'p' => new_resource.password, 'u' => new_resource.username ) - docker_cmd("login #{new_resource.server} #{login_args}") + docker_cmd!("login #{new_resource.server} #{login_args}") end From 5e416e62bac98cd22ce6243d97b2e7a0d65c62d0 Mon Sep 17 00:00:00 2001 From: Joe Crobak Date: Tue, 4 Feb 2014 02:33:05 +0000 Subject: [PATCH 4/4] Fix removal of tagged image. `docker rmi` defaults to `latest` if no tag is specified. But if you have a tagged image, there might not be a latest, in which case you must qualify the image with a tag name. --- providers/image.rb | 4 +++- .../files/default/tests/minitest/image_lwrp_test.rb | 6 +++++- test/cookbooks/docker_test/recipes/image_lwrp.rb | 1 + 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/providers/image.rb b/providers/image.rb index 6962eacfe8..98a732c047 100644 --- a/providers/image.rb +++ b/providers/image.rb @@ -204,7 +204,9 @@ def push end def remove - docker_cmd!("rmi #{new_resource.image_name}") + image_name = new_resource.image_name + image_name = "#{image_name}:#{new_resource.tag}" if new_resource.tag + docker_cmd!("rmi #{image_name}") end def repository_and_tag_args diff --git a/test/cookbooks/docker_test/files/default/tests/minitest/image_lwrp_test.rb b/test/cookbooks/docker_test/files/default/tests/minitest/image_lwrp_test.rb index f8e9a91de1..823ea045f9 100644 --- a/test/cookbooks/docker_test/files/default/tests/minitest/image_lwrp_test.rb +++ b/test/cookbooks/docker_test/files/default/tests/minitest/image_lwrp_test.rb @@ -2,7 +2,7 @@ describe_recipe "docker_test::image_lwrp_test" do include Helpers::DockerTest - + it "has base image not installed" do refute image_exists?("base") end @@ -14,4 +14,8 @@ it "has bflad/testcontainerd image installed" do assert image_exists?("bflad/testcontainerd") end + + it "has myImage image not installed" do + refute image_exists?("myImage") + end end diff --git a/test/cookbooks/docker_test/recipes/image_lwrp.rb b/test/cookbooks/docker_test/recipes/image_lwrp.rb index 1b94913b51..832ee3b27b 100644 --- a/test/cookbooks/docker_test/recipes/image_lwrp.rb +++ b/test/cookbooks/docker_test/recipes/image_lwrp.rb @@ -19,5 +19,6 @@ end docker_image "myImage" do + tag "myTag" action :remove end