Please review pull request #621: fix master acceptance tests opened by (cprice-puppet)
Description:
This pull request addresses the following tickets:
#13584: re: puppet master sometimes swallowing errors during startup in daemon mode
#13588: re: invalid permissions for log dir
#13510: re: an acceptance test that was failing due to the removal of the default to 'apply' when not specifying a subcommand
it also fixes a couple of other acceptance tests that had been masked by the recent startup failures.
These changes have gone through acceptance testing here:
https://jenkins.puppetlabs.com/view/Puppet%20FOSS%20(integration)/job/Puppet%20Acceptance%20(integration%20-%20master%20-%20continuous)/
and spec testing here:
https://jenkins.puppetlabs.com/view/Puppet%20FOSS%20(integration)/job/Puppet%20Specs%20(integration%20-%20master)/21/
- Opened: Wed Apr 04 02:05:00 UTC 2012
- Based on: puppetlabs:master (ba112e94619e6d35ca1d2d89975f787e537fe1e3)
- Requested merge: puppetlabs:integration/master/13588-log-dir-perms (dbb8e723a1c884209075dcc6be18dbf91e132d66)
Diff follows:
diff --git a/acceptance/tests/pluginsync/7316_apps_should_be_available_via_pluginsync.rb b/acceptance/tests/pluginsync/7316_apps_should_be_available_via_pluginsync.rb
index 0176810..3624ac8 100644
--- a/acceptance/tests/pluginsync/7316_apps_should_be_available_via_pluginsync.rb
+++ b/acceptance/tests/pluginsync/7316_apps_should_be_available_via_pluginsync.rb
@@ -215,7 +215,7 @@ def main()
step "clear out the libdir on the agents in preparation for the next test" do
agents.each do |agent|
- on(agent, "rm -rf #{get_test_file_path(agent, agent_module_app_file)}/*")
+ on(agent, "rm -rf #{get_test_file_path(agent, agent_lib_dir)}/*")
end
end
diff --git a/acceptance/tests/pluginsync/7316_faces_with_app_stubs_should_be_available_via_pluginsync.rb b/acceptance/tests/pluginsync/7316_faces_with_app_stubs_should_be_available_via_pluginsync.rb
index f0f7769..5b559b3 100644
--- a/acceptance/tests/pluginsync/7316_faces_with_app_stubs_should_be_available_via_pluginsync.rb
+++ b/acceptance/tests/pluginsync/7316_faces_with_app_stubs_should_be_available_via_pluginsync.rb
@@ -238,8 +238,7 @@ class Puppet::Application::#{app_name.capitalize} < Puppet::Application::FaceBas
step "clear out the libdir on the agents in preparation for the next test" do
agents.each do |agent|
- on(agent, "rm -rf #{get_test_file_path(agent, agent_module_app_file)}/*")
- on(agent, "rm -rf #{get_test_file_path(agent, agent_module_face_file)}/*")
+ on(agent, "rm -rf #{get_test_file_path(agent, agent_lib_dir)}/*")
end
end
diff --git a/acceptance/tests/ticket_6928_puppet_master_parse_fails.rb b/acceptance/tests/ticket_6928_puppet_master_parse_fails.rb
index 039390c..7141bdc 100644
--- a/acceptance/tests/ticket_6928_puppet_master_parse_fails.rb
+++ b/acceptance/tests/ticket_6928_puppet_master_parse_fails.rb
@@ -18,7 +18,7 @@
create_remote_file(host, bad, 'notify{bad:')
step "Agents: use --parseonly on an invalid manifest, should return 1 and issue deprecation warning"
- on(host, puppet('--parseonly', bad), :acceptable_exit_codes => [ 1 ]) do
+ on(host, puppet('apply', '--parseonly', bad), :acceptable_exit_codes => [ 1 ]) do
assert_match(/--parseonly has been removed. Please use \'puppet parser validate <manifest>\'/, stdout, "Deprecation warning not issued for --parseonly on #{host}" )
end
diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb
index b97f2c3..fd6269e 100755
--- a/lib/puppet/daemon.rb
+++ b/lib/puppet/daemon.rb
@@ -25,11 +25,18 @@ def daemonize
Process.setsid
Dir.chdir("/")
+ end
+
+ # Close stdin/stdout/stderr so that we can finish our transition into 'daemon' mode.
+ # @return nil
+ def self.close_streams()
+ Puppet.debug("Closing streams for daemon mode")
begin
$stdin.reopen "/dev/null"
$stdout.reopen "/dev/null", "a"
$stderr.reopen $stdout
Puppet::Util::Log.reopen
+ Puppet.debug("Finished closing streams for daemon mode")
rescue => detail
Puppet.err "Could not start #{Puppet[:name]}: #{detail}"
Puppet::Util::replace_file("/tmp/daemonout", 0644) do |f|
@@ -39,6 +46,11 @@ def daemonize
end
end
+ # Convenience signature for calling Puppet::Daemon.close_streams
+ def close_streams()
+ Puppet::Daemon.close_streams
+ end
+
# Create a pidfile for our daemon, so we can be stopped and others
# don't try to start.
def create_pidfile
@@ -123,6 +135,12 @@ def start
# Start the listening server, if required.
server.start if server
+ # now that the server has started, we've waited just about as long as possible to close
+ # our streams and become a "real" daemon process. This is in hopes of allowing
+ # errors to have the console available as a fallback for logging for as long as
+ # possible.
+ close_streams
+
# Finally, loop forever running events - or, at least, until we exit.
run_event_loop
end
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index 1ca1779..4598f65 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -49,6 +49,8 @@ module Puppet
:default => nil,
:type => :directory,
:mode => 0750,
+ :owner => "service",
+ :group => "service",
:desc => "The directory in which to store log files",
}
)
diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb
index f92d600..a328a1b 100644
--- a/lib/puppet/network/server.rb
+++ b/lib/puppet/network/server.rb
@@ -4,6 +4,10 @@
class Puppet::Network::Server
attr_reader :server_type, :address, :port
+
+ # TODO: does anything actually call this? It seems like it's a duplicate of the code in Puppet::Daemon, but that
+ # it's not actually called anywhere.
+
# Put the daemon into the background.
def daemonize
if pid = fork
@@ -16,17 +20,10 @@ def daemonize
Process.setsid
Dir.chdir("/")
- begin
- $stdin.reopen "/dev/null"
- $stdout.reopen "/dev/null", "a"
- $stderr.reopen $stdout
- Puppet::Util::Log.reopen
- rescue => detail
- Puppet::Util.replace_file("/tmp/daemonout", 0644) { |f|
- f.puts "Could not start #{Puppet[:name]}: #{detail}"
- }
- raise "Could not start #{Puppet[:name]}: #{detail}"
- end
+ end
+
+ def close_streams()
+ Puppet::Daemon.close_streams()
end
# Create a pidfile for our daemon, so we can be stopped and others
@@ -113,6 +110,7 @@ def http_server_class
def start
create_pidfile
+ close_streams
listen
end
diff --git a/lib/puppet/util.rb b/lib/puppet/util.rb
index 9d1e563..76bfbe6 100644
--- a/lib/puppet/util.rb
+++ b/lib/puppet/util.rb
@@ -476,13 +476,22 @@ def replace_file(file, default_mode, &block)
# @yield
def exit_on_fail(message, code = 1)
yield
- rescue ArgumentError, RuntimeError, NotImplementedError => detail
- Puppet.log_exception(detail, "Could not #{message}: #{detail}")
+ # First, we need to check and see if we are catching a SystemExit error. These will be raised
+ # when we daemonize/fork, and they do not necessarily indicate a failure case.
+ rescue SystemExit => err
+ raise err
+
+ # Now we need to catch *any* other kind of exception, because we may be calling third-party
+ # code (e.g. webrick), and we have no idea what they might throw.
+ rescue Exception => err
+ Puppet.log_exception(err, "Could not #{message}: #{err}")
exit(code)
end
module_function :exit_on_fail
+
+
#######################################################################################################
# Deprecated methods relating to process execution; these have been moved to Puppet::Util::Execution
#######################################################################################################
diff --git a/spec/unit/daemon_spec.rb b/spec/unit/daemon_spec.rb
index 03824bf..e6b700f 100755
--- a/spec/unit/daemon_spec.rb
+++ b/spec/unit/daemon_spec.rb
@@ -22,6 +22,7 @@ def lockfile_path
before do
@agent = Puppet::Agent.new(TestClient.new)
@daemon = Puppet::Daemon.new
+ @daemon.stubs(:close_streams).returns nil
end
it "should be able to manage an agent" do
diff --git a/spec/unit/network/server_spec.rb b/spec/unit/network/server_spec.rb
index 8da5cff..0b9e73a 100755
--- a/spec/unit/network/server_spec.rb
+++ b/spec/unit/network/server_spec.rb
@@ -13,6 +13,7 @@
Puppet::Network::HTTP.stubs(:server_class_by_type).returns(@mock_http_server_class)
Puppet.settings.stubs(:value).with(:servertype).returns(:suparserver)
@server = Puppet::Network::Server.new(:port => 31337)
+ @server.stubs(:close_streams).returns(nil)
end
describe "when initializing" do
-- 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.
