+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.

>                 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.

> +            }
> +        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
>
> --
>
> 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.


Reply via email to