From caea11bf6a65311ac45bbdfb43bd04ba0f04bc15 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 27 Oct 2022 18:43:00 +0200 Subject: [PATCH 1/7] Write out log entry Rather than defining an entry and then appending it, this writes it all out so it's clear what the structure is. --- files/report.rb | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/files/report.rb b/files/report.rb index 960a6a4..ae0ca2d 100644 --- a/files/report.rb +++ b/files/report.rb @@ -144,11 +144,17 @@ def logs_to_array logs next if log.message =~ /^Finished catalog run in \d+.\d+ seconds$/ # Match Foreman's slightly odd API format... - l = { 'log' => { 'sources' => {}, 'messages' => {} } } - l['log']['level'] = log.level.to_s - l['log']['messages']['message'] = log.message - l['log']['sources']['source'] = log.source - h << l + h << { + 'log' => { + 'level' => log.level.to_s, + 'sources' => { + 'source' => log.source, + }, + 'messages' => { + 'message' => log.message, + }, + }, + } end return h end From 45c4fab7fafa7f343e769e0ad83500f1647dd12a Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 27 Oct 2022 18:44:41 +0200 Subject: [PATCH 2/7] Use .fetch where applicable --- files/report.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/files/report.rb b/files/report.rb index ae0ca2d..e339441 100644 --- a/files/report.rb +++ b/files/report.rb @@ -37,7 +37,7 @@ def process # Default retry limit is 1. - retry_limit = SETTINGS[:report_retry_limit] ? SETTINGS[:report_retry_limit] : 1 + retry_limit = SETTINGS.fetch(:report_retry_limit, 1) tries = 0 begin # check for report metrics From 63a8350949c8de4450f5b6184d3fe4892bc682c9 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 27 Oct 2022 18:46:38 +0200 Subject: [PATCH 3/7] Drop Report format 0 and 1 Even Puppet 2.6 uses format version 2. --- files/report.rb | 40 ++++++---------------------------------- 1 file changed, 6 insertions(+), 34 deletions(-) diff --git a/files/report.rb b/files/report.rb index e339441..b1bdf22 100644 --- a/files/report.rb +++ b/files/report.rb @@ -79,7 +79,6 @@ def process def generate_report report = {} - set_report_format report['host'] = self.host # Time.to_s behaves differently in 1.8 / 1.9 so we explicity set the 1.9 format report['reported_at'] = self.time.utc.strftime("%Y-%m-%d %H:%M:%S UTC") @@ -100,13 +99,9 @@ def metrics_to_hash(report) # find our metric values METRIC.each do |m| - if @format == 0 - report_status[m] = metrics["resources"][m.to_sym] unless metrics["resources"].nil? - else - h=translate_metrics_to26(m) - mv = metrics[h[:type]] - report_status[m] = mv[h[:name].to_sym] + mv[h[:name].to_s] rescue nil - end + h=translate_metrics_to26(m) + mv = metrics[h[:type]] + report_status[m] = mv[h[:name].to_sym] + mv[h[:name].to_s] rescue nil report_status[m] ||= 0 end @@ -116,7 +111,7 @@ def metrics_to_hash(report) report_status["skipped"] = 0 end # fix for reports that contain no metrics (i.e. failed catalog) - if @format > 1 and report.respond_to?(:status) and report.status == "failed" + if report.respond_to?(:status) and report.status == "failed" report_status["failed"] += 1 end # fix for Puppet non-resource errors (i.e. failed catalog fetches before falling back to cache) @@ -165,19 +160,9 @@ def logs_to_array logs def translate_metrics_to26 metric case metric when "applied" - case @format - when 0..1 - { :type => "total", :name => :changes} - else - { :type => "changes", :name => "total"} - end + { :type => "changes", :name => "total"} when "failed_restarts" - case @format - when 0..1 - { :type => "resources", :name => metric} - else - { :type => "resources", :name => "failed_to_restart"} - end + { :type => "resources", :name => "failed_to_restart"} when "pending" { :type => "events", :name => "noop" } else @@ -185,19 +170,6 @@ def translate_metrics_to26 metric end end - def set_report_format - @format ||= case - when self.instance_variables.detect {|v| v.to_s == "@environment"} - @format = 3 - when self.instance_variables.detect {|v| v.to_s == "@report_format"} - @format = 2 - when self.instance_variables.detect {|v| v.to_s == "@resource_statuses"} - @format = 1 - else - @format = 0 - end - end - def foreman_url SETTINGS[:url] || raise(Puppet::Error, "Must provide URL in #{$settings_file}") end From 36cf3b0f6bb8ecc3ec9713789a11e539b4f40107 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 27 Oct 2022 19:05:23 +0200 Subject: [PATCH 4/7] Drop translate_metrics_to26 method --- files/report.rb | 34 +++++++++++++++------------------- 1 file changed, 15 insertions(+), 19 deletions(-) diff --git a/files/report.rb b/files/report.rb index b1bdf22..25a22fd 100644 --- a/files/report.rb +++ b/files/report.rb @@ -99,9 +99,21 @@ def metrics_to_hash(report) # find our metric values METRIC.each do |m| - h=translate_metrics_to26(m) - mv = metrics[h[:type]] - report_status[m] = mv[h[:name].to_sym] + mv[h[:name].to_s] rescue nil + case m + when "applied" + mv = metrics["changes"] + name = "total" + when "failed_restarts" + mv = metrics["resources"] + name = "failed_to_restart" + when "pending" + mv = metrics["events"] + name = "noop" + else + mv = metrics["resources"] + name = m + end + report_status[m] = mv[name.to_sym] + mv[name.to_s] rescue nil report_status[m] ||= 0 end @@ -154,22 +166,6 @@ def logs_to_array logs return h 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 translate_metrics_to26 metric - case metric - when "applied" - { :type => "changes", :name => "total"} - when "failed_restarts" - { :type => "resources", :name => "failed_to_restart"} - when "pending" - { :type => "events", :name => "noop" } - else - { :type => "resources", :name => metric} - end - end - def foreman_url SETTINGS[:url] || raise(Puppet::Error, "Must provide URL in #{$settings_file}") end From ef688ae3db2a5d909b059e123566481f602bd859 Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 27 Oct 2022 19:05:59 +0200 Subject: [PATCH 5/7] Simplify metric construction This takes advantage of the fact that each title is the same as the metric's name. --- files/report.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/files/report.rb b/files/report.rb index 25a22fd..743b3d5 100644 --- a/files/report.rb +++ b/files/report.rb @@ -133,12 +133,9 @@ def metrics_to_hash(report) end def m2h metrics - h = {} - metrics.each do |title, mtype| - h[mtype.name] ||= {} - mtype.values.each{|m| h[mtype.name].merge!({m[0].to_s => m[2]})} + metrics.to_h do |title, mtype| + [title, mtype.values.to_h { |name, _label, value| [name, value] }] end - return h end def logs_to_array logs From 07f3b0cdf66d3d95c489546474184761b37c528a Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 27 Oct 2022 19:15:22 +0200 Subject: [PATCH 6/7] Simplify generate_report definition --- files/report.rb | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/files/report.rb b/files/report.rb index 743b3d5..c6ede66 100644 --- a/files/report.rb +++ b/files/report.rb @@ -78,15 +78,14 @@ def process end def generate_report - report = {} - report['host'] = self.host - # Time.to_s behaves differently in 1.8 / 1.9 so we explicity set the 1.9 format - report['reported_at'] = self.time.utc.strftime("%Y-%m-%d %H:%M:%S UTC") - report['status'] = metrics_to_hash(self) - report['metrics'] = m2h(self.metrics) - report['logs'] = logs_to_array(self.logs) - - report + { + 'host' => self.host, + # Time.to_s behaves differently in 1.8 / 1.9 so we explicity set the 1.9 format + 'reported_at' => self.time.utc.strftime("%Y-%m-%d %H:%M:%S UTC"), + 'status' => metrics_to_hash(self), + 'metrics' => m2h(self.metrics), + 'logs' => logs_to_array(self.logs), + } end private From 5ff7a966a7ce9f18ad3d466f27c49631e5af300f Mon Sep 17 00:00:00 2001 From: Ewoud Kohl van Wijngaarden Date: Thu, 27 Oct 2022 20:21:11 +0200 Subject: [PATCH 7/7] Use a more effective method of counting This saves an intermediate array. --- files/report.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/files/report.rb b/files/report.rb index c6ede66..c63c46a 100644 --- a/files/report.rb +++ b/files/report.rb @@ -126,7 +126,7 @@ def metrics_to_hash(report) report_status["failed"] += 1 end # fix for Puppet non-resource errors (i.e. failed catalog fetches before falling back to cache) - report_status["failed"] += report.logs.find_all {|l| l.source =~ /Puppet$/ && l.level.to_s == 'err' }.count + report_status["failed"] += report.logs.count {|l| l.source =~ /Puppet$/ && l.level.to_s == 'err' } return report_status end