Merge branch 'ticket/2.6.next/5493' into 2.6.next

15 files changed, 77 insertions, 91 deletions

diff --git a/lib/puppet/application/apply.rb b/lib/puppet/application/apply.rb
index 59a95d35a..33a70ce8a 100644
--- a/lib/puppet/application/apply.rb
+++ b/lib/puppet/application/apply.rb
@@ -123,25 +123,9 @@ class Puppet::Application::Apply < Puppet::Application
 
       require 'puppet/configurer'
       configurer = Puppet::Configurer.new
-      configurer.execute_prerun_command
+      report = configurer.run(:skip_plugin_download => true, :catalog => catalog)
 
-      # And apply it
-      if Puppet[:report]
-        report = configurer.initialize_report
-        Puppet::Util::Log.newdestination(report)
-      end
-      transaction = catalog.apply
-
-      configurer.execute_postrun_command
-
-      if Puppet[:report]
-        Puppet::Util::Log.close(report)
-        configurer.send_report(report, transaction)
-      else
-        transaction.generate_report
-      end
-
-      exit( Puppet[:noop] ? 0 : options[:detailed_exitcodes] ? transaction.report.exit_status : 0 )
+      exit( Puppet[:noop] ? 0 : options[:detailed_exitcodes] ? report.exit_status : 0 )
     rescue => detail
       puts detail.backtrace if Puppet[:trace]
       $stderr.puts detail.message
diff --git a/lib/puppet/application/inspect.rb b/lib/puppet/application/inspect.rb
index c28fef326..caa32a7c2 100644
--- a/lib/puppet/application/inspect.rb
+++ b/lib/puppet/application/inspect.rb
@@ -49,6 +49,8 @@ class Puppet::Application::Inspect < Puppet::Application
       raise "Could not find catalog for #{Puppet[:certname]}"
     end
 
+    @report.configuration_version = catalog.version
+
     retrieval_time =  Time.now - retrieval_starttime
     @report.add_times("config_retrieval", retrieval_time)
 
diff --git a/lib/puppet/configurer.rb b/lib/puppet/configurer.rb
index 31d31c2d2..070176554 100644
--- a/lib/puppet/configurer.rb
+++ b/lib/puppet/configurer.rb
@@ -72,17 +72,13 @@ class Puppet::Configurer
     @splayed = false
   end
 
-  def initialize_report
-    Puppet::Transaction::Report.new
-  end
-
   # Prepare for catalog retrieval.  Downloads everything necessary, etc.
-  def prepare
+  def prepare(options)
     dostorage
 
-    download_plugins
+    download_plugins unless options[:skip_plugin_download]
 
-    download_fact_plugins
+    download_fact_plugins unless options[:skip_plugin_download]
 
     execute_prerun_command
   end
@@ -126,7 +122,7 @@ class Puppet::Configurer
   # which accepts :tags and :ignoreschedules.
   def run(options = {})
     begin
-      prepare
+      prepare(options)
     rescue SystemExit,NoMemoryError
       raise
     rescue Exception => detail
@@ -134,7 +130,7 @@ class Puppet::Configurer
       Puppet.err "Failed to prepare catalog: #{detail}"
     end
 
-    options[:report] ||= initialize_report
+    options[:report] ||= Puppet::Transaction::Report.new("apply")
     report = options[:report]
     Puppet::Util::Log.newdestination(report)
 
@@ -145,6 +141,8 @@ class Puppet::Configurer
       return
     end
 
+    report.configuration_version = catalog.version
+
     transaction = nil
 
     begin
diff --git a/lib/puppet/transaction.rb b/lib/puppet/transaction.rb
index dcd9aad0a..2d49062dd 100644
--- a/lib/puppet/transaction.rb
+++ b/lib/puppet/transaction.rb
@@ -238,7 +238,7 @@ class Puppet::Transaction
   def initialize(catalog)
     @catalog = catalog
 
-    @report = Report.new
+    @report = Report.new("apply", catalog.version)
 
     @event_manager = Puppet::Transaction::EventManager.new(self)
 
diff --git a/lib/puppet/transaction/report.rb b/lib/puppet/transaction/report.rb
index 75c08fc7a..8a928454f 100644
--- a/lib/puppet/transaction/report.rb
+++ b/lib/puppet/transaction/report.rb
@@ -10,6 +10,7 @@ class Puppet::Transaction::Report
 
   indirects :report, :terminus_class => :processor
 
+  attr_accessor :configuration_version
   attr_reader :resource_statuses, :logs, :metrics, :host, :time, :kind
 
   # This is necessary since Marshall doesn't know how to
@@ -49,7 +50,7 @@ class Puppet::Transaction::Report
     calculate_event_metrics
   end
 
-  def initialize(kind = "apply")
+  def initialize(kind, configuration_version=nil)
     @metrics = {}
     @logs = []
     @resource_statuses = {}
@@ -57,6 +58,9 @@ class Puppet::Transaction::Report
     @host = Puppet[:certname]
     @time = Time.now
     @kind = kind
+    @report_format = 2
+    @puppet_version = Puppet.version
+    @configuration_version = configuration_version
   end
 
   def name
diff --git a/spec/integration/indirector/report/rest_spec.rb b/spec/integration/indirector/report/rest_spec.rb
index fdc218975..7fa026b73 100644
--- a/spec/integration/indirector/report/rest_spec.rb
+++ b/spec/integration/indirector/report/rest_spec.rb
@@ -64,7 +64,7 @@ describe "Report REST Terminus" do
   it "should be able to send a report to the server" do
     @report.expects(:save)
 
-    report = Puppet::Transaction::Report.new
+    report = Puppet::Transaction::Report.new("apply")
 
     resourcemetrics = {
       :total => 12,
diff --git a/spec/integration/transaction/report_spec.rb b/spec/integration/transaction/report_spec.rb
index eed7acaa9..e7d952eb2 100755
--- a/spec/integration/transaction/report_spec.rb
+++ b/spec/integration/transaction/report_spec.rb
@@ -19,7 +19,7 @@ describe Puppet::Transaction::Report do
 
       Facter.stubs(:value).returns "host.domain.com"
 
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
 
       terminus.expects(:process).with(report)
 
diff --git a/spec/unit/application/apply_spec.rb b/spec/unit/application/apply_spec.rb
index 877c47bcc..4e1744206 100755
--- a/spec/unit/application/apply_spec.rb
+++ b/spec/unit/application/apply_spec.rb
@@ -4,6 +4,7 @@ require File.dirname(__FILE__) + '/../../spec_helper'
 
 require 'puppet/application/apply'
 require 'puppet/file_bucket/dipper'
+require 'puppet/configurer'
 
 describe Puppet::Application::Apply do
   before :each do
@@ -194,6 +195,9 @@ describe Puppet::Application::Apply do
         @catalog.stubs(:apply).returns(@transaction)
 
         @apply.stubs(:exit)
+
+        Puppet::Util::Storage.stubs(:load)
+        Puppet::Configurer.any_instance.stubs(:save_last_run_summary) # to prevent it from trying to write files
       end
 
       it "should set the code to run from --code" do
@@ -302,11 +306,8 @@ describe Puppet::Application::Apply do
       end
 
       it "should call the prerun and postrun commands on a Configurer instance" do
-        configurer = stub 'configurer'
-
-        Puppet::Configurer.expects(:new).returns configurer
-        configurer.expects(:execute_prerun_command)
-        configurer.expects(:execute_postrun_command)
+        Puppet::Configurer.any_instance.expects(:execute_prerun_command)
+        Puppet::Configurer.any_instance.expects(:execute_postrun_command)
 
         @apply.main
       end
@@ -321,8 +322,7 @@ describe Puppet::Application::Apply do
         it "should exit with report's computed exit status" do
           Puppet.stubs(:[]).with(:noop).returns(false)
           @apply.options.stubs(:[]).with(:detailed_exitcodes).returns(true)
-          report = stub 'report', :exit_status => 666
-          @transaction.stubs(:report).returns(report)
+          Puppet::Transaction::Report.any_instance.stubs(:exit_status).returns(666)
           @apply.expects(:exit).with(666)
 
           @apply.main
diff --git a/spec/unit/configurer_spec.rb b/spec/unit/configurer_spec.rb
index ebc5768ea..3b2a44f0f 100755
--- a/spec/unit/configurer_spec.rb
+++ b/spec/unit/configurer_spec.rb
@@ -72,14 +72,6 @@ describe Puppet::Configurer do
   end
 end
 
-describe Puppet::Configurer, "when initializing a report" do
-  it "should return an instance of a transaction report" do
-    Puppet.settings.stubs(:use).returns(true)
-    @agent = Puppet::Configurer.new
-    @agent.initialize_report.should be_instance_of(Puppet::Transaction::Report)
-  end
-end
-
 describe Puppet::Configurer, "when executing a catalog run" do
   before do
     Puppet.settings.stubs(:use).returns(true)
@@ -101,31 +93,31 @@ describe Puppet::Configurer, "when executing a catalog run" do
   end
 
   it "should initialize a transaction report if one is not provided" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns report
 
     @agent.run
   end
 
   it "should pass the new report to the catalog" do
-    report = stub 'report'
-    @agent.stubs(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.stubs(:new).returns report
     @catalog.expects(:apply).with{|options| options[:report] == report}
 
     @agent.run
   end
 
   it "should use the provided report if it was passed one" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).never
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).never
     @catalog.expects(:apply).with{|options| options[:report] == report}
 
     @agent.run(:report => report)
   end
 
   it "should set the report as a log destination" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns report
 
     Puppet::Util::Log.expects(:newdestination).with(report)
 
@@ -176,16 +168,16 @@ describe Puppet::Configurer, "when executing a catalog run" do
   end
 
   it "should send the report" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
     @agent.expects(:send_report).with { |r, trans| r == report }
 
     @agent.run
   end
 
   it "should send the transaction report with a reference to the transaction if a run was actually made" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
 
     trans = stub 'transaction'
     @catalog.expects(:apply).returns trans
@@ -198,8 +190,8 @@ describe Puppet::Configurer, "when executing a catalog run" do
   it "should send the transaction report even if the catalog could not be retrieved" do
     @agent.expects(:retrieve_catalog).returns nil
 
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
     @agent.expects(:send_report)
 
     @agent.run
@@ -208,16 +200,16 @@ describe Puppet::Configurer, "when executing a catalog run" do
   it "should send the transaction report even if there is a failure" do
     @agent.expects(:retrieve_catalog).raises "whatever"
 
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
     @agent.expects(:send_report)
 
     lambda { @agent.run }.should raise_error
   end
 
   it "should remove the report as a log destination when the run is finished" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
 
     Puppet::Util::Log.expects(:close).with(report)
 
@@ -225,8 +217,8 @@ describe Puppet::Configurer, "when executing a catalog run" do
   end
 
   it "should return the report as the result of the run" do
-    report = stub 'report'
-    @agent.expects(:initialize_report).returns report
+    report = Puppet::Transaction::Report.new("apply")
+    Puppet::Transaction::Report.expects(:new).returns(report)
 
     @agent.run.should equal(report)
   end
@@ -237,7 +229,7 @@ describe Puppet::Configurer, "when sending a report" do
     Puppet.settings.stubs(:use).returns(true)
     @configurer = Puppet::Configurer.new
 
-    @report = stub 'report'
+    @report = Puppet::Transaction::Report.new("apply")
     @trans = stub 'transaction'
   end
 
@@ -472,23 +464,23 @@ describe Puppet::Configurer, "when preparing for a run" do
   it "should initialize the metadata store" do
     @agent.class.stubs(:facts).returns(@facts)
     @agent.expects(:dostorage)
-    @agent.prepare
+    @agent.prepare({})
   end
 
   it "should download fact plugins" do
     @agent.expects(:download_fact_plugins)
 
-    @agent.prepare
+    @agent.prepare({})
   end
 
   it "should download plugins" do
     @agent.expects(:download_plugins)
 
-    @agent.prepare
+    @agent.prepare({})
   end
 
   it "should perform the pre-run commands" do
     @agent.expects(:execute_prerun_command)
-    @agent.prepare
+    @agent.prepare({})
   end
 end
diff --git a/spec/unit/reports/http_spec.rb b/spec/unit/reports/http_spec.rb
index c814975df..70742f7dc 100644
--- a/spec/unit/reports/http_spec.rb
+++ b/spec/unit/reports/http_spec.rb
@@ -18,7 +18,7 @@ processor = Puppet::Reports.report(:http)
 
 describe processor do
   before  { Net::HTTP.any_instance.stubs(:start).yields(FakeHTTP) }
-  subject { Puppet::Transaction::Report.new.extend(processor) }
+  subject { Puppet::Transaction::Report.new("apply").extend(processor) }
 
   it { should respond_to(:process) }
 
diff --git a/spec/unit/reports/tagmail_spec.rb b/spec/unit/reports/tagmail_spec.rb
index bdb16600e..1dadfc7cd 100755
--- a/spec/unit/reports/tagmail_spec.rb
+++ b/spec/unit/reports/tagmail_spec.rb
@@ -11,7 +11,7 @@ describe tagmail do
   extend PuppetTest::Support::Utils
 
   before do
-    @processor = Puppet::Transaction::Report.new
+    @processor = Puppet::Transaction::Report.new("apply")
     @processor.extend(Puppet::Reports.report(:tagmail))
   end
 
diff --git a/spec/unit/transaction/report_spec.rb b/spec/unit/transaction/report_spec.rb
index 77f82159b..604c2f54d 100755
--- a/spec/unit/transaction/report_spec.rb
+++ b/spec/unit/transaction/report_spec.rb
@@ -11,30 +11,36 @@ describe Puppet::Transaction::Report do
 
   it "should set its host name to the certname" do
     Puppet.settings.expects(:value).with(:certname).returns "myhost"
-    Puppet::Transaction::Report.new.host.should == "myhost"
+    Puppet::Transaction::Report.new("apply").host.should == "myhost"
   end
 
   it "should return its host name as its name" do
-    r = Puppet::Transaction::Report.new
+    r = Puppet::Transaction::Report.new("apply")
     r.name.should == r.host
   end
 
   it "should create an initialization timestamp" do
     Time.expects(:now).returns "mytime"
-    Puppet::Transaction::Report.new.time.should == "mytime"
-  end
-
-  it "should have a default 'kind' of 'apply'" do
-    Puppet::Transaction::Report.new.kind.should == "apply"
+    Puppet::Transaction::Report.new("apply").time.should == "mytime"
   end
 
   it "should take a 'kind' as an argument" do
     Puppet::Transaction::Report.new("inspect").kind.should == "inspect"
   end
 
+  it "should take a 'configuration_version' as an argument" do
+    Puppet::Transaction::Report.new("inspect", "some configuration version").configuration_version.should == "some configuration version"
+  end
+
+  it "should be able to set configuration_version" do
+    report = Puppet::Transaction::Report.new("inspect")
+    report.configuration_version = "some version"
+    report.configuration_version.should == "some version"
+  end
+
   describe "when accepting logs" do
     before do
-      @report = Puppet::Transaction::Report.new
+      @report = Puppet::Transaction::Report.new("apply")
     end
 
     it "should add new logs to the log list" do
@@ -50,7 +56,7 @@ describe Puppet::Transaction::Report do
 
   describe "when accepting resource statuses" do
     before do
-      @report = Puppet::Transaction::Report.new
+      @report = Puppet::Transaction::Report.new("apply")
     end
 
     it "should add each status to its status list" do
@@ -72,7 +78,7 @@ describe Puppet::Transaction::Report do
       Facter.stubs(:value).returns("eh")
       @indirection = stub 'indirection', :name => :report
       Puppet::Transaction::Report.stubs(:indirection).returns(@indirection)
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
       @indirection.expects(:save)
       report.save
     end
@@ -82,7 +88,7 @@ describe Puppet::Transaction::Report do
     end
 
     it "should delegate its name attribute to its host method" do
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
       report.expects(:host).returns "me"
       report.name.should == "me"
     end
@@ -94,21 +100,21 @@ describe Puppet::Transaction::Report do
 
   describe "when computing exit status" do
     it "should produce 2 if changes are present" do
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
       report.add_metric("changes", {:total => 1})
       report.add_metric("resources", {:failed => 0})
       report.exit_status.should == 2
     end
 
     it "should produce 4 if failures are present" do
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
       report.add_metric("changes", {:total => 0})
       report.add_metric("resources", {:failed => 1})
       report.exit_status.should == 4
     end
 
     it "should produce 6 if both changes and failures are present" do
-      report = Puppet::Transaction::Report.new
+      report = Puppet::Transaction::Report.new("apply")
       report.add_metric("changes", {:total => 1})
       report.add_metric("resources", {:failed => 1})
       report.exit_status.should == 6
@@ -117,7 +123,7 @@ describe Puppet::Transaction::Report do
 
   describe "when calculating metrics" do
     before do
-      @report = Puppet::Transaction::Report.new
+      @report = Puppet::Transaction::Report.new("apply")
     end
 
     def metric(name, value)
diff --git a/spec/unit/transaction_spec.rb b/spec/unit/transaction_spec.rb
index 2df4404be..566c90438 100755
--- a/spec/unit/transaction_spec.rb
+++ b/spec/unit/transaction_spec.rb
@@ -94,7 +94,7 @@ describe Puppet::Transaction do
   end
 
   it "should be possible to replace the report object" do
-    report = Puppet::Transaction::Report.new
+    report = Puppet::Transaction::Report.new("apply")
     @transaction.report = report
 
     @transaction.report.should == report
diff --git a/test/lib/puppettest/reporttesting.rb b/test/lib/puppettest/reporttesting.rb
index 448a6a9d8..54a2f6799 100644
--- a/test/lib/puppettest/reporttesting.rb
+++ b/test/lib/puppettest/reporttesting.rb
@@ -1,7 +1,7 @@
 module PuppetTest::Reporttesting
   def fakereport
     # Create a bunch of log messages in an array.
-    report = Puppet::Transaction::Report.new
+    report = Puppet::Transaction::Report.new("apply")
 
     3.times { |i|
       # We have to use warning so that the logs always happen
diff --git a/test/other/report.rb b/test/other/report.rb
index 8a909b41c..eacf1632b 100755
--- a/test/other/report.rb
+++ b/test/other/report.rb
@@ -68,7 +68,7 @@ class TestReports < Test::Unit::TestCase
 
   def test_store_report
     # Create a bunch of log messages in an array.
-    report = Puppet::Transaction::Report.new
+    report = Puppet::Transaction::Report.new("apply")
 
     # We have to reuse reporting here because of something going on in the
     # server/report.rb file