Skip to content

Commit

Permalink
(PUP-11328) Get the environment's name without loading the environment
Browse files Browse the repository at this point in the history
The previous commit added back the node request as a fallback, but it was
calling `Node#environment`, which will try to load the environment from disk.
Since the server-assigned environment likely doesn't exist on the agent, the run
printed the following warning and continued using "production"

    Warning: Could not find a directory environment named 'xxx' anywhere in the
    path ... Does the directory exist?

To complicate matters, node termini set the environment on the node differently.
The :rest terminus returns a node with an environment_name, but not an instance.
The :plain terminus returns a node with an environment instance, but not an
environment_name. To avoid breaking things, retrieve the environment name the
same way we did before[1].

[1] https://github.com/puppetlabs/puppet/blob/6.24.0/lib/puppet/configurer.rb#L315-L319
  • Loading branch information
joshcooper authored and luchihoratiu committed Oct 27, 2021
1 parent f0214e6 commit 7a5e454
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 4 deletions.
12 changes: 11 additions & 1 deletion lib/puppet/configurer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -513,7 +513,17 @@ def current_server_specified_environment(current_environment, configured_environ
:transaction_uuid => @transaction_uuid,
:fail_on_404 => true)

@server_specified_environment = node.environment.name.to_s
# The :rest node terminus returns a node with an environment_name, but not an
# environment instance. Attempting to get the environment instance will load
# it from disk, which will likely fail. So create a remote environment.
#
# The :plain node terminus returns a node with an environment, but not an
# environment_name.
if !node.has_environment_instance? && node.environment_name
node.environment = Puppet::Node::Environment.remote(node.environment_name)
end

@server_specified_environment = node.environment.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 }
Expand Down
9 changes: 8 additions & 1 deletion spec/integration/application/agent_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -643,7 +643,14 @@ def with_another_agent_running(&block)

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|
mounts = {}
mounts[:node] = -> (req, res) {
node = Puppet::Node.new('test', environment: Puppet::Node::Environment.remote('doesnotexistonagent'))
res.body = formatter.render(node)
res['Content-Type'] = formatter.mime
}

server.start_server(mounts: mounts) do |port|
Puppet[:serverport] = port
Puppet[:log_level] = 'debug'

Expand Down
18 changes: 16 additions & 2 deletions spec/unit/configurer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
require 'puppet/configurer'

describe Puppet::Configurer do
include PuppetSpec::Files

before do
Puppet[:server] = "puppetmaster"
Puppet[:report] = true
Expand All @@ -10,6 +12,17 @@
allow_any_instance_of(described_class).to(
receive(:valid_server_environment?).and_return(true)
)

Puppet[:lastrunfile] = file_containing('last_run_summary.yaml', <<~SUMMARY)
---
version:
config: 1624882680
puppet: #{Puppet.version}
application:
initial_environment: #{Puppet[:environment]}
converged_environment: #{Puppet[:environment]}
run_mode: agent
SUMMARY
end

let(:node_name) { Puppet[:node_name_value] }
Expand Down Expand Up @@ -1099,7 +1112,6 @@ def expects_neither_new_or_cached_catalog
end

describe "when selecting an environment" do
include PuppetSpec::Files
include PuppetSpec::Settings

describe "when the last used environment is available" do
Expand Down Expand Up @@ -1205,10 +1217,12 @@ def expects_neither_new_or_cached_catalog
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(:node) { Puppet::Node.new(Puppet[:node_name_value]) }
let(:last_server_specified_environment) { 'development' }

before do
node.environment = node_environment

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))
Expand Down

0 comments on commit 7a5e454

Please sign in to comment.