From 501af5befc934921140650498e3dceb227cd8213 Mon Sep 17 00:00:00 2001 From: Ohad Levy Date: Wed, 20 Oct 2010 18:09:02 +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 | 18 ++++ db/migrate/20101018120603_create_sources.rb | 18 ++++ db/migrate/20101018120621_create_logs.rb | 22 +++++ db/migrate/20101019122857_add_metrics_to_report.rb | 9 ++ db/migrate/20101019183859_convert_reports.rb | 36 ++++++++ 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, 272 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 @@ Puppet error from <%= @host.to_label %> - <%= render :partial => 'reports/output', :locals => {:logs => @report.log.logs } %> + <%= render :partial => 'reports/output', :locals => {:logs => @report.logs } %> 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 <%= time_ago_in_words(@report.reported_at) %> ago <% if @offset > 100 -%>
@@ -9,8 +9,8 @@ Reported at <%= @report.reported_at.getlocal %>, which is <%= time_ago_in_wor
<% 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 -%>
@@ -19,8 +19,8 @@ Reported at <%= @report.reported_at.getlocal %>, which is <%= time_ago_in_wor Metrics - <% @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 <%= time_ago_in_wor <% end %> - +
<%= value.round_with_precision(4) rescue "N/A"%>
Total<%= h @totaltime.round_with_precision(4) rescue "N/A"%>
Total<%= h (@totaltime || @report.runtime).round_with_precision(4) rescue "N/A"%>
diff --git a/db/migrate/20101018120548_create_messages.rb b/db/migrate/20101018120548_create_messages.rb new file mode 100644 index 0000000..29c531b --- /dev/null +++ b/db/migrate/20101018120548_create_messages.rb @@ -0,0 +1,18 @@ +class CreateMessages < ActiveRecord::Migration + def self.up + create_table :messages do |t| + t.text :value + end + if ActiveRecord::Base.connection.instance_values["config"][:adapter] == "mysql" + execute "ALTER TABLE messages ENGINE = MYISAM" + execute "ALTER TABLE messages ADD FULLTEXT (value)" + else + add_index :messages, :value + end + 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..b853556 --- /dev/null +++ b/db/migrate/20101018120603_create_sources.rb @@ -0,0 +1,18 @@ +class CreateSources < ActiveRecord::Migration + def self.up + create_table :sources do |t| + t.text :value + end + if ActiveRecord::Base.connection.instance_values["config"][:adapter] == "mysql" + execute "ALTER TABLE sources ENGINE = MYISAM" + execute "ALTER TABLE sources ADD FULLTEXT (value)" + else + add_index :sources, :value + end + 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..e2d8e00 --- /dev/null +++ b/db/migrate/20101019183859_convert_reports.rb @@ -0,0 +1,36 @@ +class ConvertReports < ActiveRecord::Migration + def self.up + say "About to convert all of the #{Report.count} reports log field into a more DB optimized way... this might take a 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 + # this report might have been processed already, skipping + next + 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 = "" # not really needed, but this way the db can reuse some space instead of claim new one. + + report.save + end + remove_column :reports, :log + + end + + def self.down + add_column :reports, :log, :text + say "cant recreate the data, import it again" + 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