From 95d9c5dd70a2de25fd4964f2836722ddc78ffee4 Mon Sep 17 00:00:00 2001
From: Ohad Levy <ohadlevy@gmail.com>
Date: Mon, 18 Oct 2010 15:21:49 +0200
Subject: [PATCH] Fixes #405 and Fixes #349 - Adds support to 2.6.x reports

Some of the tests needs some attention, however, this test should be
functional.

NOTE: Backup your DB prior to using this patch.
---
 app/models/log.rb                                  |   21 +++++
 app/models/message.rb                              |   10 ++
 app/models/report.rb                               |   87 ++++++++++++++++---
 app/models/source.rb                               |    9 ++
 app/views/host_mailer/error_state.text.html.erb    |    2 +-
 app/views/reports/show.rhtml                       |   12 ++--
 db/migrate/20101018120548_create_messages.rb       |   13 +++
 db/migrate/20101018120603_create_sources.rb        |   13 +++
 db/migrate/20101018120621_create_logs.rb           |   22 +++++
 db/migrate/20101019122857_add_metrics_to_report.rb |    9 ++
 db/migrate/20101019183859_convert_reports.rb       |   33 ++++++++
 test/fixtures/logs.yml                             |   11 +++
 test/fixtures/messages.yml                         |    7 ++
 test/fixtures/sources.yml                          |    7 ++
 test/unit/log_test.rb                              |    8 ++
 test/unit/message_test.rb                          |    8 ++
 test/unit/source_test.rb                           |    8 ++
 17 files changed, 259 insertions(+), 21 deletions(-)
 create mode 100644 app/models/log.rb
 create mode 100644 app/models/message.rb
 create mode 100644 app/models/source.rb
 create mode 100644 db/migrate/20101018120548_create_messages.rb
 create mode 100644 db/migrate/20101018120603_create_sources.rb
 create mode 100644 db/migrate/20101018120621_create_logs.rb
 create mode 100644 db/migrate/20101019122857_add_metrics_to_report.rb
 create mode 100644 db/migrate/20101019183859_convert_reports.rb
 create mode 100644 test/fixtures/logs.yml
 create mode 100644 test/fixtures/messages.yml
 create mode 100644 test/fixtures/sources.yml
 create mode 100644 test/unit/log_test.rb
 create mode 100644 test/unit/message_test.rb
 create mode 100644 test/unit/source_test.rb

diff --git a/app/models/log.rb b/app/models/log.rb
new file mode 100644
index 0000000..a3a3266
--- /dev/null
+++ b/app/models/log.rb
@@ -0,0 +1,21 @@
+class Log < ActiveRecord::Base
+  belongs_to :message
+  belongs_to :source
+  belongs_to :report
+  validates_presence_of :message_id, :source_id, :report_id, :level_id
+
+  LEVELS = [:debug, :info, :notice, :warning, :err, :alert, :emerg, :crit]
+
+  def to_s
+    "#{source} #{message}"
+  end
+
+  def level= l
+    write_attribute(:level_id, LEVELS.index(l))
+  end
+
+  def level
+    LEVELS[level_id]
+  end
+
+end
diff --git a/app/models/message.rb b/app/models/message.rb
new file mode 100644
index 0000000..7748300
--- /dev/null
+++ b/app/models/message.rb
@@ -0,0 +1,10 @@
+class Message < ActiveRecord::Base
+  has_many :reports, :through => :logs
+  has_many :logs
+  validates_presence_of :value
+
+  def to_s
+    value
+  end
+
+end
diff --git a/app/models/report.rb b/app/models/report.rb
index 9f0e15c..b81c5a7 100644
--- a/app/models/report.rb
+++ b/app/models/report.rb
@@ -1,7 +1,9 @@
 class Report < ActiveRecord::Base
   belongs_to :host
-  serialize :log, Puppet::Transaction::Report
-  validates_presence_of :log, :host_id, :reported_at, :status
+  has_many :messages, :through => :logs, :dependent => :destroy
+  has_many :sources, :through => :logs, :dependent => :destroy
+  has_many :logs, :dependent => :destroy
+  validates_presence_of :host_id, :reported_at, :status
   validates_uniqueness_of :reported_at, :scope => :host_id
 
   METRIC = %w[applied restarted failed failed_restarts skipped]
@@ -41,6 +43,16 @@ class Report < ActiveRecord::Base
     return type.nil? ? h : h[type]
   end
 
+  # extracts serialized metrics and keep them as a hash_with_indifferent_access
+  def metrics
+    YAML.load(read_attribute(:metrics)).with_indifferent_access
+  end
+
+  # serialize metrics as YAML
+  def metrics= m
+    write_attribute(:metrics,m.to_yaml) unless m.nil?
+  end
+
   # generate dynamically methods for all metrics
   # e.g. Report.last.applied
   METRIC.each do |method|
@@ -64,13 +76,11 @@ class Report < ActiveRecord::Base
   end
 
   def config_retrieval
-    t = validate_meteric("time", :config_retrieval)
-    t.round_with_precision(2) if t
+    metrics[:time][:config_retrieval].round_with_precision(2)
   end
 
   def runtime
-    t = validate_meteric("time", :total)
-    t.round_with_precision(2) if t
+    (metrics[:time][:total] || metrics[:time].values.sum).round_with_precision(2)
   end
 
   #imports a YAML report into database
@@ -83,6 +93,10 @@ class Report < ActiveRecord::Base
 
       # parse report metrics
       raise "Invalid report: can't find metrics information for #{report.host} at #{report.id}" if report.metrics.nil?
+
+      # Is this a pre 2.6.x report format?
+      @pre26 = !report.instance_variables.include?("@resource_statuses")
+
       # convert report status to bit field
       st = calc_status(metrics_to_hash(report))
 
@@ -101,8 +115,10 @@ class Report < ActiveRecord::Base
       host.save_with_validation(false)
 
       # and save our report
-      self.create! :host => host, :reported_at => report.time.utc, :log => report, :status => st
-
+      r = self.create!(:host => host, :reported_at => report.time.utc, :status => st, :metrics => self.m2h(report.metrics))
+      # Store all Puppet message logs
+      r.import_log_messages report
+      return r
     rescue Exception => e
       logger.warn "Failed to process report for #{report.host} due to:#{e}"
       false
@@ -142,8 +158,8 @@ class Report < ActiveRecord::Base
     status = conditions[:status]
     cond = "created_at < \'#{(Time.now.utc - timerange).to_formatted_s(:db)}\'"
     cond += " and status = #{status}" unless status.nil?
-    # delete the reports
-    count = Report.delete_all(cond)
+    # delete the reports, must use destroy_all vs. delete_all because of assoicated logs and METRIC
+    count = Report.destroy_all(cond)
     logger.info Time.now.to_s + ": Expired #{count} Reports"
     return count
   end
@@ -160,24 +176,54 @@ class Report < ActiveRecord::Base
     counter
   end
 
-  protected
+  def import_log_messages report
+    report.logs.each do |r|
+      message = Message.find_or_create_by_value r.message
+      source  = Source.find_or_create_by_value r.source
+      log = Log.create :message_id => message.id, :source_id => source.id, :report_id => self.id, :level => r.level
+      log.errors.empty?
+    end
+  end
+
+  private
 
   # Converts metrics form Puppet report into a hash
   # this hash is required by the calc_status method
   def self.metrics_to_hash(report)
-    resources = report.metrics["resources"]
     report_status = {}
+    metrics = report.metrics.with_indifferent_access
 
     # find our metric values
-    METRIC.each { |m| report_status[m] = resources[m.to_sym] }
+    METRIC.each do |m|
+      if @pre26
+        report_status[m] = metrics["resources"][m]
+      else
+        h=translate_metrics_to26(m)
+        report_status[m] = metrics[h[:type]][h[:name]]
+      end
+      report_status[m] ||= 0
+    end
+
     # special fix for false warning about skips
     # sometimes there are skip values, but there are no error messages, we ignore them.
     if report_status["skipped"] > 0 and ((report_status.values.sum) - report_status["skipped"] == report.logs.size)
       report_status["skipped"] = 0
-    end
+    end unless report_status["skipped"].nil?
     return report_status
   end
 
+
+  # return all metrics as a hash
+  def self.m2h metrics
+    h = {}
+    metrics.each do |title, mtype|
+      h[mtype.name] ||= {}
+      mtype.values.each{|m| h[mtype.name].merge!({m[0] => m[2]})}
+    end
+    return h
+  end
+
+
   # converts a hash into a bit field
   # expects a metrics_to_hash kind of hash
   def self.calc_status (hash = {})
@@ -196,4 +242,17 @@ class Report < ActiveRecord::Base
     nil
   end
 
+
+  # The metrics layout has changed in Puppet 2.6.x release,
+  # this method attempts to align the bit value metrics and the new name scheme in 2.6.x
+  # returns a hash of { :type => "metric type", :name => "metric_name"}
+  def self.translate_metrics_to26 metric
+   case metric
+    when "applied"
+      { :type => "changes", :name => :total}
+    else
+      { :type => "resources", :name => metric.to_sym}
+    end
+  end
+
 end
diff --git a/app/models/source.rb b/app/models/source.rb
new file mode 100644
index 0000000..323add4
--- /dev/null
+++ b/app/models/source.rb
@@ -0,0 +1,9 @@
+class Source < ActiveRecord::Base
+  has_many :reports, :through => :logs
+  has_many :logs
+  validates_presence_of :value
+
+  def to_s
+    value
+  end
+end
diff --git a/app/views/host_mailer/error_state.text.html.erb b/app/views/host_mailer/error_state.text.html.erb
index b6c516a..b1a0743 100644
--- a/app/views/host_mailer/error_state.text.html.erb
+++ b/app/views/host_mailer/error_state.text.html.erb
@@ -5,6 +5,6 @@
     <title>Puppet error from <%= @host.to_label %></title>
   </head>
   <body style="background-color: #ffffff;">
-    <%= render :partial => 'reports/output', :locals => {:logs => @report.log.logs } %>
+    <%= render :partial => 'reports/output', :locals => {:logs => @report.logs } %>
   </body>
 </html>
diff --git a/app/views/reports/show.rhtml b/app/views/reports/show.rhtml
index 3b162cd..ae21806 100644
--- a/app/views/reports/show.rhtml
+++ b/app/views/reports/show.rhtml
@@ -1,4 +1,4 @@
-<% title @report.host.name %>
+<% title @report.host.name -%>
 Reported at <%= @report.reported_at.getlocal %>, which is <b><%= time_ago_in_words(@report.reported_at) %> ago</b>
 <% if @offset > 100 -%>
   <div class="flash error">
@@ -9,8 +9,8 @@ Reported at <%= @report.reported_at.getlocal %>, which is <b><%= time_ago_in_wor
   </div>
 <% end -%>
 
-<% if @report.log.logs.size > 0 -%>
-  <%= render :partial => 'output', :locals => { :logs => @report.log.logs} %>
+<% if @report.logs.size > 0 -%>
+  <%= render :partial => 'output', :locals => { :logs => @report.logs} %>
 <% end -%>
 
 <div class="flash">
@@ -19,8 +19,8 @@ Reported at <%= @report.reported_at.getlocal %>, which is <b><%= time_ago_in_wor
       <td> <b>Metrics</b></td>
       <td>
         <table style="width:100%;">
-          <% @report.log.metrics["time"].values.each do |name, label, value|-%>
-            <% if label == 'Total' then -%>
+          <% @report.metrics["time"].each do |label, value|-%>
+            <% if label.to_s =~ /total/i then -%>
               <%   @totaltime = value -%>
               <%   next -%>
             <% end -%>
@@ -29,7 +29,7 @@ Reported at <%= @report.reported_at.getlocal %>, which is <b><%= time_ago_in_wor
               <td> <%= value.round_with_precision(4) rescue "N/A"%> </td>
             </tr>
           <% end %>
-          <tr><td class="last_row">Total</td><td class="last_row"><%= h @totaltime.round_with_precision(4) rescue "N/A"%></td></tr>
+          <tr><td class="last_row">Total</td><td class="last_row"><%= h (@totaltime || @report.runtime).round_with_precision(4) rescue "N/A"%></td></tr>
         </table>
       </td>
     </tr>
diff --git a/db/migrate/20101018120548_create_messages.rb b/db/migrate/20101018120548_create_messages.rb
new file mode 100644
index 0000000..8f282c5
--- /dev/null
+++ b/db/migrate/20101018120548_create_messages.rb
@@ -0,0 +1,13 @@
+class CreateMessages < ActiveRecord::Migration
+  def self.up
+    create_table :messages do |t|
+      t.text :value
+    end
+    add_index :messages, :value
+  end
+
+  def self.down
+    remove_index :messages, :value
+    drop_table :messages
+  end
+end
diff --git a/db/migrate/20101018120603_create_sources.rb b/db/migrate/20101018120603_create_sources.rb
new file mode 100644
index 0000000..adfb074
--- /dev/null
+++ b/db/migrate/20101018120603_create_sources.rb
@@ -0,0 +1,13 @@
+class CreateSources < ActiveRecord::Migration
+  def self.up
+    create_table :sources do |t|
+      t.text :value
+    end
+    add_index :sources, :value
+  end
+
+  def self.down
+    remove_index :sources, :value
+    drop_table :sources
+  end
+end
diff --git a/db/migrate/20101018120621_create_logs.rb b/db/migrate/20101018120621_create_logs.rb
new file mode 100644
index 0000000..fc55805
--- /dev/null
+++ b/db/migrate/20101018120621_create_logs.rb
@@ -0,0 +1,22 @@
+class CreateLogs < ActiveRecord::Migration
+  def self.up
+    create_table :logs do |t|
+      t.integer :source_id
+      t.integer :message_id
+      t.integer :report_id
+      t.integer :level_id
+
+      t.timestamps
+    end
+    add_index :logs, :report_id
+    add_index :logs, :message_id
+    add_index :logs, :level_id
+  end
+
+  def self.down
+    remove_index :logs, :level_id
+    remove_index :logs, :report_id
+    remove_index :logs, :message_id
+    drop_table :logs
+  end
+end
diff --git a/db/migrate/20101019122857_add_metrics_to_report.rb b/db/migrate/20101019122857_add_metrics_to_report.rb
new file mode 100644
index 0000000..8724aa0
--- /dev/null
+++ b/db/migrate/20101019122857_add_metrics_to_report.rb
@@ -0,0 +1,9 @@
+class AddMetricsToReport < ActiveRecord::Migration
+  def self.up
+    add_column :reports, :metrics, :text
+  end
+
+  def self.down
+    remove_column :reports, :metrics
+  end
+end
diff --git a/db/migrate/20101019183859_convert_reports.rb b/db/migrate/20101019183859_convert_reports.rb
new file mode 100644
index 0000000..b24dcc9
--- /dev/null
+++ b/db/migrate/20101019183859_convert_reports.rb
@@ -0,0 +1,33 @@
+class ConvertReports < ActiveRecord::Migration
+  def self.up
+    say "About to convert all of the reports log field into a more DB optimized way... this is going to take a long while....."
+
+    Report.all.each do |report|
+      case report.log.class.to_s
+      when "Puppet::Transaction::Report"
+        log = report.log
+      when "String"
+        log = YAML.load(report.log)
+      else
+        raise "Unable to process report #{report.log.class}"
+      end
+
+      # Is this a pre 2.6.x report format?
+      @pre26 = !report.instance_variables.include?("@resource_statuses")
+
+      # Recalcuate the status field if this report is from a 2.6.x puppet client
+      report.status = Report.calc_status(Report.metrics_to_hash(log)) unless @pre26
+      report.metrics = Report.m2h(log.metrics).with_indifferent_access
+
+      report.import_log_messages(log)
+      report.log = ""
+
+      report.save
+    end
+    remove_column :reports, :log
+
+  end
+
+  def self.down
+  end
+end
diff --git a/test/fixtures/logs.yml b/test/fixtures/logs.yml
new file mode 100644
index 0000000..13c76c2
--- /dev/null
+++ b/test/fixtures/logs.yml
@@ -0,0 +1,11 @@
+# Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html
+
+one:
+  source_id: 1
+  message_id: 1
+  report_id: 1
+
+two:
+  source_id: 1
+  message_id: 1
+  report_id: 1
diff --git a/test/fixtures/messages.yml b/test/fixtures/messages.yml
new file mode 100644
index 0000000..947c899
--- /dev/null
+++ b/test/fixtures/messages.yml
@@ -0,0 +1,7 @@
+# Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html
+
+one:
+  value: MyText
+
+two:
+  value: MyText
diff --git a/test/fixtures/sources.yml b/test/fixtures/sources.yml
new file mode 100644
index 0000000..947c899
--- /dev/null
+++ b/test/fixtures/sources.yml
@@ -0,0 +1,7 @@
+# Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html
+
+one:
+  value: MyText
+
+two:
+  value: MyText
diff --git a/test/unit/log_test.rb b/test/unit/log_test.rb
new file mode 100644
index 0000000..4da97dd
--- /dev/null
+++ b/test/unit/log_test.rb
@@ -0,0 +1,8 @@
+require 'test_helper'
+
+class LogTest < ActiveSupport::TestCase
+  # Replace this with your real tests.
+  test "the truth" do
+    assert true
+  end
+end
diff --git a/test/unit/message_test.rb b/test/unit/message_test.rb
new file mode 100644
index 0000000..e4de21d
--- /dev/null
+++ b/test/unit/message_test.rb
@@ -0,0 +1,8 @@
+require 'test_helper'
+
+class MessageTest < ActiveSupport::TestCase
+  # Replace this with your real tests.
+  test "the truth" do
+    assert true
+  end
+end
diff --git a/test/unit/source_test.rb b/test/unit/source_test.rb
new file mode 100644
index 0000000..69ef0fb
--- /dev/null
+++ b/test/unit/source_test.rb
@@ -0,0 +1,8 @@
+require 'test_helper'
+
+class SourceTest < ActiveSupport::TestCase
+  # Replace this with your real tests.
+  test "the truth" do
+    assert true
+  end
+end
-- 
1.7.2.3

