Skip to content

Commit

Permalink
(PUP-11328) Fallback to node request in configurer
Browse files Browse the repository at this point in the history 
If we can't load our last server specified environment, then fallback to
making a node request like we used to. This way if we're supposed to use
a server specified environment and we upgrade, then we don't revert to
"production" and redownload plugins.

A node request will not be made if the `initial_environment` and the
`converged_environment` in the lastrunfile are identical, and the
`run_mode` is `agent`.

The logic is similar to what was removed in commit 39df438 except
we're using `fail_on_404` so `Puppet::Node.indirection.find` will raise
instead of returning nil.

We also don't clear facts/query_options.

Co-authored-by: Josh Cooper <[email protected]>
  • Loading branch information
@GabrielNagy @joshcooper @luchihoratiu
2 people authored and luchihoratiu committed Oct 27, 2021
1 parent 2163522 commit f0214e6
Show file tree
Hide file tree
Showing 3 changed files with 150 additions and 32 deletions.
68 changes: 59 additions & 9 deletions lib/puppet/configurer.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -302,9 +302,16 @@ def run_internal(options)
# We only need to find out the environment to run in if we don't already have a catalog
unless (cached_catalog || options[:catalog] || Puppet.settings.set_by_cli?(:environment) || Puppet[:strict_environment_mode])
Puppet.debug(_("Environment not passed via CLI and no catalog was given, attempting to find out the last server-specified environment"))
if last_server_specified_environment
@environment = last_server_specified_environment
report.environment = last_server_specified_environment
initial_environment, loaded_last_environment = last_server_specified_environment

unless loaded_last_environment
Puppet.debug(_("Requesting environment from the server"))
initial_environment = current_server_specified_environment(@environment, configured_environment, options)
end

if initial_environment
@environment = initial_environment
report.environment = initial_environment

push_current_environment_and_loaders
else
Expand Down Expand Up @@ -463,24 +470,67 @@ def find_functional_server
end
private :find_functional_server

#
# @api private
#
# Read the last server-specified environment from the lastrunfile. The
# environment is considered to be server-specified if the values of
# `initial_environment` and `converged_environment` are different.
#
# @return [String, Boolean] An array containing a string with the environment
# read from the lastrunfile in case the server is authoritative, and a
# boolean marking whether the last environment was correctly loaded.
def last_server_specified_environment
return @last_server_specified_environment if @last_server_specified_environment
return @last_server_specified_environment, @loaded_last_environment if @last_server_specified_environment

if Puppet::FileSystem.exist?(Puppet[:lastrunfile])
summary = Puppet::Util::Yaml.safe_load_file(Puppet[:lastrunfile])
return unless summary.dig('application', 'run_mode') == 'agent'
initial_environment = summary.dig('application', 'initial_environment')
converged_environment = summary.dig('application', 'converged_environment')
return [nil, nil] unless summary['application']['run_mode'] == 'agent'
initial_environment = summary['application']['initial_environment']
converged_environment = summary['application']['converged_environment']
@last_server_specified_environment = converged_environment if initial_environment != converged_environment
Puppet.debug(_("Successfully loaded last environment from the lastrunfile"))
@loaded_last_environment = true
end

Puppet.debug(_("Found last server-specified environment: %{environment}") % { environment: @last_server_specified_environment }) if @last_server_specified_environment
@last_server_specified_environment
[@last_server_specified_environment, @loaded_last_environment]
rescue => detail
Puppet.debug(_("Could not find last server-specified environment: %{detail}") % { detail: detail })
nil
[nil, nil]
end
private :last_server_specified_environment

def current_server_specified_environment(current_environment, configured_environment, options)
return @server_specified_environment if @server_specified_environment

begin
node_retr_time = thinmark do
node = Puppet::Node.indirection.find(Puppet[:node_name_value],
:environment => Puppet::Node::Environment.remote(current_environment),
:configured_environment => configured_environment,
:ignore_cache => true,
:transaction_uuid => @transaction_uuid,
:fail_on_404 => true)

@server_specified_environment = node.environment.name.to_s

if @server_specified_environment != @environment
Puppet.notice _("Local environment: '%{local_env}' doesn't match server specified node environment '%{node_env}', switching agent to '%{node_env}'.") % { local_env: @environment, node_env: @server_specified_environment }
end
end

options[:report].add_times(:node_retrieval, node_retr_time)

@server_specified_environment
rescue => detail
Puppet.warning(_("Unable to fetch my node definition, but the agent run will continue:"))
Puppet.warning(detail)
nil
end
end
private :current_server_specified_environment

def send_report(report)
puts report.summary if Puppet[:summarize]
save_last_run_summary(report)
Expand Down
21 changes: 21 additions & 0 deletions spec/integration/application/agent_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -642,6 +642,27 @@ def with_another_agent_running(&block)
end

context "environment convergence" do
it "falls back to making a node request if the last server-specified environment cannot be loaded" do
server.start_server do |port|
Puppet[:serverport] = port
Puppet[:log_level] = 'debug'

expect {
agent.command_line.args << '--test'
agent.run
}.to exit_with(0)
.and output(a_string_matching(%r{Debug: Requesting environment from the server})).to_stdout

Puppet::Application.clear!

expect {
agent.command_line.args << '--test'
agent.run
}.to exit_with(0)
.and output(a_string_matching(%r{Debug: Successfully loaded last environment from the lastrunfile})).to_stdout
end
end

it "switches to 'newenv' environment and retries the run" do
first_run = true
libdir = File.join(my_fixture_dir, 'lib')
Expand Down
93 changes: 70 additions & 23 deletions spec/unit/configurer_spec.rb
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1116,6 +1116,9 @@ def expects_neither_new_or_cached_catalog
converged_environment: #{last_server_specified_environment}
run_mode: agent
SUMMARY

expect(Puppet::Node.indirection).not_to receive(:find)
.with(anything, hash_including(:ignore_cache => true, :fail_on_404 => true))
end

it "prefers the environment set via cli" do
Expand All @@ -1125,26 +1128,27 @@ def expects_neither_new_or_cached_catalog
expect(configurer.environment).to eq('usethis')
end

it "prefers the environment set via config" do
it "prefers the environment set via lastrunfile over config" do
FileUtils.mkdir_p(Puppet[:confdir])
set_puppet_conf(Puppet[:confdir], <<~CONF)
[main]
environment = usethis
lastrunfile = #{Puppet[:lastrunfile]}
CONF

Puppet.initialize_settings
configurer.run

expect(configurer.environment).to eq('usethis')
expect(configurer.environment).to eq(last_server_specified_environment)
end

it "uses environment from Puppet[:environment] if given a catalog" do
it "uses the environment from Puppet[:environment] if given a catalog" do
configurer.run(catalog: catalog)

expect(configurer.environment).to eq(Puppet[:environment])
end

it "uses environment from Puppet[:environment] if use_cached_catalog = true" do
it "uses the environment from Puppet[:environment] if use_cached_catalog = true" do
Puppet[:use_cached_catalog] = true
expects_cached_catalog_only(catalog)
configurer.run
Expand Down Expand Up @@ -1173,14 +1177,14 @@ def expects_neither_new_or_cached_catalog
configurer.run
end

it "uses environment from Puppet[:environment] if strict_environment_mode is set" do
it "uses the environment from Puppet[:environment] if strict_environment_mode is set" do
Puppet[:strict_environment_mode] = true
configurer.run

expect(configurer.environment).to eq(Puppet[:environment])
end

it "uses environment from Puppet[:environment] if initial_environment is the same as converged_environment" do
it "uses the environment from Puppet[:environment] if initial_environment is the same as converged_environment" do
Puppet[:lastrunfile] = file_containing('last_run_summary.yaml', <<~SUMMARY)
---
version:
Expand All @@ -1195,41 +1199,84 @@ def expects_neither_new_or_cached_catalog

expect(configurer.environment).to eq(Puppet[:environment])
end
end
end

describe "when the last used environment is not available" do
describe "when the node request succeeds" do
let(:node_environment) { Puppet::Node::Environment.remote(:salam) }
let(:node) { double(Puppet::Node, :environment => node_environment) }
let(:last_server_specified_environment) { 'development' }

before do
allow(Puppet::Node.indirection).to receive(:find)
allow(Puppet::Node.indirection).to receive(:find)
.with(anything, hash_including(:ignore_cache => true, :fail_on_404 => true))
.and_return(node)
end

it "uses environment from Puppet[:environment] if the run mode doesn't match" do
it "uses the environment from the node request if the run mode doesn't match" do
Puppet[:lastrunfile] = file_containing('last_run_summary.yaml', <<~SUMMARY)
---
version:
config: 1624882680
puppet: 6.24.0
application:
initial_environment: #{Puppet[:environment]}
converged_environment: #{last_server_specified_environment}
run_mode: user
---
version:
config: 1624882680
puppet: 6.24.0
application:
initial_environment: #{Puppet[:environment]}
converged_environment: #{last_server_specified_environment}
run_mode: user
SUMMARY
configurer.run

expect(configurer.environment).to eq(Puppet[:environment])
expect(configurer.environment).to eq(node_environment.name.to_s)
end

it "uses environment from Puppet[:environment] if lastrunfile is invalid YAML" do
it "uses the environment from the node request if lastrunfile does not contain the expected keys" do
Puppet[:lastrunfile] = file_containing('last_run_summary.yaml', <<~SUMMARY)
Key: 'this is my very very very ' +
'long string'
---
version:
config: 1624882680
puppet: 6.24.0
SUMMARY
configurer.run

expect(configurer.environment).to eq(Puppet[:environment])
expect(configurer.environment).to eq(node_environment.name.to_s)
end

it "uses environment from Puppet[:environment] if lastrunfile exists but is empty" do
it "uses the environment from the node request if lastrunfile is invalid YAML" do
Puppet[:lastrunfile] = file_containing('last_run_summary.yaml', <<~SUMMARY)
Key: 'this is my very very very ' +
'long string'
SUMMARY
configurer.run

expect(configurer.environment).to eq(node_environment.name.to_s)
end

it "uses the environment from the node request if lastrunfile exists but is empty" do
Puppet[:lastrunfile] = file_containing('last_run_summary.yaml', '')
configurer.run

expect(configurer.environment).to eq(Puppet[:environment])
expect(configurer.environment).to eq(node_environment.name.to_s)
end

it "uses the environment from the node request if the last used one cannot be found" do
Puppet[:lastrunfile] = tmpfile('last_run_summary.yaml')
configurer.run

expect(configurer.environment).to eq(node_environment.name.to_s)
end
end

describe "when the node request fails" do
before do
allow(Puppet::Node.indirection).to receive(:find).and_call_original
allow(Puppet::Node.indirection).to receive(:find)
.with(anything, hash_including(:ignore_cache => true, :fail_on_404 => true))
.and_raise(Puppet::Error)
end

it "uses environment from Puppet[:environment] if the last used one cannot be found" do
it "uses the environment from Puppet[:environment] if the last used one cannot be found" do
Puppet[:lastrunfile] = tmpfile('last_run_summary.yaml')
configurer.run

Expand Down

0 comments on commit f0214e6

Please sign in to comment.