On Wed, Dec 9, 2009 at 5:46 PM, Luke Kanies <[email protected]> wrote:
> +1, with one question/comment below.
>
> On Dec 1, 2009, at 7:46 AM, Nigel Kersten wrote:
>
>> From: nigelk <[email protected]>
>>
>>
>> Signed-off-by: Nigel Kersten <[email protected]>
>> ---
>> 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"
>
> 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.
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.
>
>> 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)
>
> That's really necessary? Ouch.
Yes. :( You really should be able to do this in a more sane manner
than stdin to dpkg --set-selections imho.
>
>> + }
>> + 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
>> - �[email protected](:dpkgquery).returns
>> @fakeresult.sub("installed", "foo")
>> -
>> + it "should consider the package absent if it is marked
>> 'config-files'" do
>> + �[email protected](: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
>> + �[email protected](:dpkgquery).returns
>> @fakeresult.sub("install", "hold")
>> + �[email protected][: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
>> + �[email protected](:unhold).once
>> + �[email protected](:dpkg)
>> + �[email protected]
>> + end
>> + end
>> +
>> + describe "when holding or unholding" do
>> + before do
>> + �...@tempfile = stub 'tempfile', :print => nil, :close =>
>> nil, :flush => nil, :path => "/other/file"
>> + �[email protected](:write)
>> + Tempfile.stubs(:new).returns @tempfile
>> + end
>> +
>> + it "should install first if holding" do
>> + �[email protected](:execute)
>> + �[email protected](:install).once
>> + �[email protected]
>> + end
>> +
>> + it "should execute dpkg --set-selections when holding" do
>> + �[email protected](:install)
>> + �[email protected](:execute).with([:dpkg, '--set-
>> selections'], {:stdinfile => @tempfile.path}).once
>> + �[email protected]
>> + end
>> +
>> + it "should execute dpkg --set-selections when unholding" do
>> + �[email protected](:install)
>> + �[email protected](:execute).with([:dpkg, '--set-
>> selections'], {:stdinfile => @tempfile.path}).once
>> + �[email protected]
>> + end
>> end
>>
>> it "should use :install to update" do
>> --
>> 1.6.5.2
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 [email protected].
>> To unsubscribe from this group, send email to
>> [email protected]
>> .
>> 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 [email protected].
> To unsubscribe from this group, send email to
> [email protected].
> For more options, visit this group at
> http://groups.google.com/group/puppet-dev?hl=en.
>
>
>
--
nigel
--
You received this message because you are subscribed to the Google Groups
"Puppet Developers" group.
To post to this group, send email to [email protected].
To unsubscribe from this group, send email to
[email protected].
For more options, visit this group at
http://groups.google.com/group/puppet-dev?hl=en.
