Please review pull request #742: (#14297) handle upstart services better opened by (haus)
Description:
This pull includes 2 commits, one to update the upstart provider to handle upstart services better and another to update the spec tests of the init and upstart providers.
- Opened: Mon May 07 21:30:26 UTC 2012
- Based on: puppetlabs:2.7.x (b245f58e0e3f4672af95e063703c88c6c3b5306f)
- Requested merge: haus:ticket/2.7.x/14297_handle_upstart_better (ab01ac6bda9165ceff1ed9ec1a8e86c3c2fcb78d)
Diff follows:
diff --git a/lib/puppet/provider/service/init.rb b/lib/puppet/provider/service/init.rb
index 2a54db2..230c279 100755
--- a/lib/puppet/provider/service/init.rb
+++ b/lib/puppet/provider/service/init.rb
@@ -15,7 +15,7 @@ class << self
when "Archlinux"
@defpath = "/etc/rc.d"
else
- @defpath = "/etc/init.d"
+ @defpath = ["/etc/init.d", "/etc/init"]
end
# We can't confine this here, because the init path can be overridden.
@@ -81,33 +81,22 @@ def paths
end
def search(name)
- paths.each { |path|
- fqname = File.join(path,name)
- begin
- stat = File.stat(fqname)
- rescue
- # should probably rescue specific errors...
- self.debug("Could not find #{name} in #{path}")
- next
- end
+ ["", ".sh", ".conf"].each do |suffix|
+ paths.each { |path|
+ fqname = File.join(path,name+suffix)
+ begin
+ stat = File.stat(fqname)
+ rescue
+ # should probably rescue specific errors...
+ self.debug("Could not find #{name}#{suffix} in #{path}")
+ next
+ end
- # if we've gotten this far, we found a valid script
- return fqname
- }
-
- paths.each { |path|
- fqname_sh = File.join(path,"#{name}.sh")
- begin
- stat = File.stat(fqname_sh)
- rescue
- # should probably rescue specific errors...
- self.debug("Could not find #{name}.sh in #{path}")
- next
- end
+ # if we've gotten this far, we found a valid script
+ return fqname
+ }
+ end
- # if we've gotten this far, we found a valid script
- return fqname_sh
- }
raise Puppet::Error, "Could not find init script for '#{name}'"
end
diff --git a/lib/puppet/provider/service/upstart.rb b/lib/puppet/provider/service/upstart.rb
index 7892bec..83537ed 100755
--- a/lib/puppet/provider/service/upstart.rb
+++ b/lib/puppet/provider/service/upstart.rb
@@ -6,7 +6,7 @@
"
# confine to :ubuntu for now because I haven't tested on other platforms
confine :operatingsystem => :ubuntu #[:ubuntu, :fedora, :debian]
-
+
defaultfor :operatingsystem => :ubuntu
commands :start => "/sbin/start",
@@ -17,7 +17,7 @@
# upstart developer haven't implemented initctl enable/disable yet:
# http://www.linuxplanet.com/linuxplanet/tutorials/7033/2/
- # has_feature :enableable
+ has_feature :enableable
def self.instances
instances = []
@@ -40,6 +40,56 @@ def self.instances
instances
end
+ def self.defpath
+ superclass.defpath
+ end
+
+ def enabled?
+ if is_upstart?
+ unless File.read(File.join("/etc/init",@resource[:name]+".conf")).grep(/^\s*start\s*on/).empty?
+ return :true
+ else
+ return :false
+ end
+ else
+ super
+ end
+ end
+
+ def enable
+ if is_upstart?
+ script = File.join("/etc/init", @resource[:name]+".conf")
+
+ script_text = File.open(script).read
+
+ # If there is no "start on" it isn't enabled and needs that line added
+ if script_text.grep(/^\s*#?\s*start\s* on/).empty?
+ enabled_script = script_text + "\nstart on runlevel [2,3,4,5]"
+ else
+ enabled_script = script_text.gsub(/^#start on/, "start on")
+ end
+
+ fh = File.open(script, 'w')
+ fh.write(enabled_script)
+ fh.close
+
+ else
+ super
+ end
+ end
+
+ def disable
+ if is_upstart?
+ script = File.join("/etc/init", @resource[:name]+".conf")
+ disabled_script = File.open(script).read.gsub(/^\s*start\s*on/, "#start on")
+ fh = File.open(script, 'w')
+ fh.write(disabled_script)
+ fh.close
+ else
+ super
+ end
+ end
+
def startcmd
is_upstart? ? [command(:start), @resource[:name]] : super
end
@@ -55,7 +105,7 @@ def restartcmd
def statuscmd
is_upstart? ? nil : super #this is because upstart is broken with its return codes
end
-
+
def status
if @resource[:status]
is_upstart?(@resource[:status]) ? upstart_status(@resource[:status]) : normal_status
@@ -65,12 +115,12 @@ def status
super
end
end
-
+
def normal_status
ucommand(:status, false)
($?.exitstatus == 0) ? :running : :stopped
end
-
+
def upstart_status(exec = @resource[:name])
output = status_exec(@resource[:name].split)
if (! $?.nil?) && (output =~ /start\//)
@@ -79,9 +129,9 @@ def upstart_status(exec = @resource[:name])
return :stopped
end
end
-
+
def is_upstart?(script = initscript)
- File.symlink?(script) && File.readlink(script) == "/lib/init/upstart-job"
+ (File.symlink?(script) && File.readlink(script) == "/lib/init/upstart-job") || (File.file?(script) && (not script.include?("init.d")))
end
end
diff --git a/spec/integration/provider/service/init_spec.rb b/spec/integration/provider/service/init_spec.rb
index 483507d..be7fdae 100755
--- a/spec/integration/provider/service/init_spec.rb
+++ b/spec/integration/provider/service/init_spec.rb
@@ -24,7 +24,7 @@
describe "when not running on FreeBSD, HP-UX or Archlinux", :if => (! %w{HP-UX FreeBSD Archlinux}.include?(Facter.value(:operatingsystem))) do
it "should set its default path to include /etc/init.d" do
- provider.defpath.should == "/etc/init.d"
+ provider.defpath.should == ["/etc/init.d", "/etc/init"]
end
end
end
diff --git a/spec/unit/provider/service/upstart_spec.rb b/spec/unit/provider/service/upstart_spec.rb
index 27983f6..3210fee 100755
--- a/spec/unit/provider/service/upstart_spec.rb
+++ b/spec/unit/provider/service/upstart_spec.rb
@@ -49,17 +49,17 @@
provider.expects(:status_exec).with(["foostartbar"]).returns("foostartbar stop/waiting")
Process::Status.any_instance.stubs(:exitstatus).returns(0)
provider.status.should == :stopped
- end
+ end
end
describe "inheritance" do
let :resource do
resource = Puppet::Type.type(:service).new(:name => "foo", :provider => :upstart)
end
-
+
let :provider do
provider = provider_class.new(resource)
end
-
+
describe "when upstart job" do
before(:each) do
provider.stubs(:is_upstart?).returns(true)
@@ -73,7 +73,7 @@
provider.statuscmd.should be_nil
end
end
-
+
describe "when init script" do
before(:each) do
provider.stubs(:is_upstart?).returns(false)
@@ -85,6 +85,102 @@
end
end
end
+ end
+
+ describe "should be enableable" do
+ let :resource do
+ resource = Puppet::Type.type(:service).new(:name => "foo", :provider => :upstart)
+ end
+
+ let :provider do
+ provider = provider_class.new(resource)
+ end
+
+ before(:each) do
+ provider.stubs(:is_upstart?).returns(true)
+ end
+
+ [:enabled?,:enable,:disable].each do |enableable|
+ it "should respond to #{enableable}" do
+ provider.should respond_to(enableable)
+ end
+ end
+ describe "when enabling" do
+ it "should open and uncomment the '#start on' line" do
+ file = stub 'file'
+ File.expects(:join).with("/etc/init", "foo.conf")
+ File.stubs(:open).returns file
+ file.stubs(:read).returns "#start on bar"
+ file.expects(:write).with("start on bar")
+ file.expects(:close)
+ provider.enable
+ end
+
+ it "should add a 'start on' line if none exists" do
+ file = stub 'file'
+ File.expects(:join).with("/etc/init", "foo.conf")
+ File.stubs(:open).returns file
+ file.stubs(:read).returns "a line without leading start on"
+ file.expects(:write).with("a line without leading start on\nstart on runlevel [2,3,4,5]")
+ file.expects(:close)
+ provider.enable
+ end
+
+ it "should do nothing if already enabled" do
+ file = stub 'file'
+ File.expects(:join).with("/etc/init", "foo.conf")
+ File.stubs(:open).returns file
+ file.stubs(:read).returns "start on bar"
+ file.expects(:write).with("start on bar")
+ file.expects(:close)
+ provider.enable
+ end
+ end
+
+ describe "when disabling" do
+ it "should open and comment the 'start on' line" do
+ file = stub 'file'
+ File.expects(:join).with("/etc/init", "foo.conf")
+ File.stubs(:open).returns file
+ file.stubs(:read).returns "start on bar"
+ file.expects(:write).with("#start on bar")
+ file.expects(:close)
+ provider.disable
+ end
+
+ it "should do nothing if already disabled" do
+ file = stub 'file'
+ File.expects(:join).with("/etc/init", "foo.conf")
+ File.stubs(:open).returns file
+ file.stubs(:read).returns "#start on bar"
+ file.expects(:write).with("#start on bar")
+ file.expects(:close)
+ provider.disable
+ end
+ end
+
+ describe "when checking whether it is enabled" do
+ it "should consider 'start on ...' to be enabled" do
+ file = stub 'file'
+ File.expects(:join).with("/etc/init", "foo.conf")
+ File.stubs(:read).returns "start on bar"
+ provider.enabled?.should == :true
+ end
+
+ it "should consider '#start on ...' to be disabled" do
+ file = stub 'file'
+ File.expects(:join).with("/etc/init", "foo.conf")
+ File.stubs(:read).returns "#start on bar"
+ provider.enabled?.should == :false
+ end
+
+ it "should consider no start on line to be disabled" do
+ file = stub 'file'
+ File.expects(:join).with("/etc/init", "foo.conf")
+ File.stubs(:read).returns "just a line with some text"
+ provider.enabled?.should == :false
+ end
+ 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.
