Please review pull request #560: (#7986) Modify pkg provider to handle publisher opened by (stschulte)
Description:
The pkg provider that is used on solaris does not handle output lines of pkg list correctly if there is a package installed that is not from the preferred publisher.
The patchset changes the regex so lines are correctly parsed, updates the spec tests and contains some general cleanup of the provider
- Opened: Sun Mar 04 17:03:40 UTC 2012
- Based on: puppetlabs:2.7.x (3c422310c05e884c65b24c9af76e56a8d625d6c7)
- Requested merge: stschulte:ticket/2.7.x/7986 (54e5d18efe988b628028c64c7944156c465864bd)
Diff follows:
diff --git a/lib/puppet/provider/package/pkg.rb b/lib/puppet/provider/package/pkg.rb
index 0b826e8..399a9df 100644
--- a/lib/puppet/provider/package/pkg.rb
+++ b/lib/puppet/provider/package/pkg.rb
@@ -12,22 +12,17 @@
def self.instances
packages = []
- cmd = "#{command(:pkg)} list -H"
- execpipe(cmd) do |process|
- hash = {}
-
+ pkg(:list, '-H').each_line do |line|
# now turn each returned line into a package object
- process.each_line { |line|
- if hash = parse_line(line)
- packages << new(hash)
- end
- }
+ if hash = parse_line(line.chomp)
+ packages << new(hash)
+ end
end
packages
end
- self::REGEX = %r{^(\S+)\s+(\S+)\s+(\S+)\s+}
+ self::REGEX = /^(\S+)(?:\s+\(.*?\))?\s+(\S+)\s+(\S+)\s+\S+$/
self::FIELDS = [:name, :version, :status]
def self.parse_line(line)
@@ -39,7 +34,6 @@ def self.parse_line(line)
}
hash[:provider] = self.name
- hash[:error] = "ok"
if hash[:status] == "installed"
hash[:ensure] = :present
@@ -47,7 +41,7 @@ def self.parse_line(line)
hash[:ensure] = :absent
end
else
- Puppet.warning "Failed to match 'pkg list' line #{line.inspect}"
+ warning "Failed to match 'pkg list' line #{line.inspect}"
return nil
end
@@ -58,8 +52,8 @@ def self.parse_line(line)
# TODO deal with multiple publishers
def latest
version = nil
- pkg(:list, "-Ha", @resource[:name]).split("\n").each do |line|
- v = line.split[2]
+ pkg(:list, "-Ha", @resource[:name]).each_line do |line|
+ v = parse_line(line.chomp)[:status]
case v
when "known"
return v
@@ -93,15 +87,10 @@ def query
output = pkg(:list, "-H", @resource[:name])
rescue Puppet::ExecutionFailure
# pkg returns 1 if the package is not found.
- return {:ensure => :absent, :status => 'missing',
- :name => @resource[:name], :error => 'ok'}
+ return {:ensure => :absent, :name => @resource[:name]}
end
- hash = self.class.parse_line(output) ||
- {:ensure => :absent, :status => 'missing', :name => @resource[:name], :error => 'ok'}
-
- raise Puppet::Error.new( "Package #{hash[:name]}, version #{hash[:version]} is in error state: #{hash[:error]}") if hash[:error] != "ok"
-
+ hash = self.class.parse_line(output.chomp) || {:ensure => :absent, :name => @resource[:name]}
hash
end
end
diff --git a/spec/fixtures/unit/provider/package/pkg/dummy b/spec/fixtures/unit/provider/package/pkg/dummy
new file mode 100644
index 0000000..2d56c39
--- /dev/null
+++ b/spec/fixtures/unit/provider/package/pkg/dummy
@@ -0,0 +1 @@
+dummy 2.5.5-0.111 installed ----
diff --git a/spec/fixtures/unit/provider/package/pkg/incomplete b/spec/fixtures/unit/provider/package/pkg/incomplete
new file mode 100644
index 0000000..24a93d1
--- /dev/null
+++ b/spec/fixtures/unit/provider/package/pkg/incomplete
@@ -0,0 +1 @@
+dummy 2.5.5-0.111 installed ---- RANDOM_TRASH
diff --git a/spec/fixtures/unit/provider/package/pkg/publisher b/spec/fixtures/unit/provider/package/pkg/publisher
new file mode 100644
index 0000000..23be5b6
--- /dev/null
+++ b/spec/fixtures/unit/provider/package/pkg/publisher
@@ -0,0 +1,2 @@
+SUNWpcre (solaris) 8.8-0.111 installed ----
+service/network/ssh (solaris) 0.5.11-0.151.0.1 installed -----
diff --git a/spec/fixtures/unit/provider/package/pkg/simple b/spec/fixtures/unit/provider/package/pkg/simple
new file mode 100644
index 0000000..80bb413
--- /dev/null
+++ b/spec/fixtures/unit/provider/package/pkg/simple
@@ -0,0 +1,4 @@
+SUNPython 2.5.5-0.111 installed ----
+SUNWbind 9.3.6.1-0.111 installed ----
+SUNWdistro-license-copyright 0.5.11-0.111 installed ----
+SUNWfppd 0.2008.8.18-0.111 installed ----
diff --git a/spec/unit/provider/package/pkg_spec.rb b/spec/unit/provider/package/pkg_spec.rb
index 04a4ae6..f5cbc6e 100755
--- a/spec/unit/provider/package/pkg_spec.rb
+++ b/spec/unit/provider/package/pkg_spec.rb
@@ -1,14 +1,10 @@
#!/usr/bin/env rspec
require 'spec_helper'
-provider = Puppet::Type.type(:package).provider(:pkg)
-
-describe provider do
- before do
- @resource = stub 'resource', :[] => "dummy"
- @provider = provider.new(@resource)
-
- @fakeresult = "install ok installed dummy 1.0\n"
+describe Puppet::Type.type(:package).provider(:pkg) do
+ before :each do
+ @resource = Puppet::Resource.new(:package, 'dummy', :parameters => {:name => 'dummy', :ensure => :latest})
+ @provider = described_class.new(@resource)
end
def self.it_should_respond_to(*actions)
@@ -21,8 +17,8 @@ def self.it_should_respond_to(*actions)
it_should_respond_to :install, :uninstall, :update, :query, :latest
- it "should be versionable" do
- provider.should_not be_versionable
+ it "should not be versionable" do
+ described_class.should_not be_versionable
end
it "should use :install to update" do
@@ -30,33 +26,60 @@ def self.it_should_respond_to(*actions)
@provider.update
end
- it "should parse a line correctly" do
- result = provider.parse_line("dummy [email protected] installed ----")
- result.should == {:name => "dummy", :version => "[email protected]",
- :ensure => :present, :status => "installed",
- :provider => :pkg, :error => "ok"}
- end
+ describe "when calling instances" do
+ it "should correctly parse lines with preferred publisher" do
+ described_class.expects(:pkg).with(:list,'-H').returns File.read(my_fixture('simple'))
+ @instances = described_class.instances.map { |p| Hash.new(:name => p.get(:name), :ensure => p.get(:ensure)) }
+ @instances.size.should == 4
+ @instances[0].should eql Hash.new(:name => 'SUNPython', :ensure => :present)
+ @instances[1].should eql Hash.new(:name => 'SUNWbind', :ensure => :present)
+ @instances[2].should eql Hash.new(:name => 'SUNWdistro-license-copyright', :ensure => :present)
+ @instances[3].should eql Hash.new(:name => 'SUNWfppd', :ensure => :present)
+ end
- it "should fail to parse an incorrect line" do
- result = provider.parse_line("foo")
- result.should be_nil
- end
+ it "should correctly parse lines with non preferred publisher" do
+ described_class.expects(:pkg).with(:list,'-H').returns File.read(my_fixture('publisher'))
+ @instances = described_class.instances.map { |p| Hash.new(:name => p.get(:name), :ensure => p.get(:ensure)) }
+ @instances.size.should == 2
+ @instances[0].should eql Hash.new(:name => 'SUNWpcre', :ensure => :present)
+ @instances[1].should eql Hash.new(:name => 'service/network/ssh', :ensure => :present)
+ end
- it "should fail to list a missing package" do
- @provider.expects(:pkg).with(:list, "-H", "dummy").returns "1"
- @provider.query.should == {:status=>"missing", :ensure=>:absent,
- :name=>"dummy", :error=>"ok"}
+ it "should warn about incorrect lines" do
+ fake_output = File.read(my_fixture('incomplete'))
+ error_line = fake_output.lines[0]
+ described_class.expects(:pkg).with(:list,'-H').returns fake_output
+ described_class.expects(:warning).with "Failed to match 'pkg list' line #{error_line.inspect}"
+ described_class.instances
+ end
end
- it "should fail to list a package when it can't parse the output line" do
- @provider.expects(:pkg).with(:list, "-H", "dummy").returns "failed"
- @provider.query.should == {:status=>"missing", :ensure=>:absent, :name=>"dummy", :error=>"ok"}
- end
+ describe "when query a package" do
+ it "should find the package" do
+ @provider.stubs(:pkg).with(:list,'-H','dummy').returns File.read(my_fixture('dummy'))
+ @provider.query.should == {
+ :name => 'dummy',
+ :ensure => :present,
+ :version => '2.5.5-0.111',
+ :status => "installed",
+ :provider => :pkg,
+ }
+ end
+
+ it "should return :absent when the package is not found" do
+ # I dont know what the acutal error looks like, but according to type/pkg.rb we're just
+ # reacting on the Exception anyways
+ @provider.expects(:pkg).with(:list, "-H", "dummy").raises Puppet::ExecutionFailure, 'Not found'
+ @provider.query.should == {:ensure => :absent, :name => "dummy"}
+ end
- it "should list package correctly" do
- @provider.expects(:pkg).with(:list, "-H", "dummy").returns "dummy [email protected] installed ----"
- @provider.query.should == {:name => "dummy", :version => "[email protected]",
- :ensure => :present, :status => "installed",
- :provider => :pkg, :error => "ok"}
+ it "should return :absent when the packageline cannot be parsed" do
+ @provider.stubs(:pkg).with(:list,'-H','dummy').returns File.read(my_fixture('incomplete'))
+ @provider.query.should == {
+ :name => 'dummy',
+ :ensure => :absent
+ }
+ end
end
+
end
-- 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.
