(PUP-1537) add hold attribute

This commit adds hold attribute to hold a package on Debian and Solaris
instead using "held" value for ensure.

Using "held" value for ensure prints deprecation warning

Author: Ciprian Badescu

lib/puppet/provider/package/dpkg.rb

@@ -67,7 +67,7 @@ def self.parse_line(line)
       elsif ['config-files', 'half-installed', 'unpacked', 'half-configured'].include?(hash[:status])
         hash[:ensure] = :absent
       end
-      hash[:ensure] = :held if hash[:desired] == 'hold'
+      hash[:hold] = (hash[:desired] == 'hold')
     else
       Puppet.debug("Failed to match dpkg-query line #{line.inspect}")
     end
@@ -81,6 +81,11 @@ def install
     unless file = @resource[:source]
       raise ArgumentError, _("You cannot install dpkg packages without a source")
     end
+
+    if :held == @resource[:ensure] && @resource[:hold]
+      raise ArgumentError, _("You cannot use \"hold=>true\" while \"ensure=>held\"")
+    end
+
     args = []
 
     # We always unhold when installing to remove any prior hold.
@@ -94,6 +99,7 @@ def install
     args << '-i' << file
 
     dpkg(*args)
+    self.hold if @resource[:hold]
   end
 
   def update
@@ -144,10 +150,15 @@ def purge
     dpkg "--purge", @resource[:name]
   end
 
-  def hold
+  def deprecated_hold
+    Puppet.warning "\"ensure=>held\" has been deprecated and will be removed in a future version, use \"hold=true\" instead "
     if package_not_installed?(@resource[:name])
       self.install
     end
+    hold
+  end
+
+  def hold
     Tempfile.open('puppet_dpkg_set_selection') do |tmpfile|
       tmpfile.write("#{@resource[:name]} hold\n")
       tmpfile.flush
@@ -163,6 +174,7 @@ def unhold
     end
   end
 
+  #needed until ensure=held is removed
   def package_not_installed?(name)
     if !name.nil? && !name.empty?
       begin

lib/puppet/provider/package/pkg.rb

@@ -49,7 +49,8 @@ def self.ifo_flag(flags)
     ).merge(
       case flags[1..1]
       when 'f'
-        {:ensure => 'held'}
+        {:hold => true}
+
       when '-'
         {}
       else
@@ -107,6 +108,11 @@ def self.parse_line(line)
     end).merge({:provider => self.name})
   end
 
+  def deprecated_hold
+    Puppet.warning "\"ensure=>held\" has been deprecated and will be removed in a future version, use \"hold=true\" instead "
+    hold
+  end
+
   def hold
     pkg(:freeze, @resource[:name])
   end
@@ -217,6 +223,8 @@ def install(nofail = false)
       name += "@#{should}"
     end
     r = exec_cmd(command(:pkg), command, *args, name)
+    self.hold if @resource[:hold]
+
     return r if nofail
     raise Puppet::Error, _("Unable to update %{package}") % { package: r[:out] } if r[:exit] != 0
   end

lib/puppet/type/package.rb

@@ -54,8 +54,7 @@ module Puppet
     feature :holdable, "The provider is capable of placing packages on hold
         such that they are not automatically upgraded as a result of
         other package dependencies unless explicit action is taken by
-        a user or another package. Held is considered a superset of
-        installed.",
+        a user or another package.",
       :methods => [:hold]
     feature :install_only, "The provider accepts options to only install packages never update (kernels, etc.)"
     feature :install_options, "The provider accepts options to be
@@ -101,7 +100,7 @@ module Puppet
       end
 
       newvalue(:held, :event => :package_held, :required_features => :holdable) do
-        provider.hold
+        provider.deprecated_hold
       end
 
       # Alias the 'present' value.
@@ -611,5 +610,9 @@ def refresh
         provider.reinstall
       end
     end
+
+    newparam(:held, :parent => Puppet::Parameter::Boolean, :required_features => :holdable) do
+      desc "Set to true to tell Debian apt/Solaris pkg to hold the package version"
+    end
   end
 end

spec/unit/provider/package/dpkg_spec.rb

@@ -30,7 +30,7 @@
       expect(Puppet::Util::Execution).to receive(:execpipe).with(execpipe_args).and_yield(bash_installed_io)
 
       installed = double('bash')
-      expect(described_class).to receive(:new).with(:ensure => "4.2-5ubuntu3", :error => "ok", :desired => "install", :name => "bash", :status => "installed", :provider => :dpkg).and_return(installed)
+      expect(described_class).to receive(:new).with(:ensure => "4.2-5ubuntu3", :hold => false, :error => "ok", :desired => "install", :name => "bash", :status => "installed", :provider => :dpkg).and_return(installed)
 
       expect(described_class.instances).to eq([installed])
     end
@@ -39,9 +39,9 @@
       expect(Puppet::Util::Execution).to receive(:execpipe).with(execpipe_args).and_yield(all_installed_io)
 
       bash = double('bash')
-      expect(described_class).to receive(:new).with(:ensure => "4.2-5ubuntu3", :error => "ok", :desired => "install", :name => "bash", :status => "installed", :provider => :dpkg).and_return(bash)
+      expect(described_class).to receive(:new).with(:ensure => "4.2-5ubuntu3", :hold => false, :error => "ok", :desired => "install", :name => "bash", :status => "installed", :provider => :dpkg).and_return(bash)
       vim = double('vim')
-      expect(described_class).to receive(:new).with(:ensure => "2:7.3.547-6ubuntu5", :error => "ok", :desired => "install", :name => "vim", :status => "installed", :provider => :dpkg).and_return(vim)
+      expect(described_class).to receive(:new).with(:ensure => "2:7.3.547-6ubuntu5", :hold => false, :error => "ok", :desired => "install", :name => "vim", :status => "installed", :provider => :dpkg).and_return(vim)
 
       expect(described_class.instances).to eq([bash, vim])
     end
@@ -81,7 +81,7 @@ def dpkg_query_execution_returns(output)
     it "returns a hash of the found package status for an installed package" do
       dpkg_query_execution_returns(bash_installed_output)
 
-      expect(provider.query).to eq({:ensure => "4.2-5ubuntu3", :error => "ok", :desired => "install", :name => "bash", :status => "installed", :provider => :dpkg})
+      expect(provider.query).to eq({:ensure => "4.2-5ubuntu3", :hold => false, :error => "ok", :desired => "install", :name => "bash", :status => "installed", :provider => :dpkg})
     end
 
     it "considers the package absent if the dpkg-query result cannot be interpreted" do
@@ -126,7 +126,10 @@ def dpkg_query_execution_returns(output)
 
     it "considers the package held if its state is 'hold'" do
       dpkg_query_execution_returns(bash_installed_output.gsub("install","hold"))
-      expect(provider.query[:ensure]).to eq(:held)
+      query=provider.query
+      expect(query[:ensure]).to eq("4.2-5ubuntu3")
+      expect(query[:hold]).to eq(true)
+
     end
 
     context "parsing tests" do
@@ -138,6 +141,7 @@ def dpkg_query_execution_returns(output)
           :status => 'status',
           :name => resource_name,
           :ensure => 'ensure',
+          :hold => false,
           :provider => :dpkg,
         }
       end
@@ -183,6 +187,7 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0)
     end
 
     it "uses 'dpkg -i' to install the package" do
+      expect(resource).to receive(:[]).with(:hold).and_return(false)
       expect(resource).to receive(:[]).with(:source).and_return("mypackagefile")
       expect(provider).to receive(:unhold)
       expect(provider).to receive(:dpkg).with(any_args, "-i", "mypackagefile")
@@ -191,6 +196,7 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0)
     end
 
     it "keeps old config files if told to do so" do
+      expect(resource).to receive(:[]).with(:hold).and_return(false)
       expect(resource).to receive(:[]).with(:configfiles).and_return(:keep)
       expect(provider).to receive(:unhold)
       expect(provider).to receive(:dpkg).with("--force-confold", any_args)
@@ -199,6 +205,7 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0)
     end
 
     it "replaces old config files if told to do so" do
+      expect(resource).to receive(:[]).with(:hold).and_return(false)
       expect(resource).to receive(:[]).with(:configfiles).and_return(:replace)
       expect(provider).to receive(:unhold)
       expect(provider).to receive(:dpkg).with("--force-confnew", any_args)
@@ -207,6 +214,7 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0)
     end
 
     it "ensures any hold is removed" do
+      expect(resource).to receive(:[]).with(:hold).and_return(false)
       expect(provider).to receive(:unhold).once
       expect(provider).to receive(:dpkg)
       provider.install
@@ -226,7 +234,7 @@ def parser_test(dpkg_output_string, gold_hash, number_of_debug_logs = 0)
       allow(provider).to receive(:execute)
       allow(provider).to receive(:package_not_installed?).and_return(true)
       expect(provider).to receive(:install).once
-      provider.hold
+      provider.deprecated_hold
     end
 
     it "skips install new package if package is allready installed" do

spec/unit/provider/package/pkg_spec.rb

@@ -90,7 +90,7 @@ def self.it_should_respond_to(*actions)
 
       {
         'pkg://omnios/[email protected],5.11-0.151006:20130506T161045Z    i--' => {:name => 'SUNWcs', :ensure => '0.5.11,5.11-0.151006:20130506T161045Z', :status => 'installed', :provider => :pkg, :publisher => 'omnios'},
-        'pkg://omnios/incorporation/jeos/illumos-gate@11,5.11-0.151006:20130506T183443Z if-' => {:name => 'incorporation/jeos/illumos-gate', :ensure => 'held', :status => 'installed', :provider => :pkg, :publisher => 'omnios'},
+        'pkg://omnios/incorporation/jeos/illumos-gate@11,5.11-0.151006:20130506T183443Z if-' => {:name => 'incorporation/jeos/illumos-gate', :ensure => "11,5.11-0.151006:20130506T183443Z", :hold => true, :status => 'installed', :provider => :pkg, :publisher => 'omnios'},
         'pkg://solaris/[email protected],5.11-0.151.0.1:20101105T001108Z      installed  -----' => {:name => 'SUNWcs', :ensure => '0.5.11,5.11-0.151.0.1:20101105T001108Z', :status => 'installed', :provider => :pkg, :publisher => 'solaris'},
        }.each do |k, v|
         it "[#{k}] should correctly parse" do