(maint) Add testing around exec via Puppet

acceptance/lib/pxp-agent/test_helper.rb

@@ -291,11 +291,6 @@ def rpc_request(broker, targets,
     end
   end
 
-  message = PCP::Message.new({
-    :message_type => blocking ? 'http://puppetlabs.com/rpc_blocking_request' : 'http://puppetlabs.com/rpc_non_blocking_request',
-    :targets => targets
-  })
-
   message_data = {
     :transaction_id => transaction_id,
     :module         => pxp_module,
@@ -305,13 +300,22 @@ def rpc_request(broker, targets,
   if !blocking then
     message_data[:notify_outcome] = false
   end
-  message.data = message_data.to_json
 
-  message_expiry = 10 # Seconds for the PCP message to be considered failed
-  rpc_action_expiry = 60 # Seconds for the entire RPC action to be considered failed
-  message.expires(message_expiry)
+  targets.each do |target|
+    message = PCP::Message.new({
+      :message_type => blocking ? 'http://puppetlabs.com/rpc_blocking_request' : 'http://puppetlabs.com/rpc_non_blocking_request',
+      :targets => [target]
+    })
 
-  client.send(message)
+    message.data = message_data.to_json
+
+    message_expiry = 10 # Seconds for the PCP message to be considered failed
+    message.expires(message_expiry)
+
+    client.send(message)
+  end
+
+  rpc_action_expiry = 60 # Seconds for the entire RPC action to be considered failed
 
   begin
     Timeout::timeout(rpc_action_expiry) do

acceptance/tests/pxp-module-puppet/run_puppet_exec.rb

@@ -0,0 +1,87 @@
+require 'pxp-agent/test_helper.rb'
+require 'puppet/acceptance/environment_utils'
+
+test_name 'C98107 - Run puppet with an exec' do
+
+  extend Puppet::Acceptance::EnvironmentUtils
+
+  env_name = test_file_name = File.basename(__FILE__, '.*')
+  environment_name = mk_tmp_environment(env_name)
+
+  step 'On master, create a new environment that execs hostname' do
+    site_manifest = "#{environmentpath}/#{environment_name}/manifests/site.pp"
+    create_remote_file(master, site_manifest, <<-SITEPP)
+node default {
+  exec { "hostname":
+    path => ["/bin", "/usr/bin", "C:/cygwin32/bin", "C:/cygwin64/bin"],
+    logoutput => true,
+  }
+}
+SITEPP
+    on(master, "chmod 644 #{site_manifest}")
+  end
+
+  step 'Ensure each agent host has pxp-agent running and associated' do
+    agents.each do |agent|
+      on agent, puppet('resource service pxp-agent ensure=stopped')
+      create_remote_file(agent, pxp_agent_config_file(agent), pxp_config_json_using_puppet_certs(master, agent).to_s)
+      on agent, puppet('resource service pxp-agent ensure=running')
+
+      assert(is_associated?(master, "pcp://#{agent}/agent"),
+             "Agent #{agent} with PCP identity pcp://#{agent}/agent should be associated with pcp-broker")
+    end
+  end
+
+  def last_run_report(h)
+    if h['platform'] =~ /windows/
+      'C:/ProgramData/PuppetLabs/puppet/cache'
+    else
+      '/opt/puppetlabs/puppet/cache'
+    end + '/state/last_run_report.yaml'
+  end
+
+  step "Send an rpc_blocking_request to all agents" do
+    target_identities = []
+    agents.each do |agent|
+      target_identities << "pcp://#{agent}/agent"
+    end
+    responses = nil # Declare here so not local to begin/rescue below
+    begin
+      responses = rpc_blocking_request(master, target_identities,
+                                      'pxp-module-puppet', 'run',
+                                      {:env => [], :flags => ['--environment', environment_name]})
+    rescue => exception
+      fail("Exception occurred when trying to run Puppet on all agents: #{exception.message}")
+    end
+    agents.each_with_index do |agent|
+      step "Check Run Puppet response for #{agent}" do
+        identity = "pcp://#{agent}/agent"
+        assert(responses[identity][:data].has_key?("results"),
+              "Agent #{agent} received a response for rpc_blocking_request but it is missing a 'results' entry: #{responses[identity]}")
+        action_result = responses[identity][:data]["results"]
+        # The test's pass/fail criteria is only the value of 'status'. However, if something goes wrong and Puppet needs to default
+        # the environment to 'production' and results in 'unchanged' then it's better to fail specifically on the environment.
+        assert(action_result.has_key?('environment'), "Results for pxp-module-puppet run on #{agent} should contain an 'environment' field")
+        assert_equal(environment_name, action_result['environment'], "Result of pxp-module-puppet run on #{agent} should run with the "\
+                                                                     "#{environment_name} environment")
+        assert(action_result.has_key?('status'), "Results for pxp-module-puppet run on #{agent} should contain a 'status' field")
+        assert_equal('changed', action_result['status'], "Result of pxp-module-puppet run on #{agent} should be 'changed'")
+      end
+
+      step "Check run report for #{agent}" do
+        result = on(agent, "cat #{last_run_report(agent)}")
+
+        # Parse the last run report, ignoring object tags
+        data = YAML.parse(result.stdout)
+        data.root.each do |o|
+          o.tag = nil if o.respond_to?(:tag=)
+        end
+        data = data.to_ruby
+
+        hostname = on(agent, 'hostname').stdout.chomp
+        expected = data['logs'].select {|log| log['source'] =~ /Exec\[hostname\]/}.select {|log| log['message'] == hostname}
+        assert_equal(1, expected.count, 'puppet failed to exec hostname')
+      end
+    end
+  end
+end

acceptance/tests/pxp-module-puppet/run_puppet_killed_puppet.rb

@@ -52,11 +52,6 @@
     transaction_ids = start_puppet_non_blocking_request(master, target_identities)
   end
 
-  teardown do
-    # Make sure we stop sleep processes when the test finishes, to avoid leaving stranded processes running.
-    stop_sleep_process(agents)
-  end
-
   agents.each do |agent|
     step "Kill the first agent run on #{agent}" do
       lockfile = nil
@@ -75,6 +70,10 @@
       else
         on agent, "kill -9 #{pid}"
       end
+
+      # Also halt the sleep process here, as on Windows the parent process won't exit while the child process
+      # has an open handle shared with it (the stdout pipe).
+      stop_sleep_process(agent)
     end
   end