Please review pull request #681: (#14036) Handle upstart better opened by (jeffweiss)
Description:
Change the upstart provider to better handle the mix of some services
that are upstart controlled and some that are init script controlled.
Make the upstart provider the default for Ubuntu.
Prior to this commit, Ubuntu was patching puppet with a bastardized version of the init provider. As of this commit, puppet properly handles the lack of different return codes from upstart and can properly differentiate between a service that is init-script-based and a service that is an upstart job, and the provider will do the correct behaviour without the Ubuntu user no longer needing to explicitly specify provider => upstart or provider => debian.
- Opened: Wed Apr 18 21:01:22 UTC 2012
- Based on: puppetlabs:2.7.x (3a4ac604c85952511fd45c2f0699ce956eda3773)
- Requested merge: jeffweiss:ticket/2.7.x/14036_handle_upstart_better (c2554142d705d9177eadb6f76af0b066b104e93e)
Diff follows:
diff --git a/lib/puppet/provider/service/upstart.rb b/lib/puppet/provider/service/upstart.rb
index 4551204..7892bec 100755
--- a/lib/puppet/provider/service/upstart.rb
+++ b/lib/puppet/provider/service/upstart.rb
@@ -1,4 +1,4 @@
-Puppet::Type.type(:service).provide :upstart, :parent => :init do
+Puppet::Type.type(:service).provide :upstart, :parent => :debian do
desc "Ubuntu service management with `upstart`.
This provider manages `upstart` jobs, which have replaced `initd` services
@@ -6,6 +6,8 @@
"
# 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",
:stop => "/sbin/stop",
@@ -39,33 +41,47 @@ def self.instances
end
def startcmd
- [command(:start), @resource[:name]]
+ is_upstart? ? [command(:start), @resource[:name]] : super
end
def stopcmd
- [command(:stop), @resource[:name]]
+ is_upstart? ? [command(:stop), @resource[:name]] : super
end
def restartcmd
- (@resource[:hasrestart] == :true) && [command(:restart), @resource[:name]]
+ is_upstart? ? (@resource[:hasrestart] == :true) && [command(:restart), @resource[:name]] : super
end
+ def statuscmd
+ is_upstart? ? nil : super #this is because upstart is broken with its return codes
+ end
+
def status
- # allows user override of status command
if @resource[:status]
- ucommand(:status, false)
- if $?.exitstatus == 0
- return :running
- else
- return :stopped
- end
+ is_upstart?(@resource[:status]) ? upstart_status(@resource[:status]) : normal_status
+ elsif is_upstart?
+ upstart_status
else
- output = status_exec(@resource[:name].split)
- if (! $?.nil?) && (output =~ /start\//)
- return :running
- else
- return :stopped
- end
+ 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\//)
+ return :running
+ else
+ return :stopped
+ end
+ end
+
+ def is_upstart?(script = initscript)
+ File.symlink?(script) && File.readlink(script) == "/lib/init/upstart-job"
+ end
+
end
diff --git a/spec/unit/provider/service/upstart_spec.rb b/spec/unit/provider/service/upstart_spec.rb
index 259eb2c..27983f6 100755
--- a/spec/unit/provider/service/upstart_spec.rb
+++ b/spec/unit/provider/service/upstart_spec.rb
@@ -34,6 +34,7 @@
it "should use the default status command if none is specified" do
resource = Puppet::Type.type(:service).new(:name => "foo", :provider => :upstart)
provider = provider_class.new(resource)
+ provider.stubs(:is_upstart?).returns(true)
provider.expects(:status_exec).with(["foo"]).returns("foo start/running, process 1000")
Process::Status.any_instance.stubs(:exitstatus).returns(0)
@@ -43,10 +44,47 @@
it "should properly handle services with 'start' in their name" do
resource = Puppet::Type.type(:service).new(:name => "foostartbar", :provider => :upstart)
provider = provider_class.new(resource)
+ provider.stubs(:is_upstart?).returns(true)
provider.expects(:status_exec).with(["foostartbar"]).returns("foostartbar stop/waiting")
Process::Status.any_instance.stubs(:exitstatus).returns(0)
provider.status.should == :stopped
+ 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)
+ end
+ ["start", "stop"].each do |command|
+ it "should return the #{command}cmd of its parent provider" do
+ provider.send("#{command}cmd".to_sym).should == [provider.command(command.to_sym), resource.name]
+ end
+ end
+ it "should return nil for the statuscmd" do
+ provider.statuscmd.should be_nil
+ end
+ end
+
+ describe "when init script" do
+ before(:each) do
+ provider.stubs(:is_upstart?).returns(false)
+ end
+ ["start", "stop", "status"].each do |command|
+ it "should return the #{command}cmd of its parent provider" do
+ provider.expects(:search).with('foo').returns("/etc/init.d/foo")
+ provider.send("#{command}cmd".to_sym).should == ["/etc/init.d/foo", command.to_sym]
+ end
+ 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.
