On Aug 14, 2009, at 1:45 PM, Nigel Kersten wrote: > > > Signed-off-by: Nigel Kersten <[email protected]> > --- > lib/puppet/provider/service/debian.rb | 33 ++++++++----- > spec/unit/provider/service/debian.rb | 83 ++++++++++++++++++++++++ > +++++++++ > 2 files changed, 103 insertions(+), 13 deletions(-) > create mode 100755 spec/unit/provider/service/debian.rb > > diff --git a/lib/puppet/provider/service/debian.rb b/lib/puppet/ > provider/service/debian.rb > index 8527ae0..658b463 100755 > --- a/lib/puppet/provider/service/debian.rb > +++ b/lib/puppet/provider/service/debian.rb > @@ -4,11 +4,16 @@ > Puppet::Type.type(:service).provide :debian, :parent => :init do > desc "Debian's form of ``init``-style management. > > The only difference is that this supports service enabling and > disabling > - via ``update-rc.d``. > + via ``update-rc.d`` and determines enabled status via ``invoke- > rc.d``.
This is a behaviour change and should be marked so on the ticket. No idea how many people it will affect. > > " > > - commands :update => "/usr/sbin/update-rc.d" > + commands :update_rc => "/usr/sbin/update-rc.d" > + # note this isn't being used as a command until > + # http://projects.reductivelabs.com/issues/2538 > + # is resolved. > + commands :invoke_rc => "/usr/sbin/invoke-rc.d" > + > defaultfor :operatingsystem => [:debian, :ubuntu] > > def self.defpath > @@ -17,24 +22,26 @@ > Puppet::Type.type(:service).provide :debian, :parent => :init do > > # Remove the symlinks > def disable > - update "-f", @resource[:name], "remove" > - update @resource[:name], "stop", "00", "1", "2", "3", "4", > "5", "6", "." > + update_rc "-f", @resource[:name], "remove" > + update_rc @resource[:name], "stop", "00", "1", "2", "3", > "4", "5", "6", "." > end > > def enabled? > - output = update "-n", "-f", @resource[:name], "remove" > - > - # If it's enabled, then it will print output showing > removal of > - # links. > - if output =~ /etc\/rc[\dS].d\/S|not installed/ > - return :true > + # TODO: Replace system() call when Puppet::Util.execute > gives us a way > + # to determine exit status. > http://projects.reductivelabs.com/issues/2538 > + system("/usr/sbin/invoke-rc.d --query #...@resource[:name]} > start") Even if you're using system(), you should pass it an array. That way you get a direct exec instead of a shell interpretation. With this code, someone could have 'foo; rm -rf /etc; echo' as the service name and it would do what you don't want. Not that we never make this mistake in the code, but I try to make it as rarely as possible (and it's why execute() prefers an array). > > + # 104 is the exit status when you query start an enabled > service. > + # See x-man-page://invoke-rc.d > + if $?.exitstatus == 104 > + return :true > else > - return :false > + return :false > end > end > > def enable > - update "-f", @resource[:name], "remove" > - update @resource[:name], "defaults" > + update_rc "-f", @resource[:name], "remove" > + update_rc @resource[:name], "defaults" > end > end > diff --git a/spec/unit/provider/service/debian.rb b/spec/unit/ > provider/service/debian.rb > new file mode 100755 > index 0000000..0c31c01 > --- /dev/null > +++ b/spec/unit/provider/service/debian.rb > @@ -0,0 +1,83 @@ > +#!/usr/bin/env ruby > +# > +# Unit testing for the debian service provider > +# > + > +require File.dirname(__FILE__) + '/../../../spec_helper' > + > +provider_class = Puppet::Type.type(:service).provider(:debian) > + > +describe provider_class do > + > + before(:each) do > + # Create a mock resource > + @resource = stub 'resource' > + > + @provider = provider_class.new > + > + # A catch all; no parameters set > + @resource.stubs(:[]).returns(nil) > + > + # But set name, source and path > + @resource.stubs(:[]).with(:name).returns "myservice" > + @resource.stubs(:[]).with(:ensure).returns :enabled > + @resource.stubs(:ref).returns "Service[myservice]" > + > + @provider.resource = @resource > + > + @provider.stubs(:command).with(:update_rc).returns > "update_rc" > + @provider.stubs(:command).with(:invoke_rc).returns > "invoke_rc" > + > + @provider.stubs(:update_rc) > + @provider.stubs(:invoke_rc) > + end > + > + it "should have an enabled? method" do > + @provider.should respond_to(:enabled?) > + end > + > + it "should have an enable method" do > + @provider.should respond_to(:enable) > + end > + > + it "should have a disable method" do > + @provider.should respond_to(:disable) > + end > + > + describe "when enabling" do > + it "should call update-rc.d twice" do > + @provider.expects(:update_rc).twice > + @provider.enable > + end > + end > + > + describe "when disabling" do > + it "should call update-rc.d twice" do > + @provider.expects(:update_rc).twice > + @provider.disable > + end > + end These aren't the most meaningful of tests, but I'd at least just say 'it should use update-rc.d' rather than 'it should call it twice'. > > + describe "when checking whether it is enabled" do > + it "should call Kernel.system() with the appropriate > parameters" do > + @provider.expects(:system).with("/usr/sbin/invoke-rc.d > --query #...@resource[:name]} start").once > + @provider.enabled? > + end > + > + it "should return true when invoke-rc.d exits with 104 > status" do > + @provider.stubs(:system) > + $?.stubs(:exitstatus).returns(104) > + @provider.enabled?.should == :true > + end > + > + # pick a range of non-104 numbers, strings and booleans to > test with. > + [-100, -1, 0, 1, 100, "foo", "", :true, :false].each do | > exitstatus| > + it "should return false when invoke-rc.d exits with > #{exitstatus} status" do > + @provider.stubs(:system) > + $?.stubs(:exitstatus).returns(exitstatus) > + @provider.enabled?.should == :false > + end > + end > + end > + > + end > -- > 1.6.4 > > > > -- A censor is a man who knows more than he thinks you ought to. -- Granville Hicks --------------------------------------------------------------------- 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 -~----------~----~----~----~------~----~------~--~---
