[CHEF-5168] Apt Package provider times out

- Convert APT package resource to use `provides :package`
- Add timeout parameter

5 files changed, 111 insertions, 54 deletions

diff --git a/lib/chef/platform/provider_mapping.rb b/lib/chef/platform/provider_mapping.rb
index 4ebbde3486..68e206a698 100644
--- a/lib/chef/platform/provider_mapping.rb
+++ b/lib/chef/platform/provider_mapping.rb
@@ -64,7 +64,6 @@ class Chef
             },
             :ubuntu   => {
               :default => {
-                :package => Chef::Provider::Package::Apt,
                 :service => Chef::Provider::Service::Debian,
                 :cron => Chef::Provider::Cron,
                 :mdadm => Chef::Provider::Mdadm
@@ -75,7 +74,6 @@ class Chef
             },
             :gcel   => {
               :default => {
-                :package => Chef::Provider::Package::Apt,
                 :service => Chef::Provider::Service::Debian,
                 :cron => Chef::Provider::Cron,
                 :mdadm => Chef::Provider::Mdadm
@@ -83,7 +81,6 @@ class Chef
             },
             :linaro   => {
               :default => {
-                :package => Chef::Provider::Package::Apt,
                 :service => Chef::Provider::Service::Debian,
                 :cron => Chef::Provider::Cron,
                 :mdadm => Chef::Provider::Mdadm
@@ -91,7 +88,6 @@ class Chef
             },
             :raspbian   => {
               :default => {
-                :package => Chef::Provider::Package::Apt,
                 :service => Chef::Provider::Service::Debian,
                 :cron => Chef::Provider::Cron,
                 :mdadm => Chef::Provider::Mdadm
@@ -99,7 +95,6 @@ class Chef
             },
             :linuxmint   => {
               :default => {
-                :package => Chef::Provider::Package::Apt,
                 :service => Chef::Provider::Service::Upstart,
                 :cron => Chef::Provider::Cron,
                 :mdadm => Chef::Provider::Mdadm
@@ -107,7 +102,6 @@ class Chef
             },
             :debian => {
               :default => {
-                :package => Chef::Provider::Package::Apt,
                 :service => Chef::Provider::Service::Debian,
                 :cron => Chef::Provider::Cron,
                 :mdadm => Chef::Provider::Mdadm
diff --git a/lib/chef/provider/package/apt.rb b/lib/chef/provider/package/apt.rb
index dc7b3f2086..40c220bdbf 100644
--- a/lib/chef/provider/package/apt.rb
+++ b/lib/chef/provider/package/apt.rb
@@ -18,7 +18,7 @@
 
 require 'chef/provider/package'
 require 'chef/mixin/command'
-require 'chef/resource/package'
+require 'chef/resource/apt_package'
 require 'chef/mixin/shell_out'
 
 
@@ -31,7 +31,7 @@ class Chef
         attr_accessor :is_virtual_package
 
         def load_current_resource
-          @current_resource = Chef::Resource::Package.new(@new_resource.name)
+          @current_resource = Chef::Resource::AptPackage.new(@new_resource.name)
           @current_resource.package_name(@new_resource.package_name)
           check_package_state(@new_resource.package_name)
           @current_resource
@@ -46,7 +46,7 @@ class Chef
           Chef::Log.debug("#{@new_resource} checking package status for #{package}")
           installed = false
 
-          shell_out!("apt-cache#{expand_options(default_release_options)} policy #{package}").stdout.each_line do |line|
+          shell_out!("apt-cache#{expand_options(default_release_options)} policy #{package}", :timeout => @new_resource.timeout).stdout.each_line do |line|
             case line
             when /^\s{2}Installed: (.+)$/
               installed_version = $1
@@ -63,7 +63,7 @@ class Chef
               if candidate_version == '(none)'
                 # This may not be an appropriate assumption, but it shouldn't break anything that already worked -- btm
                 @is_virtual_package = true
-                showpkg = shell_out!("apt-cache showpkg #{package}").stdout
+                showpkg = shell_out!("apt-cache showpkg #{package}", :timeout => @new_resource.timeout).stdout
                 providers = Hash.new
                 # Returns all lines after 'Reverse Provides:'
                 showpkg.rpartition(/Reverse Provides:\s*#{$/}/)[2].each_line do |line|
@@ -123,7 +123,7 @@ class Chef
         # interactive prompts. Command is run with default localization rather
         # than forcing locale to "C", so command output may not be stable.
         def run_noninteractive(command)
-          shell_out!(command, :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+          shell_out!(command, :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil }, :timeout => @new_resource.timeout)
         end
 
       end
diff --git a/lib/chef/resource/apt_package.rb b/lib/chef/resource/apt_package.rb
index 050cf838ae..0b91b0cdbf 100644
--- a/lib/chef/resource/apt_package.rb
+++ b/lib/chef/resource/apt_package.rb
@@ -23,11 +23,22 @@ class Chef
   class Resource
     class AptPackage < Chef::Resource::Package
 
+      provides :package, :on_platforms => ["ubuntu", "gcel", "linaro", "raspbian", "linuxmint", "debian"]
+
       def initialize(name, run_context=nil)
         super
         @resource_name = :apt_package
         @provider = Chef::Provider::Package::Apt
         @default_release = nil
+        @timeout = 900
+      end
+
+      def timeout(arg=nil)
+        set_or_return(
+          :timeout,
+          arg,
+          :kind_of => [String, Integer]
+        )
       end
 
       def default_release(arg=nil)
diff --git a/spec/unit/provider/package/apt_spec.rb b/spec/unit/provider/package/apt_spec.rb
index b8e23d8756..ddd8704b77 100644
--- a/spec/unit/provider/package/apt_spec.rb
+++ b/spec/unit/provider/package/apt_spec.rb
@@ -24,7 +24,7 @@ describe Chef::Provider::Package::Apt do
     @node = Chef::Node.new
     @events = Chef::EventDispatch::Dispatcher.new
     @run_context = Chef::RunContext.new(@node, {}, @events)
-    @new_resource = Chef::Resource::Package.new("irssi", @run_context)
+    @new_resource = Chef::Resource::AptPackage.new("irssi", @run_context)
 
     @status = double("Status", :exitstatus => 0)
     @provider = Chef::Provider::Package::Apt.new(@new_resource, @run_context)
@@ -39,16 +39,20 @@ irssi:
 PKG_STATUS
     @stderr = ""
     @shell_out = OpenStruct.new(:stdout => @stdout,:stdin => @stdin,:stderr => @stderr,:status => @status,:exitstatus => 0)
+    @timeout = 900
   end
 
   describe "when loading current resource" do
 
     it "should create a current resource with the name of the new_resource" do
-      @provider.should_receive(:shell_out!).with("apt-cache policy #{@new_resource.package_name}").and_return(@shell_out)
+      @provider.should_receive(:shell_out!).with(
+        "apt-cache policy #{@new_resource.package_name}",
+        :timeout => @timeout
+      ).and_return(@shell_out)
       @provider.load_current_resource
 
       current_resource = @provider.current_resource
-      current_resource.should be_a(Chef::Resource::Package)
+      current_resource.should be_a(Chef::Resource::AptPackage)
       current_resource.name.should == "irssi"
       current_resource.package_name.should == "irssi"
       current_resource.version.should be_nil
@@ -84,7 +88,10 @@ libmysqlclient15-dev:
   Version table:
 VPKG_STDOUT
       virtual_package = double(:stdout => virtual_package_out,:exitstatus => 0)
-      @provider.should_receive(:shell_out!).with("apt-cache policy libmysqlclient15-dev").and_return(virtual_package)
+      @provider.should_receive(:shell_out!).with(
+        "apt-cache policy libmysqlclient15-dev",
+        :timeout => @timeout
+      ).and_return(virtual_package)
       showpkg_out =<<-SHOWPKG_STDOUT
 Package: libmysqlclient15-dev
 Versions:
@@ -104,7 +111,10 @@ libmysqlclient-dev 5.1.41-3ubuntu12.10
 libmysqlclient-dev 5.1.41-3ubuntu12
 SHOWPKG_STDOUT
       showpkg = double(:stdout => showpkg_out,:exitstatus => 0)
-      @provider.should_receive(:shell_out!).with("apt-cache showpkg libmysqlclient15-dev").and_return(showpkg)
+      @provider.should_receive(:shell_out!).with(
+        "apt-cache showpkg libmysqlclient15-dev",
+        :timeout => @timeout
+      ).and_return(showpkg)
       real_package_out=<<-RPKG_STDOUT
 libmysqlclient-dev:
   Installed: 5.1.41-3ubuntu12.10
@@ -119,7 +129,10 @@ libmysqlclient-dev:
         500 http://us.archive.ubuntu.com/ubuntu/ lucid/main Packages
 RPKG_STDOUT
       real_package = double(:stdout => real_package_out,:exitstatus => 0)
-      @provider.should_receive(:shell_out!).with("apt-cache policy libmysqlclient-dev").and_return(real_package)
+      @provider.should_receive(:shell_out!).with(
+        "apt-cache policy libmysqlclient-dev",
+        :timeout => @timeout
+      ).and_return(real_package)
       @provider.load_current_resource
     end
 
@@ -132,7 +145,10 @@ mp3-decoder:
   Version table:
 VPKG_STDOUT
       virtual_package = double(:stdout => virtual_package_out,:exitstatus => 0)
-      @provider.should_receive(:shell_out!).with("apt-cache policy mp3-decoder").and_return(virtual_package)
+      @provider.should_receive(:shell_out!).with(
+        "apt-cache policy mp3-decoder",
+        :timeout => @timeout
+      ).and_return(virtual_package)
       showpkg_out=<<-SHOWPKG_STDOUT
 Package: mp3-decoder
 Versions:
@@ -155,7 +171,10 @@ mpg321 0.2.10.6
 mpg123 1.12.1-0ubuntu1
 SHOWPKG_STDOUT
       showpkg = double(:stdout => showpkg_out,:exitstatus => 0)
-      @provider.should_receive(:shell_out!).with("apt-cache showpkg mp3-decoder").and_return(showpkg)
+      @provider.should_receive(:shell_out!).with(
+        "apt-cache showpkg mp3-decoder",
+        :timeout => @timeout
+      ).and_return(showpkg)
       lambda { @provider.load_current_resource }.should raise_error(Chef::Exceptions::Package)
     end
 
@@ -165,7 +184,10 @@ SHOWPKG_STDOUT
 
       @new_resource.stub(:default_release).and_return("lenny-backports")
       @new_resource.stub(:provider).and_return("Chef::Provider::Package::Apt")
-      @provider.should_receive(:shell_out!).with("apt-cache -o APT::Default-Release=lenny-backports policy irssi").and_return(@shell_out)
+      @provider.should_receive(:shell_out!).with(
+        "apt-cache -o APT::Default-Release=lenny-backports policy irssi",
+        :timeout => @timeout
+      ).and_return(@shell_out)
       @provider.load_current_resource
     end
 
@@ -173,22 +195,26 @@ SHOWPKG_STDOUT
 
   context "after loading the current resource" do
     before do
-      @current_resource = Chef::Resource::Package.new("irssi", @run_context)
+      @current_resource = Chef::Resource::AptPackage.new("irssi", @run_context)
       @provider.current_resource = @current_resource
     end
 
     describe "install_package" do
       it "should run apt-get install with the package name and version" do
-        @provider.should_receive(:shell_out!).
-          with("apt-get -q -y install irssi=0.8.12-7",
-               :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil})
+        @provider.should_receive(:shell_out!). with(
+          "apt-get -q -y install irssi=0.8.12-7",
+          :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil},
+          :timeout => @timeout
+        )
         @provider.install_package("irssi", "0.8.12-7")
       end
 
       it "should run apt-get install with the package name and version and options if specified" do
-        @provider.should_receive(:shell_out!).
-          with("apt-get -q -y --force-yes install irssi=0.8.12-7",
-               :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+        @provider.should_receive(:shell_out!).with(
+          "apt-get -q -y --force-yes install irssi=0.8.12-7",
+          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
+          :timeout => @timeout
+        )
         @new_resource.options("--force-yes")
         @provider.install_package("irssi", "0.8.12-7")
       end
@@ -200,9 +226,11 @@ SHOWPKG_STDOUT
 
         @provider.new_resource = @new_resource
 
-        @provider.should_receive(:shell_out!).
-          with("apt-get -q -y -o APT::Default-Release=lenny-backports install irssi=0.8.12-7",
-                :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+        @provider.should_receive(:shell_out!).with(
+          "apt-get -q -y -o APT::Default-Release=lenny-backports install irssi=0.8.12-7",
+          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
+          :timeout => @timeout
+        )
 
         @provider.install_package("irssi", "0.8.12-7")
       end
@@ -219,16 +247,20 @@ SHOWPKG_STDOUT
     describe Chef::Provider::Package::Apt, "remove_package" do
 
       it "should run apt-get remove with the package name" do
-        @provider.should_receive(:shell_out!).
-          with("apt-get -q -y remove irssi",
-               :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil})
+        @provider.should_receive(:shell_out!).with(
+          "apt-get -q -y remove irssi",
+          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil},
+          :timeout => @timeout
+        )
         @provider.remove_package("irssi", "0.8.12-7")
       end
 
       it "should run apt-get remove with the package name and options if specified" do
-        @provider.should_receive(:shell_out!).
-          with("apt-get -q -y --force-yes remove irssi",
-               :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+        @provider.should_receive(:shell_out!).with(
+          "apt-get -q -y --force-yes remove irssi",
+          :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
+          :timeout => @timeout
+        )
         @new_resource.options("--force-yes")
 
         @provider.remove_package("irssi", "0.8.12-7")
@@ -238,16 +270,20 @@ SHOWPKG_STDOUT
     describe "when purging a package" do
 
       it "should run apt-get purge with the package name" do
-        @provider.should_receive(:shell_out!).
-          with("apt-get -q -y purge irssi",
-               :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+        @provider.should_receive(:shell_out!).with(
+          "apt-get -q -y purge irssi",
+          :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
+          :timeout => @timeout
+        )
         @provider.purge_package("irssi", "0.8.12-7")
       end
 
       it "should run apt-get purge with the package name and options if specified" do
-        @provider.should_receive(:shell_out!).
-          with("apt-get -q -y --force-yes purge irssi",
-          :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+        @provider.should_receive(:shell_out!).with(
+          "apt-get -q -y --force-yes purge irssi",
+          :env => { "DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
+          :timeout => @timeout
+        )
         @new_resource.options("--force-yes")
 
         @provider.purge_package("irssi", "0.8.12-7")
@@ -263,17 +299,21 @@ SHOWPKG_STDOUT
         @provider.should_receive(:get_preseed_file).with("irssi", "0.8.12-7").and_return("/tmp/irssi-0.8.12-7.seed")
         file = @provider.get_preseed_file("irssi", "0.8.12-7")
 
-        @provider.should_receive(:shell_out!).
-          with("debconf-set-selections /tmp/irssi-0.8.12-7.seed",
-          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil})
+        @provider.should_receive(:shell_out!).with(
+          "debconf-set-selections /tmp/irssi-0.8.12-7.seed",
+          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil},
+          :timeout => @timeout
+        )
 
         @provider.preseed_package(file)
       end
 
       it "should run debconf-set-selections on the preseed file if it has changed" do
-        @provider.should_receive(:shell_out!).
-          with("debconf-set-selections /tmp/irssi-0.8.12-7.seed",
-          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil})
+        @provider.should_receive(:shell_out!).with(
+          "debconf-set-selections /tmp/irssi-0.8.12-7.seed",
+          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil},
+          :timeout => @timeout
+        )
         file = @provider.get_preseed_file("irssi", "0.8.12-7")
         @provider.preseed_package(file)
       end
@@ -290,9 +330,11 @@ SHOWPKG_STDOUT
 
     describe "when reconfiguring a package" do
       it "should run dpkg-reconfigure package" do
-        @provider.should_receive(:shell_out!).
-          with("dpkg-reconfigure irssi",
-          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+        @provider.should_receive(:shell_out!).with(
+          "dpkg-reconfigure irssi",
+          :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
+          :timeout => @timeout
+        )
         @provider.reconfig_package("irssi", "0.8.12-7")
       end
     end
@@ -300,9 +342,11 @@ SHOWPKG_STDOUT
     describe "when installing a virtual package" do
       it "should install the package without specifying a version" do
           @provider.is_virtual_package = true
-          @provider.should_receive(:shell_out!).
-            with("apt-get -q -y install libmysqlclient-dev",
-                 :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil })
+          @provider.should_receive(:shell_out!).with(
+            "apt-get -q -y install libmysqlclient-dev",
+            :env => {"DEBIAN_FRONTEND" => "noninteractive", "LC_ALL" => nil },
+            :timeout => @timeout
+          )
           @provider.install_package("libmysqlclient-dev", "not_a_real_version")
       end
     end
diff --git a/spec/unit/resource/apt_package_spec.rb b/spec/unit/resource/apt_package_spec.rb
index 58b007c327..81df04c59f 100644
--- a/spec/unit/resource/apt_package_spec.rb
+++ b/spec/unit/resource/apt_package_spec.rb
@@ -40,4 +40,12 @@ describe Chef::Resource::AptPackage, "initialize" do
     @resource.default_release("lenny-backports")
     @resource.default_release.should eql("lenny-backports")
   end
+
+  # String, Integer
+  [ "600", 600 ].each do |val|
+    it "supports setting a timeout as a #{val.class}" do
+      @resource.timeout(val)
+      expect(@resource.timeout).to eql(val)
+    end
+  end
 end