diff options
| -rw-r--r-- | CHANGELOG | 3 | ||||
| -rw-r--r-- | examples/code/mac_automount.pp | 16 | ||||
| -rwxr-xr-x | lib/puppet/provider/service/base.rb | 6 | ||||
| -rw-r--r-- | lib/puppet/util.rb | 83 | ||||
| -rw-r--r-- | test/README | 15 | ||||
| -rwxr-xr-x | test/util/utiltest.rb | 30 |
6 files changed, 118 insertions, 35 deletions
@@ -1,3 +1,6 @@ + STDIN, STDOUT, STDERR are now redirected to /dev/null in + service providers descending from base. + Certificates are now valid starting one day before they are created, to help handle small amounts of clock skew. diff --git a/examples/code/mac_automount.pp b/examples/code/mac_automount.pp new file mode 100644 index 000000000..bab0136fc --- /dev/null +++ b/examples/code/mac_automount.pp @@ -0,0 +1,16 @@ +#!/usr/bin/env puppet +# Jeff McCune <mccune@math.ohio-state.edu> +# +# Apple's Automounter spawns a child that sends the parent +# a SIGTERM. This makes it *very* difficult to figure out +# if the process started correctly or not. +# + +service {"automount-test": + provider => base, + hasrestart => false, + pattern => '/tmp/hometest', + start => "/usr/sbin/automount -m /tmp/home /dev/null -mnt /tmp/hometest", + stop => "ps auxww | grep '/tmp/hometest' | grep -v grep | awk '{print \$2}' | xargs kill", + ensure => running +} diff --git a/lib/puppet/provider/service/base.rb b/lib/puppet/provider/service/base.rb index c615c30c0..c736c20c2 100755 --- a/lib/puppet/provider/service/base.rb +++ b/lib/puppet/provider/service/base.rb @@ -114,12 +114,12 @@ Puppet::Type.type(:service).provide :base do # A simple wrapper so execution failures are a bit more informative. def texecute(type, command, fof = true) begin - output = execute(command, fof) + # #565: Services generally produce no output, so squelch them. + execute(command, :failonfail => fof, :squelch => true) rescue Puppet::ExecutionFailure => detail @model.fail "Could not %s %s: %s" % [type, @model.ref, detail] end - - return output + return nil end # Use either a specified command or the default for our provider. diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb index 65c48fff9..8e07a27a1 100644 --- a/lib/puppet/util.rb +++ b/lib/puppet/util.rb @@ -267,7 +267,8 @@ module Util end # Execute the desired command, and return the status and output. - def execute(command, failonfail = true, uid = nil, gid = nil) + # def execute(command, failonfail = true, uid = nil, gid = nil) + def execute(command, arguments = {:failonfail => true}) if command.is_a?(Array) command = command.flatten.collect { |i| i.to_s } str = command.join(" ") @@ -284,30 +285,35 @@ module Util Puppet.debug "Executing '%s'" % str end - if uid - uid = Puppet::Util::SUIDManager.convert_xid(:uid, uid) + if arguments[:uid] + arguments[:uid] = Puppet::Util::SUIDManager.convert_xid(:uid, arguments[:uid]) end - if gid - gid = Puppet::Util::SUIDManager.convert_xid(:gid, gid) + if arguments[:gid] + arguments[:gid] = Puppet::Util::SUIDManager.convert_xid(:gid, arguments[:gid]) end @@os ||= Facter.value(:operatingsystem) output = nil - IO.popen("-") do |f| - if f - output = f.read + child_pid, child_status = nil + # The idea here is to avoid IO#read whenever possible. + if arguments[:squelch] + child_pid = Kernel.fork + if child_pid + # Parent process executes this + child_status = Process.waitpid2(child_pid)[1] else + # Child process executes this begin $stdin.reopen("/dev/null") - $stderr.close - $stderr = $stdout.dup - if gid - Process.egid = gid - Process.gid = gid unless @@os == "Darwin" + $stdout.reopen("/dev/null") + $stderr.reopen("/dev/null") + if arguments[:gid] + Process.egid = arguments[:gid] + Process.gid = arguments[:gid] unless @@os == "Darwin" end - if uid - Process.euid = uid - Process.uid = uid unless @@os == "Darwin" + if arguments[:uid] + Process.euid = arguments[:uid] + Process.uid = arguments[:uid] unless @@os == "Darwin" end if command.is_a?(Array) Kernel.exec(*command) @@ -317,13 +323,44 @@ module Util rescue => detail puts detail.to_s exit!(1) - end - end - end - - if failonfail - unless $? == 0 - raise ExecutionFailure, "Execution of '%s' returned %s: %s" % [str, $?.exitstatus, output] + end # begin; rescue + end # if child_pid; else + else + IO.popen("-") do |f| + if f + # Parent process executes this + output = f.read + else + # Parent process executes this + begin + $stdin.reopen("/dev/null") + $stderr.close + $stderr = $stdout.dup + if arguments[:gid] + Process.egid = arguments[:gid] + Process.gid = arguments[:gid] unless @@os == "Darwin" + end + if arguments[:uid] + Process.euid = arguments[:uid] + Process.uid = arguments[:uid] unless @@os == "Darwin" + end + if command.is_a?(Array) + Kernel.exec(*command) + else + Kernel.exec(command) + end + rescue => detail + puts detail.to_s + exit!(1) + end # begin; rescue + end # if f; else + end # IO.popen do |f| + child_status = $? + end # if arguments[:squelch]; else + + if arguments[:failonfail] + unless child_status == 0 + raise ExecutionFailure, "Execution of '%s' returned %s: %s" % [str, child_status.inspect, output] end end diff --git a/test/README b/test/README index 80d98b4bd..82a749a69 100644 --- a/test/README +++ b/test/README @@ -1,5 +1,15 @@ $Id$ +To run all tests, run: 'rake test'. To run an individual suite, run the file +directly. e.g. cd test/util; ./utiltest.rb + +You might need to run some tests as root. + +If you do not have rake installed: + gem install rake + +## The following information is possibly out of date? + Tests are organized into a dual hierarchy: each subdirectory is considered a test suite, and each file in the subdirectory is considered a test case. You can use any test case as an example of how to write @@ -12,8 +22,3 @@ if __FILE__ == $0 $:.unshift '../../lib' $blinkbase = "../.." end - -To run all tests, just type './test'. To run an individual suite, run -'./test <suite>'. To run an individual case, cd into the suite -subdirectory and run './tc_<case>.rb'. Tests are basically guaranteed -not to work without cd'ing into the subdirectory. diff --git a/test/util/utiltest.rb b/test/util/utiltest.rb index fd7c6c38f..07023d138 100755 --- a/test/util/utiltest.rb +++ b/test/util/utiltest.rb @@ -277,11 +277,15 @@ class TestPuppetUtil < Test::Unit::TestCase # Now try it with a single quote assert_nothing_raised do output = Puppet::Util.execute([command, "yay'test", "funtest"]) - # output = Puppet::Util.execute(command) - end assert_equal("yay'test\nfuntest\n", output) + # Now make sure we can squelch output (#565) + assert_nothing_raised do + output = Puppet::Util.execute([command, "yay'test", "funtest"], :squelch => true) + end + assert_equal(nil, output) + # Now test that we correctly fail if the command returns non-zero assert_raise(Puppet::ExecutionFailure) do out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"]) @@ -289,7 +293,7 @@ class TestPuppetUtil < Test::Unit::TestCase # And that we can tell it not to fail assert_nothing_raised() do - out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"], false) + out = Puppet::Util.execute(["touch", "/no/such/file/could/exist"], :failonfail => false) end if Process.uid == 0 @@ -298,7 +302,7 @@ class TestPuppetUtil < Test::Unit::TestCase group = nonrootgroup file = tempfile() assert_nothing_raised do - Puppet::Util.execute(["touch", file], true, user.name, group.name) + Puppet::Util.execute(["touch", file], :uid => user.name, :gid => group.name) end assert(FileTest.exists?(file), "file was not created") assert_equal(user.uid, File.stat(file).uid, "uid was not set correctly") @@ -308,6 +312,24 @@ class TestPuppetUtil < Test::Unit::TestCase # assert_equal(group.gid, File.stat(file).gid, # "gid was not set correctly") end + + # (#565) Test the case of patricide. + patricidecommand = tempfile() + File.open(patricidecommand, "w") { |f| + f.puts %{#!/bin/bash\n/bin/bash -c 'kill -TERM \$PPID' &;\n while [ 1 ]; do echo -n ''; done;\n} + } + File.chmod(0755, patricidecommand) + assert_nothing_raised do + output = Puppet::Util.execute([patricidecommand], :squelch => true) + end + assert_equal(nil, output) + # See what happens if we try and read the pipe to the command... + assert_raise(Puppet::ExecutionFailure) do + output = Puppet::Util.execute([patricidecommand]) + end + assert_nothing_raised do + output = Puppet::Util.execute([patricidecommand], :failonfail => false) + end end # Check whether execute() accepts strings in addition to arrays. |