[PATCH/puppet 1/1] Fixes #2836. Adds hold support to dpkg provider

[Nigel Kersten]

From: nigelk <nig...@dirtyhippies.localdomain>


Signed-off-by: Nigel Kersten <nig...@google.com>
---
lib/puppet/provider/package/dpkg.rb | 31 +++++++++++++++++++++++++-
lib/puppet/type/package.rb | 10 +++++++-
spec/unit/provider/package/dpkg.rb | 42 ++++++++++++++++++++++++++++++++--
3 files changed, 78 insertions(+), 5 deletions(-)

diff --git a/lib/puppet/provider/package/dpkg.rb b/lib/puppet/provider/package/dpkg.rb
index a4c3982..4f1c45a 100755
--- a/lib/puppet/provider/package/dpkg.rb
+++ b/lib/puppet/provider/package/dpkg.rb
@@ -5,6 +5,8 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
and not ``apt``, you must specify the source of any packages you want
to manage."

+ has_feature :holdable
+
commands :dpkg => "/usr/bin/dpkg"
commands :dpkg_deb => "/usr/bin/dpkg-deb"
commands :dpkgquery => "/usr/bin/dpkg-query"
@@ -47,9 +49,12 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package

if hash[:status] == 'not-installed'
hash[:ensure] = :purged
- elsif hash[:status] != "installed"
+ elsif hash[:status] == "config-files"
hash[:ensure] = :absent
end
+ if hash[:desired] == "hold"
+ hash[:ensure] = :held
+ end
else
Puppet.warning "Failed to match dpkg-query line %s" % line.inspect
return nil
@@ -65,6 +70,9 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package

args = []

+ # We always unhold when installing to remove any prior hold.
+ self.unhold
+
if @resource[:configfiles] == :keep
args << '--force-confold'
else
@@ -127,4 +135,25 @@ Puppet::Type.type(:package).provide :dpkg, :parent => Puppet::Provider::Package
def purge
dpkg "--purge", @resource[:name]
end
+
+ def hold
+ self.install
+ begin
+ Tempfile.open('puppet_dpkg_set_selection') { |tmpfile|
+ tmpfile.write("#{@resource[:name]} hold\n")
+ tmpfile.flush
+ execute([:dpkg, "--set-selections"], :stdinfile => tmpfile.path.to_s)
+ }
+ end
+ end
+
+ def unhold
+ begin
+ Tempfile.open('puppet_dpkg_set_selection') { |tmpfile|
+ tmpfile.write("#{@resource[:name]} install\n")
+ tmpfile.flush
+ execute([:dpkg, "--set-selections"], :stdinfile => tmpfile.path.to_s)
+ }
+ end
+ end
end
diff --git a/lib/puppet/type/package.rb b/lib/puppet/type/package.rb
index bb3db28..789d27f 100644
--- a/lib/puppet/type/package.rb
+++ b/lib/puppet/type/package.rb
@@ -31,6 +31,10 @@ module Puppet
existing configuration files. This feature is thus destructive
and should be used with the utmost care.",
:methods => [:purge]
+ feature :holdable, "The provider can hold packages. This generally means
+ that the package will not be automatically upgraded. Note that
+ a held status is considered a superset of installed",
+ :methods => [:hold]
feature :versionable, "The provider is capable of interrogating the
package database for installed version(s), and can select
which out of a set of available versions of a package to
@@ -60,6 +64,10 @@ module Puppet
provider.purge
end

+ newvalue(:held, :event => :package_held, :required_features => :holdable) do
+ provider.hold
+ end
+
# Alias the 'present' value.
aliasvalue(:installed, :present)

@@ -111,7 +119,7 @@ module Puppet
@should.each { |should|
case should
when :present
- return true unless [:absent, :purged].include?(is)
+ return true unless [:absent, :purged, :held].include?(is)
when :latest
# Short-circuit packages that are not present
return false if is == :absent or is == :purged
diff --git a/spec/unit/provider/package/dpkg.rb b/spec/unit/provider/package/dpkg.rb
index 08aaca8..305dc4e 100755
--- a/spec/unit/provider/package/dpkg.rb
+++ b/spec/unit/provider/package/dpkg.rb
@@ -88,11 +88,15 @@ describe provider do
@provider.query[:ensure].should == :purged
end

- it "should consider the package absent if its status is neither 'installed' nor 'not-installed'" do
- @provider.expects(:dpkgquery).returns @fakeresult.sub("installed", "foo")
-
+ it "should consider the package absent if it is marked 'config-files'" do
+ @provider.expects(:dpkgquery).returns @fakeresult.sub("installed", "config-files")
@provider.query[:ensure].should == :absent
end
+
+ it "should consider the package held if its state is 'hold'" do
+ @provider.expects(:dpkgquery).returns @fakeresult.sub("install", "hold")
+ @provider.query[:ensure].should == :held
+ end
end

it "should be able to install" do
@@ -130,6 +134,38 @@ describe provider do

@provider.install
end
+
+ it "should ensure any hold is removed" do
+ @provider.expects(:unhold).once
+ @provider.expects(:dpkg)
+ @provider.install
+ end
+ end
+
+ describe "when holding or unholding" do
+ before do
+ @tempfile = stub 'tempfile', :print => nil, :close => nil, :flush => nil, :path => "/other/file"
+ @tempfile.stubs(:write)
+ Tempfile.stubs(:new).returns @tempfile
+ end
+
+ it "should install first if holding" do
+ @provider.stubs(:execute)
+ @provider.expects(:install).once
+ @provider.hold
+ end
+
+ it "should execute dpkg --set-selections when holding" do
+ @provider.stubs(:install)
+ @provider.expects(:execute).with([:dpkg, '--set-selections'], {:stdinfile => @tempfile.path}).once
+ @provider.hold
+ end
+
+ it "should execute dpkg --set-selections when unholding" do
+ @provider.stubs(:install)
+ @provider.expects(:execute).with([:dpkg, '--set-selections'], {:stdinfile => @tempfile.path}).once
+ @provider.hold
+ end
end

it "should use :install to update" do
--
1.6.5.2

[Nigel Kersten]

Any comments from the apt/dpkg/aptitude provider users?

Note that because I'm doing this in the dpkg provider, and both apt
and aptitude inherit from it, they both get this functionality.

In the case of aptitude this is slightly counter-intuitive, as it
means implementing a hold in an aptitude provided package resource
will implement a dpkg hold, not an aptitude hold.

If people feel violently opposed to this behavior, speak up...

> --
>
> You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
>
>
>



--
nigel

[Nigel Kersten]

Anyone? I guess at this stage now I'm just looking for a +1 from
anyone from a code point of view....

--
nigel

[Luke Kanies]

+1, with one question/comment below.

Aren't there other options here besides 'config-files', 'not-
installed', and 'installed'? That is, it seems like we should have an
'else' block but we don't.

That's really necessary? Ouch.

> --
>
> You received this message because you are subscribed to the Google
> Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com
> .
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en
> .
>
>


--

Of the thirty-six ways of avoiding disaster, running away is best.
-- Chinese Proverb
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com

[Nigel Kersten]

argh. you're right. Previously we considered half-installed, unpacked
and half-configured as "absent", and those are the only other possible
states. I do want to handle them explicitly though.

I'm not entirely convinced that we want to consider them absent... I'm
going to think about this for a day or so and gather some more debian
folks opinions.

Yes. :( You really should be able to do this in a more sane manner
than stdin to dpkg --set-selections imho.

You were ok with the tests though Luke? It took me a little while to
get them to a state I was happy with.

>>
>> --
>>
>> You received this message because you are subscribed to the Google
>> Groups "Puppet Developers" group.
>> To post to this group, send email to puppe...@googlegroups.com.
>> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com
>> .
>> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en
>> .
>>
>>
>
>
> --
> Of the thirty-six ways of avoiding disaster, running away is best.
>     -- Chinese Proverb
> ---------------------------------------------------------------------
> Luke Kanies | http://reductivelabs.com | http://madstop.com
>
> --
>
> You received this message because you are subscribed to the Google Groups "Puppet Developers" group.
> To post to this group, send email to puppe...@googlegroups.com.
> To unsubscribe from this group, send email to puppet-dev+...@googlegroups.com.
> For more options, visit this group at http://groups.google.com/group/puppet-dev?hl=en.
>
>
>



--

nigel