Please review pull request #569: Ticket/master/3910 server authoritative opened by (pcarlisle)
Description:
(#3910) Make puppet agent use environment specified in catalog
If there is an environment specified in the catalog to be applied (i.e. an
external node classifier was used and gave an environment for this node) the
puppet agent will switch to that environment.When switching environment, the agent needs to get facts and plugins again using the new environment, and then try fetching the catalog again using the new facts, since this may change the catalog. It loops in this process several times, until the environment doesn't change. If it fails to stabilize in a limited number of tries it will raise an error and abort the run. This change also means that the prerun command runs after fetching catalog but before applying. It previously ran before fetching the catalog.
- Opened: Thu Mar 08 23:26:53 UTC 2012
- Based on: puppetlabs:master (fc0e8c4800356833dcb06ed7850febfe51abae2c)
- Requested merge: pcarlisle:ticket/master/3910-server-authoritative (e79817635d3798aafb324407117b8ddf61803ddd)
Diff follows:
diff --git a/acceptance/tests/environment/use_agent_environment_when_no_enc.rb b/acceptance/tests/environment/use_agent_environment_when_no_enc.rb
new file mode 100644
index 0000000..87be8b3
--- /dev/null
+++ b/acceptance/tests/environment/use_agent_environment_when_no_enc.rb
@@ -0,0 +1,30 @@
+test_name "Agent should use agent environment if there is no enc-specified environment"
+
+testdir = master.tmpdir('use_agent_env')
+
+create_remote_file master, "#{testdir}/puppet.conf", <<END
+[main]
+manifest = "#{testdir}/site.pp"
+
+[production]
+manifest = "#{testdir}/different.pp"
+
+[more_different]
+manifest = "#{testdir}/more_different.pp"
+END
+
+create_remote_file(master, "#{testdir}/different.pp", 'notify { "production environment": }')
+create_remote_file(master, "#{testdir}/more_different.pp", 'notify { "more_different_string": }')
+
+on master, "chown -R root:puppet #{testdir}"
+on master, "chmod -R g+rwX #{testdir}"
+
+with_master_running_on(master, "--config #{testdir}/puppet.conf --daemonize --dns_alt_names=\"puppet,$(hostname -s),$(hostname -f)\" --autosign true") do
+
+ agents.each do |agent|
+ run_agent_on(agent, "--no-daemonize --onetime --server #{master} --verbose --environment more_different")
+ assert_match(/more_different_string/, stdout, "Did not find more_different_string from \"more_different\" environment")
+ end
+end
+
+on master, "rm -rf #{testdir}"
diff --git a/acceptance/tests/environment/use_enc_environment.rb b/acceptance/tests/environment/use_enc_environment.rb
new file mode 100644
index 0000000..553e1ad
--- /dev/null
+++ b/acceptance/tests/environment/use_enc_environment.rb
@@ -0,0 +1,37 @@
+test_name "Agent should environment given by ENC"
+
+testdir = master.tmpdir('use_enc_env')
+
+create_remote_file master, "#{testdir}/enc.rb", <<END
+#!/usr/bin/env ruby
+puts <<YAML
+parameters:
+environment: special
+YAML
+END
+on master, "chmod 755 #{testdir}/enc.rb"
+
+create_remote_file master, "#{testdir}/puppet.conf", <<END
+[main]
+node_terminus = exec
+external_nodes = "#{testdir}/enc.rb"
+manifest = "#{testdir}/site.pp"
+
+[special]
+manifest = "#{testdir}/different.pp"
+END
+
+create_remote_file(master, "#{testdir}/different.pp", 'notify { "expected_string": }')
+
+on master, "chown -R root:puppet #{testdir}"
+on master, "chmod -R g+rwX #{testdir}"
+
+with_master_running_on(master, "--config #{testdir}/puppet.conf --daemonize --dns_alt_names=\"puppet,$(hostname -s),$(hostname -f)\" --autosign true") do
+
+ agents.each do |agent|
+ run_agent_on(agent, "--no-daemonize --onetime --server #{master} --verbose")
+ assert_match(/expected_string/, stdout, "Did not find expected_string from \"special\" environment")
+ end
+end
+
+on master, "rm -rf #{testdir}"
diff --git a/acceptance/tests/environment/use_enc_environment_for_files.rb b/acceptance/tests/environment/use_enc_environment_for_files.rb
new file mode 100644
index 0000000..b921fa9
--- /dev/null
+++ b/acceptance/tests/environment/use_enc_environment_for_files.rb
@@ -0,0 +1,52 @@
+test_name "Agent should environment given by ENC for pluginsync"
+
+testdir = master.tmpdir('respect_enc_test')
+
+create_remote_file master, "#{testdir}/enc.rb", <<END
+#!/usr/bin/env ruby
+puts <<YAML
+parameters:
+environment: special
+YAML
+END
+on master, "chmod 755 #{testdir}/enc.rb"
+
+create_remote_file master, "#{testdir}/puppet.conf", <<END
+[main]
+node_terminus = exec
+external_nodes = "#{testdir}/enc.rb"
+
+[special]
+modulepath = "#{testdir}/special"
+manifest = "#{testdir}/different.pp"
+END
+
+on master, "mkdir -p #{testdir}/modules"
+# Create a plugin file on the master
+on master, "mkdir -p #{testdir}/special/amod/files"
+create_remote_file(master, "#{testdir}/special/amod/files/testy", "special_environment")
+
+on master, "chown -R root:puppet #{testdir}"
+on master, "chmod -R g+rwX #{testdir}"
+
+with_master_running_on(master, "--config #{testdir}/puppet.conf --daemonize --dns_alt_names=\"puppet,$(hostname -s),$(hostname -f)\" --autosign true") do
+
+ agents.each do |agent|
+ atmp = agent.tmpdir('respect_enc_test')
+ create_remote_file master, "#{testdir}/different.pp", <<END
+file { "#{atmp}/special_testy":
+ source => "puppet:///amod/testy",
+}
+
+notify { "mytemp is ${::mytemp}": }
+END
+ on master, "chmod 644 #{testdir}/different.pp"
+
+ run_agent_on(agent, "--no-daemonize --onetime --server #{master} --verbose")
+ on agent, "cat #{atmp}/special_testy"
+ assert_match(/special_environment/, stdout, "The file from environment 'special' was not found")
+ on agent, "rm -rf #{atmp}"
+ end
+end
+
+on master, "rm -rf #{testdir}"
diff --git a/acceptance/tests/environment/use_enc_environment_for_pluginsync.rb b/acceptance/tests/environment/use_enc_environment_for_pluginsync.rb
new file mode 100644
index 0000000..d447daa
--- /dev/null
+++ b/acceptance/tests/environment/use_enc_environment_for_pluginsync.rb
@@ -0,0 +1,41 @@
+test_name "Agent should use environment given by ENC for pluginsync"
+
+testdir = master.tmpdir('respect_enc_test')
+
+create_remote_file master, "#{testdir}/enc.rb", <<END
+#!/usr/bin/env ruby
+puts <<YAML
+parameters:
+environment: special
+YAML
+END
+on master, "chmod 755 #{testdir}/enc.rb"
+
+create_remote_file master, "#{testdir}/puppet.conf", <<END
+[main]
+node_terminus = exec
+external_nodes = "#{testdir}/enc.rb"
+
+[special]
+modulepath = "#{testdir}/special"
+END
+
+on master, "mkdir -p #{testdir}/modules"
+# Create a plugin file on the master
+on master, "mkdir -p #{testdir}/special/amod/lib/puppet"
+create_remote_file(master, "#{testdir}/special/amod/lib/puppet/foo.rb", "#special_version")
+
+on master, "chown -R root:puppet #{testdir}"
+on master, "chmod -R g+rwX #{testdir}"
+
+with_master_running_on(master, "--config #{testdir}/puppet.conf --daemonize --dns_alt_names=\"puppet,$(hostname -s),$(hostname -f)\" --autosign true") do
+
+ agents.each do |agent|
+ run_agent_on(agent, "--no-daemonize --onetime --server #{master}")
+ on agent, "cat #{agent['puppetvardir']}/lib/puppet/foo.rb"
+ assert_match(/#special_version/, stdout, "The plugin from environment 'special' was not synced")
+ on agent, "rm -rf #{agent['puppetvardir']}/lib"
+ end
+end
+
+on master, "rm -rf #{testdir}"
diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index 2853dc4..22e8f97 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -33,11 +33,6 @@ def self.lockfile_path
Puppet[:puppetdlockfile]
end
- def clear
- @catalog.clear(true) if @catalog
- @catalog = nil
- end
-
def execute_postrun_command
execute_from_setting(:postrun_command)
end
@@ -102,14 +97,29 @@ def convert_catalog(result, duration)
catalog
end
- # Retrieve (optionally) and apply a catalog. If a catalog is passed in
- # the options, then apply that one, otherwise retrieve it.
- def retrieve_and_apply_catalog(options, fact_options)
+ def prepare_and_retrieve_catalog(options)
+ prepare(options)
+
+ if Puppet::Resource::Catalog.indirection.terminus_class == :rest
+ # This is a bit complicated. We need the serialized and escaped facts,
+ # and we need to know which format they're encoded in. Thus, we
+ # get a hash with both of these pieces of information.
+ fact_options = facts_for_uploading
+ end
+
+ # set report host name now that we have the fact
+ options[:report].host = Puppet[:node_name_value]
+
unless catalog = (options.delete(:catalog) || retrieve_catalog(fact_options))
Puppet.err "Could not retrieve catalog; skipping run"
return
end
+ catalog
+ end
+ # Retrieve (optionally) and apply a catalog. If a catalog is passed in
+ # the options, then apply that one, otherwise retrieve it.
+ def apply_catalog(catalog, options)
report = options[:report]
report.configuration_version = catalog.version
report.environment = Puppet[:environment]
@@ -131,25 +141,29 @@ def run(options = {})
Puppet::Util::Log.newdestination(report)
begin
- prepare(options)
-
- if Puppet::Resource::Catalog.indirection.terminus_class == :rest
- # This is a bit complicated. We need the serialized and escaped facts,
- # and we need to know which format they're encoded in. Thus, we
- # get a hash with both of these pieces of information.
- fact_options = facts_for_uploading
- end
+ begin
+ unless catalog = prepare_and_retrieve_catalog(options)
+ return nil
+ end
- # set report host name now that we have the fact
- report.host = Puppet[:node_name_value]
+ # Here we set the local environment based on what we get from the
+ # catalog. Since a change in environment means a change in facts, and
+ # facts may be used to determine which catalog we get, we need to
+ # rerun the process if the environment is changed.
+ tries = 0
+ while catalog.environment and not catalog.environment.empty? and catalog.environment != Puppet[:environment]
+ if tries > 3
+ raise Puppet::Error, "Catalog environment didn't stabilize after #{tries} fetches, aborting run"
+ end
+ Puppet.warning "Local environment: \"#{Puppet[:environment]}\" doesn't match server specified environment \"#{catalog.environment}\", restarting agent run with new environment"
+ Puppet[:environment] = catalog.environment
+ return nil unless catalog = prepare_and_retrieve_catalog(options)
+ tries += 1
+ end
- begin
execute_prerun_command or return nil
- if retrieve_and_apply_catalog(options, fact_options)
- report.exit_status
- else
- nil
- end
+ apply_catalog(catalog, options)
+ report.exit_status
rescue SystemExit,NoMemoryError
raise
rescue => detail
diff --git a/lib/puppet/parser/compiler.rb b/lib/puppet/parser/compiler.rb
index 9b3b5d4..703a779 100644
--- a/lib/puppet/parser/compiler.rb
+++ b/lib/puppet/parser/compiler.rb
@@ -430,6 +430,8 @@ def initvars
@catalog = Puppet::Resource::Catalog.new(@node.name)
@catalog.version = known_resource_types.version
+ @catalog.environment = @node.environment.to_s
+
# Create our initial scope and a resource that will evaluate main.
@topscope = Puppet::Parser::Scope.new(:compiler => self)
diff --git a/lib/puppet/resource/catalog.rb b/lib/puppet/resource/catalog.rb
index 2c7cdd5..2f139bd 100644
--- a/lib/puppet/resource/catalog.rb
+++ b/lib/puppet/resource/catalog.rb
@@ -44,6 +44,9 @@ class DuplicateResourceError < Puppet::Error; end
# Some metadata to help us compile and generally respond to the current state.
attr_accessor :client_version, :server_version
+ # The environment for this catalog
+ attr_accessor :environment
+
# Add classes to our class list.
def add_class(*classes)
classes.each do |klass|
@@ -396,6 +399,10 @@ def self.from_pson(data)
result.version = version
end
+ if environment = data['environment']
+ result.environment = environment
+ end
+
if resources = data['resources']
resources = PSON.parse(resources) if resources.is_a?(String)
resources.each do |res|
@@ -447,6 +454,7 @@ def to_pson_data_hash
'tags' => tags,
'name' => name,
'version' => version,
+ 'environment' => environment.to_s,
'resources' => vertices.collect { |v| v.to_pson_data_hash },
'edges' => edges. collect { |e| e.to_pson_data_hash },
'classes' => classes
@@ -536,6 +544,7 @@ def to_catalog(convert)
result = self.class.new(self.name)
result.version = self.version
+ result.environment = self.environment
map = {}
vertices.each do |resource|
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index e7956a6..f14c854 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -249,9 +249,8 @@
Puppet.settings[:prerun_command] = "/my/command"
Puppet::Util::Execution.expects(:execute).with(["/my/command"]).raises(Puppet::ExecutionFailure, "Failed")
- report.expects(:<<).with { |log| log.message.include?("Could not run command from prerun_command") }
-
@agent.run.should be_nil
+ report.logs.find { |x| x.message =~ /Could not run command from prerun_command/ }.should be
end
it "should send the transaction report even if the post-run command fails" do
@@ -320,6 +319,22 @@
@agent.run.should be_nil
end
+ it "should refetch the catalog if the server specifies a new environment in the catalog" do
+ @catalog.stubs(:environment).returns("second_env")
+ @agent.expects(:prepare).twice
+ @agent.expects(:retrieve_catalog).returns(@catalog).twice
+
+ @agent.run
+ end
+
+ it "should change the environment setting if the server specifies a new environment in the catalog" do
+ @catalog.stubs(:environment).returns("second_env")
+
+ @agent.run
+
+ Puppet[:environment].should == "second_env"
+ end
+
describe "when not using a REST terminus for catalogs" do
it "should not pass any facts when retrieving the catalog" do
Puppet::Resource::Catalog.indirection.terminus_class = :compiler
-- 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.
