(PUP-1353) abstract away the priority setting

Author: lollipopman

lib/puppet/provider/service/service.rb

@@ -24,13 +24,12 @@ def texecute(type, command, fof = true, squelch = false, combine = true)
     # Set the process priority to 0 (or normal in Windows) so that services
     # which are started as children of puppet will start with normal priority,
     # rather than the priority of the puppet process itself.
-    priority = Puppet::Util::Platform.windows? ? Process::NORMAL_PRIORITY_CLASS : 0
     begin
       opts = {
         :combine => combine,
         :failonfail => fof,
         :override_locale => false,
-        :priority => priority,
+        :priority => :normal,
         :squelch => squelch,
       }
       execute(command, opts)

lib/puppet/util/execution.rb

@@ -134,7 +134,7 @@ def self.execfail(command, exception)
   #   Passing in a value of false for this option will allow the command to be executed using the user/system locale.
   # @option options [Hash<{String => String}>] :custom_environment ({}) a hash of key/value pairs to set as environment variables for the duration
   #   of the command.
-  # @option options [Integer] :priority (nil) The priority of child process, aka the nice value in Posix operatingsystems.
+  # @option options [Symbol] :priority (nil) The priority of child process, see Puppet::Settings::PrioritySetting::PRIORITY_MAP for values.
   # @return [Puppet::Util::Execution::ProcessOutput] output as specified by options
   # @raise [Puppet::ExecutionFailure] if the executed chiled process did not exit with status == 0 and `failonfail` is
   #   `true`.
@@ -341,7 +341,7 @@ def self.execute_posix(command, options, stdin, stdout, stderr)
       Process.setsid
       if options[:priority]
         if Puppet.features.root?
-          setpriority(options[:priority])
+          setpriority(Puppet::Settings::PrioritySetting::PRIORITY_MAP[options[:priority]])
         end
       end
       begin

lib/puppet/util/windows/process.rb

@@ -24,9 +24,13 @@ def execute(command, arguments, stdin, stdout, stderr)
         :stderr => stderr
       },
       :close_handles => false,
+      :creation_flags => 0,
     }
     if arguments[:suppress_window]
-      create_args[:creation_flags] = CREATE_NO_WINDOW
+      create_args[:creation_flags] |= CREATE_NO_WINDOW
+    end
+    if arguments[:priority]
+      create_args[:creation_flags] |= Puppet::Settings::PrioritySetting::PRIORITY_MAP[arguments[:priority]]
     end
     if arguments[:cwd]
       create_args[:cwd] = arguments[:cwd]

spec/unit/provider/service/bsd_spec.rb

@@ -94,13 +94,13 @@
 
     it "should use the supplied start command if specified" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :start => '/bin/foo'))
-      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       provider.start
     end
 
     it "should start the serviced directly otherwise" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd'))
-      expect(provider).to receive(:execute).with(['/etc/rc.d/sshd', :onestart], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/etc/rc.d/sshd', :onestart], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       expect(provider).to receive(:search).with('sshd').and_return('/etc/rc.d/sshd')
       provider.start
     end
@@ -113,13 +113,13 @@
 
     it "should use the supplied stop command if specified" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :stop => '/bin/foo'))
-      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       provider.stop
     end
 
     it "should stop the serviced directly otherwise" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd'))
-      expect(provider).to receive(:execute).with(['/etc/rc.d/sshd', :onestop], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/etc/rc.d/sshd', :onestop], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       expect(provider).to receive(:search).with('sshd').and_return('/etc/rc.d/sshd')
       provider.stop
     end

spec/unit/provider/service/gentoo_spec.rb

@@ -79,13 +79,13 @@
   describe "#start" do
     it "should use the supplied start command if specified" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :start => '/bin/foo'))
-      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       provider.start
     end
 
     it "should start the service with <initscript> start otherwise" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd'))
-      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:start], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:start], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       expect(provider).to receive(:search).with('sshd').and_return('/etc/init.d/sshd')
       provider.start
     end
@@ -94,13 +94,13 @@
   describe "#stop" do
     it "should use the supplied stop command if specified" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :stop => '/bin/foo'))
-      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       provider.stop
     end
 
     it "should stop the service with <initscript> stop otherwise" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd'))
-      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:stop], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:stop], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       expect(provider).to receive(:search).with('sshd').and_return('/etc/init.d/sshd')
       provider.stop
     end
@@ -160,24 +160,24 @@
     describe "when a special status command is specified" do
       it "should use the status command from the resource" do
         provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :status => '/bin/foo'))
-        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
-        expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
+        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
+        expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
         allow($CHILD_STATUS).to receive(:exitstatus).and_return(0)
         provider.status
       end
 
       it "should return :stopped when the status command returns with a non-zero exitcode" do
         provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :status => '/bin/foo'))
-        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
-        expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
+        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
+        expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
         allow($CHILD_STATUS).to receive(:exitstatus).and_return(3)
         expect(provider.status).to eq(:stopped)
       end
 
       it "should return :running when the status command returns with a zero exitcode" do
         provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :status => '/bin/foo'))
-        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
-        expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
+        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
+        expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
         allow($CHILD_STATUS).to receive(:exitstatus).and_return(0)
         expect(provider.status).to eq(:running)
       end
@@ -186,14 +186,14 @@
     describe "when hasstatus is false" do
       it "should return running if a pid can be found" do
         provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :hasstatus => false))
-        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
+        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
         expect(provider).to receive(:getpid).and_return(1000)
         expect(provider.status).to eq(:running)
       end
 
       it "should return stopped if no pid can be found" do
         provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :hasstatus => false))
-        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
+        expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
         expect(provider).to receive(:getpid).and_return(nil)
         expect(provider.status).to eq(:stopped)
       end
@@ -203,15 +203,15 @@
       it "should return running if <initscript> status exits with a zero exitcode" do
         provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :hasstatus => true))
         expect(provider).to receive(:search).with('sshd').and_return('/etc/init.d/sshd')
-        expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
+        expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
         allow($CHILD_STATUS).to receive(:exitstatus).and_return(0)
         expect(provider.status).to eq(:running)
       end
 
       it "should return stopped if <initscript> status exits with a non-zero exitcode" do
         provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :hasstatus => true))
         expect(provider).to receive(:search).with('sshd').and_return('/etc/init.d/sshd')
-        expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => 0, :squelch => false)
+        expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:status], :combine => true, :failonfail => false, :override_locale => false, :priority => :normal, :squelch => false)
         allow($CHILD_STATUS).to receive(:exitstatus).and_return(3)
         expect(provider.status).to eq(:stopped)
       end
@@ -221,24 +221,24 @@
   describe "#restart" do
     it "should use the supplied restart command if specified" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :restart => '/bin/foo'))
-      expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:restart], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
-      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:restart], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
+      expect(provider).to receive(:execute).with(['/bin/foo'], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       provider.restart
     end
 
     it "should restart the service with <initscript> restart if hasrestart is true" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :hasrestart => true))
       expect(provider).to receive(:search).with('sshd').and_return('/etc/init.d/sshd')
-      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:restart], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:restart], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       provider.restart
     end
 
     it "should restart the service with <initscript> stop/start if hasrestart is false" do
       provider = provider_class.new(Puppet::Type.type(:service).new(:name => 'sshd', :hasrestart => false))
       expect(provider).to receive(:search).with('sshd').and_return('/etc/init.d/sshd')
-      expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:restart], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
-      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:stop], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
-      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:start], :combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false)
+      expect(provider).not_to receive(:execute).with(['/etc/init.d/sshd',:restart], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
+      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:stop], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
+      expect(provider).to receive(:execute).with(['/etc/init.d/sshd',:start], :combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false)
       provider.restart
     end
   end

spec/unit/provider/service/init_spec.rb

@@ -242,7 +242,7 @@
     describe "when starting a service on Solaris" do
       it "should use ctrun" do
         allow(Facter).to receive(:value).with(:osfamily).and_return('Solaris')
-        expect(provider).to receive(:execute).with('/usr/bin/ctrun -l child /service/path/myservice start', {:combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false}).and_return("")
+        expect(provider).to receive(:execute).with('/usr/bin/ctrun -l child /service/path/myservice start', {:combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false}).and_return("")
         allow($CHILD_STATUS).to receive(:exitstatus).and_return(0)
         provider.start
       end
@@ -251,7 +251,7 @@
     describe "when starting a service on RedHat" do
       it "should not use ctrun" do
         allow(Facter).to receive(:value).with(:osfamily).and_return('RedHat')
-        expect(provider).to receive(:execute).with(['/service/path/myservice', :start], {:combine => true, :failonfail => true, :override_locale => false, :priority => 0, :squelch => false}).and_return("")
+        expect(provider).to receive(:execute).with(['/service/path/myservice', :start], {:combine => true, :failonfail => true, :override_locale => false, :priority => :normal, :squelch => false}).and_return("")
         allow($CHILD_STATUS).to receive(:exitstatus).and_return(0)
         provider.start
       end