diff --git a/README.md b/README.md index b0e25dcb..45f9749d 100644 --- a/README.md +++ b/README.md @@ -361,13 +361,13 @@ Tickets: File bug tickets at https://tickets.puppet.com/browse/PCP and add the [nssm]: https://nssm.cc [modules_docs]: https://github.com/puppetlabs/pxp-agent/blob/master/modules/README.md [pcp-broker]: https://github.com/puppetlabs/pcp-broker -[pcp_specs_root]: https://github.com/puppetlabs/pcp-specifications/blob/master/pcp/README.md +[pcp_specs_root]: https://github.com/puppetlabs/pcp-specifications/blob/master/pcp/versions/1.0/README.md [pxp-module-puppet_docs]: https://github.com/puppetlabs/pxp-agent/blob/master/lib/tests/resources/modules/reverse_valid [pxp-module-puppet_script]: https://github.com/puppetlabs/pxp-agent/blob/master/modules/pxp-module-puppet.md -[pxp_specs_actions]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/actions.md -[pxp_specs_request_response]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/request_response.md -[pxp_specs_root]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/README.md -[pxp_specs_transaction_status]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/transaction_status.md +[pxp_specs_actions]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/versions/1.0/actions.md +[pxp_specs_request_response]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/versions/1.0/request_response.md +[pxp_specs_root]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/versions/1.0/README.md +[pxp_specs_transaction_status]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/versions/1.0/transaction_status.md [websocketpp]: https://github.com/zaphoyd/websocketpp [MinGW-w64]: http://mingw-w64.sourceforge.net/ [Chocolatey]: https://chocolatey.org diff --git a/acceptance/lib/pxp-agent/test_helper.rb b/acceptance/lib/pxp-agent/test_helper.rb index e32eabae..dbd0f68f 100644 --- a/acceptance/lib/pxp-agent/test_helper.rb +++ b/acceptance/lib/pxp-agent/test_helper.rb @@ -128,7 +128,7 @@ def assert_eventmachine_thread_running end # Query pcp-broker's inventory of associated clients -# Reference: https://github.com/puppetlabs/pcp-specifications/blob/master/pcp/inventory.md +# Reference: https://github.com/puppetlabs/pcp-specifications/blob/master/pcp/versions/1.0/inventory.md # @param broker hostname or beaker host object of the pcp-broker host # @param query pattern of client identities that you want to check e.g. pcp://*/agent for all agents, or pcp://*/* for everything # @return Ruby array of strings that are the client identities that match the query pattern @@ -233,7 +233,7 @@ def is_not_associated?(broker, identity, retries = 60) end # Make an rpc_blocking_request to pxp-agent -# Reference: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/request_response.md +# Reference: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/versions/1.0/request_response.md # @param broker hostname or beaker host object of the machine running PCP broker # @param targets array of PCP identities to send request to # e.g. ["pcp://client01.example.com/agent","pcp://client02.example.com/agent"] @@ -250,7 +250,7 @@ def rpc_blocking_request(broker, targets, end # Make an rpc_non_blocking_request to pxp-agent -# Reference: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/request_response.md +# Reference: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/versions/1.0/request_response.md # @param broker hostname or beaker host object of the machine running PCP broker # @param targets array of PCP identities to send request to # e.g. ["pcp://client01.example.com/agent","pcp://client02.example.com/agent"] diff --git a/modules/README.md b/modules/README.md index fa077d99..1fec57a5 100644 --- a/modules/README.md +++ b/modules/README.md @@ -192,4 +192,4 @@ the [PXP Transaction Response][transaction_status]. The only reserved code is `5`; such code should be used in case the `output_files` entry was included, but the module failed to write the action's results on file. -[transaction_status]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/transaction_status.md +[transaction_status]: https://github.com/puppetlabs/pcp-specifications/blob/master/pxp/versions/1.0/transaction_status.md diff --git a/modules/pxp-module-puppet b/modules/pxp-module-puppet index 1d595d50..30e64716 100755 --- a/modules/pxp-module-puppet +++ b/modules/pxp-module-puppet @@ -83,16 +83,15 @@ def make_error_result(exitcode, error_type, error_message) return result end -def get_result_from_report(exitcode, config, start_time) - last_run_report = check_config_print("lastrunreport",config) - if last_run_report.empty? +def get_result_from_report(last_run_report, exitcode, config, start_time) + if !File.exist?(last_run_report) return make_error_result(exitcode, Errors::NoLastRunReport, - "could not find the location of the last run report") + "#{last_run_report} doesn't exist") end - if !File.exist?(last_run_report) + if start_time && File.mtime(last_run_report) == start_time return make_error_result(exitcode, Errors::NoLastRunReport, - "#{last_run_report} doesn't exist") + "#{last_run_report} was not written") end last_run_report_yaml = {} @@ -111,13 +110,11 @@ def get_result_from_report(exitcode, config, start_time) "Puppet agent exited with a non 0 exitcode") end - if start_time.to_i < last_run_report_yaml.time.to_i - run_result["kind"] = last_run_report_yaml.kind - run_result["time"] = last_run_report_yaml.time - run_result["transaction_uuid"] = last_run_report_yaml.transaction_uuid - run_result["environment"] = last_run_report_yaml.environment - run_result["status"] = last_run_report_yaml.status - end + run_result["kind"] = last_run_report_yaml.kind + run_result["time"] = last_run_report_yaml.time + run_result["transaction_uuid"] = last_run_report_yaml.transaction_uuid + run_result["environment"] = last_run_report_yaml.environment + run_result["status"] = last_run_report_yaml.status return run_result end @@ -126,7 +123,13 @@ def start_run(config, action_input) exitcode = DEFAULT_EXITCODE cmd = make_command_array(config, action_input) env = make_environment_hash(action_input) - start_time = Time.now + + last_run_report = check_config_print("lastrunreport", config) + if last_run_report.empty? + return make_error_result(exitcode, Errors::NoLastRunReport, + "could not find the location of the last run report") + end + start_time = File.mtime(last_run_report) if File.exist?(last_run_report) run_result = Puppet::Util::Execution.execute(cmd, {:failonfail => false, :custom_environment => env}) @@ -147,7 +150,7 @@ def start_run(config, action_input) "Puppet agent is already performing a run") end - return get_result_from_report(exitcode, config, start_time) + return get_result_from_report(last_run_report, exitcode, config, start_time) end def metadata() diff --git a/modules/spec/unit/modules/puppet_spec.rb b/modules/spec/unit/modules/puppet_spec.rb index c7d16022..fb34888d 100644 --- a/modules/spec/unit/modules/puppet_spec.rb +++ b/modules/spec/unit/modules/puppet_spec.rb @@ -102,33 +102,37 @@ end describe "get_result_from_report" do + let(:last_run_report_path) { + "/opt/puppetlabs/puppet/cache/state/fake_last_run_report.yaml" + } + it "doesn't process the last_run_report if the file doens't exist" do allow(File).to receive(:exist?).and_return(false) - allow_any_instance_of(Object).to receive(:check_config_print).and_return("/opt/puppetlabs/puppet/cache/state/last_run_report.yaml") - expect(get_result_from_report(0, default_configuration, Time.now)).to be == + expect(get_result_from_report(last_run_report_path, 0, default_configuration, Time.now)).to be == {"kind" => "unknown", "time" => "unknown", "transaction_uuid" => "unknown", "environment" => "unknown", "status" => "unknown", "error_type" => "no_last_run_report", - "error" => "/opt/puppetlabs/puppet/cache/state/last_run_report.yaml doesn't exist", + "error" => "#{last_run_report_path} doesn't exist", "exitcode" => 0, "version" => 1} end it "doesn't process the last_run_report if the file cant be loaded" do + start_time = Time.now + allow(File).to receive(:mtime).and_return(start_time+1) allow(File).to receive(:exist?).and_return(true) - allow_any_instance_of(Object).to receive(:check_config_print).and_return("/opt/puppetlabs/puppet/cache/state/last_run_report.yaml") allow(YAML).to receive(:load_file).and_raise("error") - expect(get_result_from_report(0, default_configuration, Time.now)).to be == + expect(get_result_from_report(last_run_report_path, 0, default_configuration, start_time)).to be == {"kind" => "unknown", "time" => "unknown", "transaction_uuid" => "unknown", "environment" => "unknown", "status" => "unknown", "error_type" => "invalid_last_run_report", - "error" => "/opt/puppetlabs/puppet/cache/state/last_run_report.yaml could not be loaded: error", + "error" => "#{last_run_report_path} could not be loaded: error", "exitcode" => 0, "version" => 1} end @@ -144,18 +148,18 @@ allow(last_run_report).to receive(:environment).and_return("production") allow(last_run_report).to receive(:status).and_return("changed") + allow(File).to receive(:mtime).and_return(start_time) allow(File).to receive(:exist?).and_return(true) - allow_any_instance_of(Object).to receive(:check_config_print).and_return("/opt/puppetlabs/puppet/cache/state/last_run_report.yaml") - allow(YAML).to receive(:load_file).and_return(last_run_report) + allow(YAML).to receive(:load_file).with(last_run_report_path).and_return(last_run_report) - expect(get_result_from_report(-1, default_configuration, start_time)).to be == + expect(get_result_from_report(last_run_report_path, -1, default_configuration, start_time)).to be == {"kind" => "unknown", "time" => "unknown", "transaction_uuid" => "unknown", "environment" => "unknown", "status" => "unknown", - "error_type" => "agent_exit_non_zero", - "error" => "Puppet agent exited with a non 0 exitcode", + "error_type" => "no_last_run_report", + "error" => "#{last_run_report_path} was not written", "exitcode" => -1, "version" => 1} end @@ -171,11 +175,11 @@ allow(last_run_report).to receive(:environment).and_return("production") allow(last_run_report).to receive(:status).and_return("changed") + allow(File).to receive(:mtime).and_return(start_time+1) allow(File).to receive(:exist?).and_return(true) - allow_any_instance_of(Object).to receive(:check_config_print).and_return("/opt/puppetlabs/puppet/cache/state/last_run_report.yaml") - allow(YAML).to receive(:load_file).and_return(last_run_report) + allow(YAML).to receive(:load_file).with(last_run_report_path).and_return(last_run_report) - expect(get_result_from_report(-1, default_configuration, start_time)).to be == + expect(get_result_from_report(last_run_report_path, -1, default_configuration, start_time)).to be == {"kind" => "apply", "time" => run_time, "transaction_uuid" => "ac59acbe-6a0f-49c9-8ece-f781a689fda9", @@ -198,11 +202,11 @@ allow(last_run_report).to receive(:environment).and_return("production") allow(last_run_report).to receive(:status).and_return("changed") + allow(File).to receive(:mtime).and_return(start_time+1) allow(File).to receive(:exist?).and_return(true) - allow_any_instance_of(Object).to receive(:check_config_print).and_return("/opt/puppetlabs/puppet/cache/state/last_run_report.yaml") - allow(YAML).to receive(:load_file).and_return(last_run_report) + allow(YAML).to receive(:load_file).with(last_run_report_path).and_return(last_run_report) - expect(get_result_from_report(0, default_configuration, start_time)).to be == + expect(get_result_from_report(last_run_report_path, 0, default_configuration, start_time)).to be == {"kind" => "apply", "time" => run_time, "transaction_uuid" => "ac59acbe-6a0f-49c9-8ece-f781a689fda9", @@ -218,17 +222,25 @@ double(:runoutcome) } + let(:last_run_report) { + "/opt/puppetlabs/puppet/cache/state/last_run_report.yaml" + } + + before :each do + allow_any_instance_of(Object).to receive(:check_config_print).and_return(last_run_report) + end + it "populates output when it terminated normally" do allow(Puppet::Util::Execution).to receive(:execute).and_return(runoutcome) allow(runoutcome).to receive(:exitstatus).and_return(0) - expect_any_instance_of(Object).to receive(:get_result_from_report).with(0, default_configuration, anything) + expect_any_instance_of(Object).to receive(:get_result_from_report).with(last_run_report, 0, default_configuration, anything) start_run(default_configuration, default_input) end it "populates output when it terminated with a non 0 code" do allow(Puppet::Util::Execution).to receive(:execute).and_return(runoutcome) allow(runoutcome).to receive(:exitstatus).and_return(1) - expect_any_instance_of(Object).to receive(:get_result_from_report).with(1, default_configuration, anything) + expect_any_instance_of(Object).to receive(:get_result_from_report).with(last_run_report, 1, default_configuration, anything) start_run(default_configuration, default_input) end