Please review pull request #695: (#13966) Remove Puppet[:name] option opened by (jeffweiss)
Description:
Replace Puppet[:name] with run_mode.
Set application default pidfile for puppet queue.
Make application defaults trump defaults.rb for default comment inside--genconfig.
- Opened: Fri Apr 20 07:00:25 UTC 2012
- Based on: puppetlabs:master (54e1c83afde1b721e5433adea17a4dd0caffbc63)
- Requested merge: jeffweiss:ticket/master/13966_remove_name_option (8b8179470f7a0a9a60c42fa253ee52ef511793e7)
Diff follows:
diff --git a/lib/puppet/application.rb b/lib/puppet/application.rb
index ad92aa1..b009902 100644
--- a/lib/puppet/application.rb
+++ b/lib/puppet/application.rb
@@ -267,7 +267,6 @@ def run_mode( mode_name = nil)
def app_defaults()
{
- :name => name,
:run_mode => self.class.run_mode.name,
:confdir => self.class.run_mode.conf_dir,
:vardir => self.class.run_mode.var_dir,
diff --git a/lib/puppet/application/queue.rb b/lib/puppet/application/queue.rb
index 22bdfe0..e0304de 100644
--- a/lib/puppet/application/queue.rb
+++ b/lib/puppet/application/queue.rb
@@ -4,6 +4,10 @@
class Puppet::Application::Queue < Puppet::Application
attr_accessor :daemon
+
+ def app_defaults()
+ super.merge( :pidfile => "$rundir/queue.pid" )
+ end
def preinit
require 'puppet/daemon'
diff --git a/lib/puppet/daemon.rb b/lib/puppet/daemon.rb
index fc60f2e..15358f0 100755
--- a/lib/puppet/daemon.rb
+++ b/lib/puppet/daemon.rb
@@ -8,7 +8,7 @@ class Puppet::Daemon
attr_accessor :agent, :server, :argv
def daemonname
- Puppet[:name]
+ Puppet.run_mode.name
end
# Put the daemon into the background.
@@ -38,9 +38,9 @@ def self.close_streams()
Puppet::Util::Log.reopen
Puppet.debug("Finished closing streams for daemon mode")
rescue => detail
- Puppet.err "Could not start #{Puppet[:name]}: #{detail}"
+ Puppet.err "Could not start #{Puppet.run_mode.name}: #{detail}"
Puppet::Util::replace_file("/tmp/daemonout", 0644) do |f|
- f.puts "Could not start #{Puppet[:name]}: #{detail}"
+ f.puts "Could not start #{Puppet.run_mode.name}: #{detail}"
end
exit(12)
end
@@ -54,7 +54,7 @@ def close_streams()
# Create a pidfile for our daemon, so we can be stopped and others
# don't try to start.
def create_pidfile
- Puppet::Util.synchronize_on(Puppet[:name],Sync::EX) do
+ Puppet::Util.synchronize_on(Puppet.run_mode.name,Sync::EX) do
raise "Could not create PID file: #{pidfile}" unless Puppet::Util::Pidlock.new(pidfile).lock
end
end
@@ -84,7 +84,7 @@ def reload
# Remove the pid file for our daemon.
def remove_pidfile
- Puppet::Util.synchronize_on(Puppet[:name],Sync::EX) do
+ Puppet::Util.synchronize_on(Puppet.run_mode.name,Sync::EX) do
Puppet::Util::Pidlock.new(pidfile).unlock
end
end
diff --git a/lib/puppet/defaults.rb b/lib/puppet/defaults.rb
index b579829..0f87700 100644
--- a/lib/puppet/defaults.rb
+++ b/lib/puppet/defaults.rb
@@ -678,7 +678,7 @@ module Puppet
},
:pidfile => {
:type => :file,
- :default => "$rundir/$name.pid",
+ :default => "$rundir/#{Puppet.run_mode.name}.pid",
:desc => "The pid file",
},
:bindaddress => {
diff --git a/lib/puppet/network/server.rb b/lib/puppet/network/server.rb
index f2ed829..275eaf7 100644
--- a/lib/puppet/network/server.rb
+++ b/lib/puppet/network/server.rb
@@ -29,14 +29,14 @@ def close_streams()
# Create a pidfile for our daemon, so we can be stopped and others
# don't try to start.
def create_pidfile
- Puppet::Util.synchronize_on(Puppet[:name],Sync::EX) do
+ Puppet::Util.synchronize_on(Puppet.run_mode.name,Sync::EX) do
raise "Could not create PID file: #{pidfile}" unless Puppet::Util::Pidlock.new(pidfile).lock
end
end
# Remove the pid file for our daemon.
def remove_pidfile
- Puppet::Util.synchronize_on(Puppet[:name],Sync::EX) do
+ Puppet::Util.synchronize_on(Puppet.run_mode.name,Sync::EX) do
Puppet::Util::Pidlock.new(pidfile).unlock
end
end
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index d0c426c..adc1d6f 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -225,7 +225,7 @@ def add_dynamically_generated_resources
# Should we ignore tags?
def ignore_tags?
- ! (@catalog.host_config? or Puppet[:name] == "puppet")
+ ! @catalog.host_config?
end
# this should only be called by a Puppet::Type::Component resource now
diff --git a/lib/puppet/util/log/destinations.rb b/lib/puppet/util/log/destinations.rb
index 777cbf5..4dc1da4 100644
--- a/lib/puppet/util/log/destinations.rb
+++ b/lib/puppet/util/log/destinations.rb
@@ -9,8 +9,7 @@ def close
def initialize
Syslog.close if Syslog.opened?
- name = Puppet[:name]
- name = "puppet-#{name}" unless name =~ /puppet/
+ name = "puppet-#{Puppet.run_mode.name}"
options = Syslog::LOG_PID | Syslog::LOG_NDELAY
diff --git a/lib/puppet/util/settings.rb b/lib/puppet/util/settings.rb
index d6b54a3..5558cf8 100644
--- a/lib/puppet/util/settings.rb
+++ b/lib/puppet/util/settings.rb
@@ -18,10 +18,10 @@ class Puppet::Util::Settings
attr_accessor :files
attr_reader :timer
- ReadOnly = [:run_mode, :name]
+ ReadOnly = [:run_mode]
# These are the settings that every app is required to specify; there are reasonable defaults defined in application.rb.
- REQUIRED_APP_SETTINGS = [:name, :run_mode, :logdir, :confdir, :vardir]
+ REQUIRED_APP_SETTINGS = [:run_mode, :logdir, :confdir, :vardir]
def self.default_global_config_dir()
Puppet.features.microsoft_windows? ? File.join(Dir::COMMON_APPDATA, "PuppetLabs", "puppet", "etc") : "/etc/puppet"
@@ -102,6 +102,8 @@ def unsafe_clear(clear_cli = true, clear_application_defaults = false)
# and we want to retain this cli values.
@used = [] if clear_cli
+ @app_defaults_initialized = false if clear_application_defaults
+
@cache.clear
end
private :unsafe_clear
@@ -128,7 +130,6 @@ def initialize_app_defaults(app_defaults)
set_value(key, value, :application_defaults)
end
-
@app_defaults_initialized = true
end
@@ -771,7 +772,7 @@ def to_catalog(*sections)
# Convert our list of config settings into a configuration file.
def to_config
- str = %{The configuration file for #{Puppet[:name]}. Note that this file
+ str = %{The configuration file for #{Puppet.run_mode.name}. Note that this file
is likely to have unused configuration parameters in it; any parameter that's
valid anywhere in Puppet can be in any config file, even if it's not used.
@@ -866,7 +867,7 @@ def find_value(environment, param)
# Find the correct value using our search path. Optionally accept an environment
# in which to search before the other configuration sections.
- def value(param, environment = nil)
+ def value(param, environment = nil, bypass_interpolation = false)
param = param.to_sym
environment &&= environment.to_sym
@@ -885,6 +886,7 @@ def value(param, environment = nil)
val = uninterpolated_value(param, environment)
+ return val if bypass_interpolation
if param == :code
# if we interpolate code, all hell breaks loose.
return val
diff --git a/lib/puppet/util/settings/string_setting.rb b/lib/puppet/util/settings/string_setting.rb
index 7405952..22428d5 100644
--- a/lib/puppet/util/settings/string_setting.rb
+++ b/lib/puppet/util/settings/string_setting.rb
@@ -75,13 +75,18 @@ def short=(value)
raise ArgumentError, "Short names can only be one character." if value.to_s.length != 1
@short = value.to_s
end
+
+ def default(check_application_defaults_first = false)
+ return @default unless check_application_defaults_first
+ return @settings.value(name, :application_defaults, true) || @default
+ end
# Convert the object to a config statement.
def to_config
str = @desc.gsub(/^/, "# ") + "\n"
# Add in a statement about the default.
- str += "# The default value is '#{@default}'.\n" if @default
+ str += "# The default value is '#{default(true)}'.\n" if default(true)
# If the value has not been overridden, then print it out commented
# and unconverted, so it's clear that that's the default and how it
diff --git a/spec/unit/daemon_spec.rb b/spec/unit/daemon_spec.rb
index e6b700f..207db3d 100755
--- a/spec/unit/daemon_spec.rb
+++ b/spec/unit/daemon_spec.rb
@@ -128,7 +128,7 @@ def lockfile_path
describe "when creating its pidfile" do
it "should use an exclusive mutex" do
- Puppet.settings.expects(:value).with(:name).returns "me"
+ Puppet.run_mode.expects(:name).returns "me"
Puppet::Util.expects(:synchronize_on).with("me",Sync::EX)
@daemon.create_pidfile
end
@@ -136,7 +136,7 @@ def lockfile_path
it "should lock the pidfile using the Pidlock class" do
pidfile = mock 'pidfile'
- Puppet.settings.stubs(:value).with(:name).returns "eh"
+ Puppet.run_mode.expects(:name).returns "eh"
Puppet.settings.expects(:value).with(:pidfile).returns make_absolute("/my/file")
Puppet::Util::Pidlock.expects(:new).with(make_absolute("/my/file")).returns pidfile
@@ -148,7 +148,7 @@ def lockfile_path
it "should fail if it cannot lock" do
pidfile = mock 'pidfile'
- Puppet.settings.stubs(:value).with(:name).returns "eh"
+ Puppet.run_mode.expects(:name).returns "eh"
Puppet.settings.stubs(:value).with(:pidfile).returns make_absolute("/my/file")
Puppet::Util::Pidlock.expects(:new).with(make_absolute("/my/file")).returns pidfile
@@ -161,7 +161,7 @@ def lockfile_path
describe "when removing its pidfile" do
it "should use an exclusive mutex" do
- Puppet.settings.expects(:value).with(:name).returns "me"
+ Puppet.run_mode.expects(:name).returns "me"
Puppet::Util.expects(:synchronize_on).with("me",Sync::EX)
diff --git a/spec/unit/network/server_spec.rb b/spec/unit/network/server_spec.rb
index 20c34da..00fca42 100755
--- a/spec/unit/network/server_spec.rb
+++ b/spec/unit/network/server_spec.rb
@@ -6,7 +6,7 @@
before do
@mock_http_server_class = mock('http server class')
Puppet.settings.stubs(:use)
- Puppet.settings.stubs(:value).with(:name).returns("me")
+ Puppet.run_mode.stubs(:name).returns "me"
Puppet.settings.stubs(:value).with(:servertype).returns(:suparserver)
Puppet.settings.stubs(:value).with(:bindaddress).returns("")
Puppet.settings.stubs(:value).with(:masterport).returns(8140)
@@ -152,7 +152,7 @@
describe "when creating its pidfile" do
it "should use an exclusive mutex" do
- Puppet.settings.expects(:value).with(:name).returns "me"
+ Puppet.run_mode.expects(:name).returns "me"
Puppet::Util.expects(:synchronize_on).with("me",Sync::EX)
@server.create_pidfile
end
@@ -160,7 +160,7 @@
it "should lock the pidfile using the Pidlock class" do
pidfile = mock 'pidfile'
- Puppet.settings.stubs(:value).with(:name).returns "eh"
+ Puppet.run_mode.expects(:name).returns "eh"
Puppet.settings.expects(:value).with(:pidfile).returns "/my/file"
Puppet::Util::Pidlock.expects(:new).with("/my/file").returns pidfile
@@ -172,7 +172,7 @@
it "should fail if it cannot lock" do
pidfile = mock 'pidfile'
- Puppet.settings.stubs(:value).with(:name).returns "eh"
+ Puppet.run_mode.stubs(:name).returns "eh"
Puppet.settings.stubs(:value).with(:pidfile).returns "/my/file"
Puppet::Util::Pidlock.expects(:new).with("/my/file").returns pidfile
@@ -185,7 +185,7 @@
describe "when removing its pidfile" do
it "should use an exclusive mutex" do
- Puppet.settings.expects(:value).with(:name).returns "me"
+ Puppet.run_mode.expects(:name).returns "me"
Puppet::Util.expects(:synchronize_on).with("me",Sync::EX)
@server.remove_pidfile
end
diff --git a/spec/unit/util/settings/string_setting_spec.rb b/spec/unit/util/settings/string_setting_spec.rb
new file mode 100644
index 0000000..f0b314f
--- /dev/null
+++ b/spec/unit/util/settings/string_setting_spec.rb
@@ -0,0 +1,75 @@
+#!/usr/bin/env rspec
+require 'spec_helper'
+
+require 'puppet/util/settings'
+require 'puppet/util/settings/string_setting'
+
+describe Puppet::Util::Settings::StringSetting do
+ StringSetting = Puppet::Util::Settings::StringSetting
+
+ before(:each) do
+ @test_setting_name = :test_setting
+ @test_setting_default = "my_crazy_default/$var"
+ @application_setting = "application/$var"
+ @application_defaults = { }
+ Puppet::Util::Settings::REQUIRED_APP_SETTINGS.each do |key|
+ @application_defaults[key] = "foo"
+ end
+ @application_defaults[:run_mode] = :user
+ @settings = Puppet::Util::Settings.new
+ @application_defaults.each { |k,v| @settings.define_settings :main, k => {:default=>"", :desc => "blah"} }
+ @settings.define_settings :main, :var => { :default => "interpolate!",
+ :type => :string,
+ :desc => "my var desc" },
+ @test_setting_name => { :default => @test_setting_default,
+ :type => :string,
+ :desc => "my test desc" }
+ @test_setting = @settings.setting(@test_setting_name)
+ end
+
+ describe "#default" do
+ describe "with no arguments" do
+ it "should return the setting default" do
+ @test_setting.default.should == @test_setting_default
+ end
+
+ it "should be uninterpolated" do
+ @test_setting.default.should_not =~ /interpolate/
+ end
+ end
+
+ describe "checking application defaults first" do
+ describe "if application defaults set" do
+ before(:each) do
+ @settings.initialize_app_defaults @application_defaults.merge @test_setting_name => @application_setting
+ end
+
+ it "should return the application-set default" do
+ @test_setting.default(true).should == @application_setting
+ end
+
+ it "should be uninterpolated" do
+ @test_setting.default(true).should_not =~ /interpolate/
+ end
+
+ end
+
+ describe "if application defaults not set" do
+ it "should return the regular default" do
+ @test_setting.default(true).should == @test_setting_default
+ end
+
+ it "should be uninterpolated" do
+ @test_setting.default(true).should_not =~ /interpolate/
+ end
+ end
+ end
+ end
+
+ describe "#value" do
+ it "should be interpolated" do
+ @test_setting.value.should =~ /interpolate/
+ 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.
