From 3e2e70fccc50be16a2b0c0b880ed0a4a93a84526 Mon Sep 17 00:00:00 2001 From: Mooli Tayer Date: Wed, 8 Mar 2017 16:49:08 +0200 Subject: [PATCH 1/2] Fix event description extraction --- app/models/miq_alert.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/models/miq_alert.rb b/app/models/miq_alert.rb index e3f8c9a1d58..128af582aa2 100644 --- a/app/models/miq_alert.rb +++ b/app/models/miq_alert.rb @@ -193,8 +193,7 @@ def evaluate(target, inputs = {}) # If we are alerting, invoke the alert actions, then add a status so we can limit how often to alert # Otherwise, destroy this alert's statuses for our target invoke_actions(target, inputs) if result - add_status_post_evaluate(target, result, inputs[:description]) - + add_status_post_evaluate(target, result, inputs[:ems_event]) result end From 0fbf598e68058d095750a24902fb8a915221965b Mon Sep 17 00:00:00 2001 From: Mooli Tayer Date: Wed, 8 Mar 2017 16:49:31 +0200 Subject: [PATCH 2/2] Pass metadata from an EmsEvent to an alert --- app/models/ems_event.rb | 8 +++++ app/models/miq_alert.rb | 5 ++- spec/models/miq_alert_spec.rb | 59 +++++++++++++++++++++++++++++++++-- 3 files changed, 69 insertions(+), 3 deletions(-) diff --git a/app/models/ems_event.rb b/app/models/ems_event.rb index 8a674f5a279..5885a1d3512 100644 --- a/app/models/ems_event.rb +++ b/app/models/ems_event.rb @@ -164,6 +164,14 @@ def self.first_chained_event(ems_id, chain_id) EmsEvent.where(:ems_id => ems_id, :chain_id => chain_id).order(:id).first end + def parse_event_metadata + [ + event_type == "datawarehouse_alert" ? message : nil, + full_data.try(:[], :severity), + full_data.try(:[], :url), + ] + end + def first_chained_event @first_chained_event ||= EmsEvent.first_chained_event(ems_id, chain_id) || self end diff --git a/app/models/miq_alert.rb b/app/models/miq_alert.rb index 128af582aa2..efadc77d204 100644 --- a/app/models/miq_alert.rb +++ b/app/models/miq_alert.rb @@ -197,11 +197,14 @@ def evaluate(target, inputs = {}) result end - def add_status_post_evaluate(target, result, status_description) + def add_status_post_evaluate(target, result, event) + status_description, severity, url = event.parse_event_metadata if event.respond_to?(:parse_event_metadata) status = miq_alert_statuses.find_or_initialize_by(:resource => target) status.result = result status.ems_id = target.try(:ems_id) status.description = status_description || description + status.severity = severity unless severity.blank? + status.url = url unless url.blank? status.evaluated_on = Time.now.utc status.save miq_alert_statuses << status diff --git a/spec/models/miq_alert_spec.rb b/spec/models/miq_alert_spec.rb index cc415405c7f..4145ffd92bc 100644 --- a/spec/models/miq_alert_spec.rb +++ b/spec/models/miq_alert_spec.rb @@ -96,32 +96,86 @@ before(:each) do @alert = MiqAlert.find_by(:description => "VM Unregistered") allow(@alert).to receive_messages(:eval_expression => true) - @alert.evaluate([@vm.class.base_class.name, @vm.id]) end it "should have a link from the MiqAlert to the miq alert status" do + @alert.evaluate([@vm.class.base_class.name, @vm.id]) expect(@alert.miq_alert_statuses.where(:resource_type => @vm.class.base_class.name, :resource_id => @vm.id).count).to eq(1) end it "should have a miq alert status for MiqAlert with a result of true" do + @alert.evaluate([@vm.class.base_class.name, @vm.id]) expect(@alert.miq_alert_statuses.find_by(:resource_type => @vm.class.base_class.name, :resource_id => @vm.id).result).to be_truthy end - it "cookie stamps the description on the alert" do + it "does not explode if evaluate.input = {}" do + expect { @alert.evaluate([@vm.class.base_class.name, @vm.id]) }.to_not raise_error + end + + it "miq_alert_status.description = miq_alert.description event if overriden by ems_event.description" do + @alert.evaluate( + [@vm.class.base_class.name, @vm.id], + :ems_event => FactoryGirl.create(:ems_event, :message => "oh no!", :type => 'WhateverEvent') + ) mas = @alert.miq_alert_statuses.where(:resource_type => @vm.class.base_class.name, :resource_id => @vm.id).first expect(mas.description).to eq("VM Unregistered") end + it "miq_alert_status.description = ems_event.message if present and datawarehouse_alert" do + @alert.evaluate( + [@vm.class.base_class.name, @vm.id], + :ems_event => FactoryGirl.create(:ems_event, :message => "oh no!", :event_type => "datawarehouse_alert") + ) + mas = @alert.miq_alert_statuses.where(:resource_type => @vm.class.base_class.name, :resource_id => @vm.id).first + expect(mas.description).to eq("oh no!") + end + + it "miq_alert_status.severity = ems_event.full_data.severity if present" do + @alert.evaluate( + [@vm.class.base_class.name, @vm.id], + :ems_event => FactoryGirl.create(:ems_event, :full_data => {:severity => 'warning'}) + ) + mas = @alert.miq_alert_statuses.where(:resource_type => @vm.class.base_class.name, :resource_id => @vm.id).first + expect(mas.severity).to eq('warning') + end + + it "miq_alert_status.severity = nil if ems_event.full_data.severity not present" do + @alert.evaluate([@vm.class.base_class.name, @vm.id]) + mas = @alert.miq_alert_statuses.where(:resource_type => @vm.class.base_class.name, :resource_id => @vm.id).first + expect(mas.severity).to eq(nil) + end + + it "miq_alert_status.url = ems_event.full_data.url if present" do + @alert.evaluate( + [@vm.class.base_class.name, @vm.id], + :ems_event => FactoryGirl.create( + :ems_event, + :full_data => {:url => 'https://www.youtube.com/watch?v=dQw4w9WgXcQ'} + ) + ) + mas = @alert.miq_alert_statuses.where(:resource_type => @vm.class.base_class.name, :resource_id => @vm.id).first + expect(mas.url).to eq('https://www.youtube.com/watch?v=dQw4w9WgXcQ') + end + + it "miq_alert_status.url = nil if ems_event.full_data.url is not present" do + @alert.evaluate([@vm.class.base_class.name, @vm.id]) + mas = @alert.miq_alert_statuses.where(:resource_type => @vm.class.base_class.name, :resource_id => @vm.id).first + expect(mas.url).to eq(nil) + end + it "should have a link from the Vm to the miq alert status" do + @alert.evaluate([@vm.class.base_class.name, @vm.id]) expect(@vm.miq_alert_statuses.where(:miq_alert_id => @alert.id).count).to eq(1) end it "should have a miq alert status for Vm with a result of true" do + @alert.evaluate([@vm.class.base_class.name, @vm.id]) expect(@vm.miq_alert_statuses.find_by(:miq_alert_id => @alert.id).result).to be_truthy end context "with the alert now evaluated to false" do before(:each) do + @alert.evaluate([@vm.class.base_class.name, @vm.id]) allow(@alert).to receive_messages(:eval_expression => false) @alert.options.store_path(:notifications, :delay_next_evaluation, 0) @alert.evaluate([@vm.class.base_class.name, @vm.id]) @@ -146,6 +200,7 @@ context "with a delay_next_evaluation value of 5 minutes" do before(:each) do + @alert.evaluate([@vm.class.base_class.name, @vm.id]) @alert.options ||= {} @alert.options.store_path(:notifications, :delay_next_evaluation, 5.minutes) @alert.save