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 (c35d8114dbafd88a2ad3bc3a9ed272209b41d0cf)
Diff follows:
diff --git a/lib/puppet/provider/service/init.rb b/lib/puppet/provider/service/init.rb
index 2a54db2..fc07225 100755
--- a/lib/puppet/provider/service/init.rb
+++ b/lib/puppet/provider/service/init.rb
@@ -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"].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..f19d4b8 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,111 @@ def self.instances
instances
end
+ def self.defpath
+ ["/etc/init.d", "/etc/init"]
+ end
+
+ def search(name)
+ # Search prefers .conf as that is what upstart uses
+ [".conf", "", ".sh"].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
+ }
+ end
+
+ raise Puppet::Error, "Could not find init script or upstart conf file for '#{name}'"
+ end
+
+ def enabled?
+ if is_upstart?
+ if File.open(initscript).read.match(/^\s*start\s+on/)
+ return :true
+ else
+ return :false
+ end
+ else
+ super
+ end
+ end
+
+ def enable
+ if is_upstart?
+ # Parens is needed to match parens in a multiline upstart start on stanza
+ parens = 0
+
+ script_text = File.open(initscript).read
+ enabled_script =
+ # Two cases, either there is a start on line already or we need to add one
+ if script_text.to_s.match(/^\s*#*\s*start\s+on/)
+ script_text.map do |line|
+ if line.match(/^\s*#+\s*start\s+on/)
+ # If there are more opening parens than closing parens, we need to uncomment a multiline 'start on' stanzas.
+ if (line.count('(') > line.count(')') )
+ parens = line.count('(') - line.count(')')
+ end
+ line.gsub(/^(\s*)#+(\s*start\s+on)/, '\1\2')
+ elsif parens > 0
+ # If there are still more opening than closing parens we need to continue uncommenting lines
+ parens += (line.count('(') - line.count(')') )
+ line.gsub(/^(\s*)#+/, '\1')
+ else
+ line
+ end
+ end
+ else
+ # If there is no "start on" it isn't enabled and needs that line added
+ script_text.to_s + "\nstart on runlevel [2,3,4,5]"
+ end
+
+ Puppet::Util.replace_file(initscript, 0644) do |file|
+ file.write(enabled_script)
+ end
+
+ else
+ super
+ end
+ end
+
+ def disable
+ if is_upstart?
+ # Parens is needed to match parens in a multiline upstart start on stanza
+ parens = 0
+ script_text = File.open(initscript).read
+
+ disabled_script = script_text.map do |line|
+ if line.match(/^\s*start\s+on/)
+ # If there are more opening parens than closing parens, we need to comment out a multiline 'start on' stanza
+ if (line.count('(') > line.count(')') )
+ parens = line.count('(') - line.count(')')
+ end
+ line.gsub(/^(\s*start\s+on)/, '#\1')
+ elsif parens > 0
+ # If there are still more opening than closing parens we need to continue uncommenting lines
+ parens += (line.count('(') - line.count(')') )
+ "#" << line
+ else
+ line
+ end
+ end
+
+ Puppet::Util.replace_file(initscript, 0644) do |file|
+ file.write(disabled_script)
+ end
+ else
+ super
+ end
+ end
+
def startcmd
is_upstart? ? [command(:start), @resource[:name]] : super
end
@@ -55,7 +160,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 +170,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 +184,11 @@ 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"
+ return true if (File.symlink?(script) && File.readlink(script) == "/lib/init/upstart-job")
+ return true if (File.file?(script) && (not script.include?("init.d")))
+ return false
end
end
diff --git a/spec/unit/provider/service/upstart_spec.rb b/spec/unit/provider/service/upstart_spec.rb
index 27983f6..2e3c20c 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,141 @@
end
end
end
+ end
+
+ describe "should be enableable" do
+ let :resource do
+ Puppet::Type.type(:service).new(:name => "foo", :provider => :upstart)
+ end
+
+ let :provider do
+ provider_class.new(resource)
+ end
+
+ let :init_script do
+ PuppetSpec::Files.tmpfile("foo.conf")
+ end
+
+ let :disabled_content do
+ "\t # \t start on\nother file stuff"
+ end
+
+ let :multiline_disabled do
+ "# \t start on other file stuff (\n" +
+ "# more stuff (\n" +
+ "# finishing up )\n" +
+ "# and done )\n" +
+ "this line shouldn't be touched\n"
+ end
+
+ let :multiline_enabled do
+ " \t start on other file stuff (\n" +
+ " more stuff (\n" +
+ " finishing up )\n" +
+ " and done )\n" +
+ "this line shouldn't be touched\n"
+ end
+
+ let :enabled_content do
+ "\t \t start on\nother file stuff"
+ end
+
+ let :content do
+ "just some text"
+ end
+
+ before(:each) do
+ provider.stubs(:is_upstart?).returns(true)
+ provider.stubs(:search).returns(init_script)
+ 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 = File.open(init_script, 'w')
+ file.write(disabled_content)
+ file.close
+ provider.enable
+ File.open(init_script).read.should == enabled_content
+ end
+
+ it "should add a 'start on' line if none exists" do
+ file = File.open(init_script, 'w')
+ file.write("this is a file")
+ file.close
+ provider.enable
+ File.open(init_script).read.should == "this is a file\nstart on runlevel [2,3,4,5]"
+ end
+
+ it "should do nothing if already enabled" do
+ file = File.open(init_script, 'w')
+ file.write(enabled_content)
+ file.close
+ provider.enable
+ File.open(init_script).read.should == enabled_content
+ end
+
+ it "should handle multiline 'start on' stanzas" do
+ file = File.open(init_script, 'w')
+ file.write(multiline_disabled)
+ file.close
+ provider.enable
+ File.open(init_script).read.should == multiline_enabled
+ end
+ end
+
+ describe "when disabling" do
+ it "should open and comment the 'start on' line" do
+ file = File.open(init_script, 'w')
+ file.write(enabled_content)
+ file.close
+ provider.disable
+ File.open(init_script).read.should == "#" + enabled_content
+ end
+
+ it "should do nothing if already disabled" do
+ file = File.open(init_script, 'w')
+ file.write(disabled_content)
+ file.close
+ provider.disable
+ File.open(init_script).read.should == disabled_content
+ end
+
+ it "should handle multiline 'start on' stanzas" do
+ file = File.open(init_script, 'w')
+ file.write(multiline_enabled)
+ file.close
+ provider.disable
+ File.open(init_script).read.should == multiline_disabled
+ end
+ end
+
+ describe "when checking whether it is enabled" do
+ it "should consider 'start on ...' to be enabled" do
+ file = File.open(init_script, 'w')
+ file.write(enabled_content)
+ file.close
+ provider.enabled?.should == :true
+ end
+
+ it "should consider '#start on ...' to be disabled" do
+ file = File.open(init_script, 'w')
+ file.write(disabled_content)
+ file.close
+ provider.enabled?.should == :false
+ end
+
+ it "should consider no start on line to be disabled" do
+ file = File.open(init_script, 'w')
+ file.write(content)
+ file.close
+ 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.
