Support alerts on container nodes #13812

moolitayer · 2017-02-08T15:19:11Z

Add the ability to define miq alerts on container nodes.
The usual flow miq alert statuses are created: an ems event is collected and then evaluated for an alert[1]
Currently only ems events from Hawkular are evaluated for alerts on container nodes.
Since we are offloading the logic of triggering to Hawkular every ems event already means a miq_alert_status needs to be created (if a miq alert has been defined)

Currently we only support defining the miq alert on all the nodes in the enterprise since that is what we would use (actual target object is found based on metadata on the incoming event)

Extracted from #12773

[1]

manageiq/lib/ems_event_helper.rb

Line 33 in ff425fd

    
           @event.policy("src_vm", @event.event_type) if MiqAlert.event_alertable?(@event.event_type)

moolitayer · 2017-02-08T15:19:40Z

@simon3z @durandom please review

moolitayer · 2017-02-08T15:19:48Z

@cben please review

moolitayer · 2017-02-08T15:20:14Z

@miq-bot add_label enhancement, providers/containers

moolitayer · 2017-02-08T20:34:17Z

@simon3z please review

moolitayer · 2017-02-09T10:29:08Z

@simon3z @cben please review

simon3z · 2017-02-09T10:55:27Z

@cben can you review this first?

moolitayer · 2017-02-09T12:26:44Z

app/models/ems_event.rb

@@ -196,6 +196,7 @@ def get_target(target_type)
    target_type = "src_vm_or_template"  if target_type == "src_vm"
    target_type = "dest_vm_or_template" if target_type == "dest_vm"
    target_type = "middleware_server"   if event.event_type == "hawkular_alert"
+    target_type = "container_node"      if event.event_type == "datawarehouse_event"


these need to be renamed to datawarehouse_alert as @lucasponce pointed here

Sufficient now but you need a plan for deriving target_type when you'll have more entities beside container_node...

Yes though of that but I'm not sure yet

moolitayer · 2017-02-12T13:02:43Z

@cben please review

cben · 2017-02-12T13:13:57Z

app/models/container_node.rb

+    else
+      User.super_admin.tap { |u| u.current_group = Tenant.root_tenant.default_miq_group }
+    end
+  end


I wish we could stop copy-pasting this black magic, but that would take someone who understands it to suggest an appropriate place...

Turns out we can 😄 since TenantIdentityMixin is already included here 👍

cben · 2017-02-12T13:17:56Z

app/models/container_node.rb

+  def evaluate_alert(_alert_id, _event)
+    # currently only EmsEvents from hawkular are tested for node alerts,
+    # and these should automaticaly be translated to alerts.
+    true


Fine for now, because we don't currently support regular MiqExpression-evaluating alerts on container entities.
But would it be hard to check something on the event to confirm it's a hawkular alerting event, just in case?

I plan to add something that will propegate to this point in #12773 then I will be able to add a check

cben · 2017-02-12T13:19:03Z

app/models/ems_event.rb

@@ -196,6 +196,7 @@ def get_target(target_type)
    target_type = "src_vm_or_template"  if target_type == "src_vm"
    target_type = "dest_vm_or_template" if target_type == "dest_vm"
    target_type = "middleware_server"   if event.event_type == "hawkular_alert"
+    target_type = "container_node"      if event.event_type == "datawarehouse_event"


Sufficient now but you need a plan for deriving target_type when you'll have more entities beside container_node...

cben · 2017-02-12T13:31:52Z

app/models/miq_alert.rb

@@ -434,7 +435,9 @@ def self.automate_expressions
        :options => [
          {:name => :mw_operator, :description => _("Operator"), :values => [">", ">=", "<", "<=", "="]},
          {:name => :value_mw_garbage_collector, :description => _("Duration Per Minute (ms)"), :numeric => true}
-        ]}
+        ]},
+      {:name => "hwk_generic", :description => _("All hawkular alerts"), :db => ["ContainerNode"], :responds_to_events => "datawarehouse_event",


"All hawkular alerts" may need more context given the existing middleware conditions above?

Maybe "All Datawarehouse Events" ?

moolitayer · 2017-02-12T19:41:11Z

Thanks @cben 🎉

moolitayer · 2017-02-13T09:00:42Z

@cben @simon3z please review

simon3z · 2017-02-13T09:08:19Z

@cben can you review/ack this?

cben · 2017-02-13T09:56:57Z

LGTM

simon3z · 2017-02-13T10:19:19Z

@moolitayer @cben anything to test?

durandom

I really like the size of the PR 👍

durandom · 2017-02-13T10:46:59Z

app/models/ems_event.rb

@@ -196,6 +196,7 @@ def get_target(target_type)
    target_type = "src_vm_or_template"  if target_type == "src_vm"
    target_type = "dest_vm_or_template" if target_type == "dest_vm"
    target_type = "middleware_server"   if event.event_type == "hawkular_alert"
+    target_type = "container_node"      if event.event_type == "datawarehouse_alert"


I already dislike the one line above, where it does implicitly connects hawkular to middleware_server
Is it possible to switch on something else than datawarehose_alert ? Maybe not switch on event.event_type but on some other meta data?

This as well as the logic in container_node.evaluate_alert would be much, much easier to improve in #12773 when this is merged. Can we try to do it at that iteration please?

ok, I guess you are the only provider that sends datawarehouse_alert anyway

durandom · 2017-02-13T10:47:53Z

app/models/miq_alert.rb

@@ -434,7 +435,9 @@ def self.automate_expressions
        :options => [
          {:name => :mw_operator, :description => _("Operator"), :values => [">", ">=", "<", "<=", "="]},
          {:name => :value_mw_garbage_collector, :description => _("Duration Per Minute (ms)"), :numeric => true}
-        ]}
+        ]},
+      {:name => "hwk_generic", :description => _("All Datawarehouse alerts"), :db => ["ContainerNode"], :responds_to_events => "datawarehouse_alert",


could this be dwh_generic ?

miq-bot · 2017-02-13T17:57:34Z

Checked commit moolitayer@92cc359 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 0 offenses detected
Everything looks good. ⭐

moolitayer · 2017-02-13T18:22:19Z

@moolitayer @cben anything to test?

I'm adding tests for the entire flow in the event collection patch
Some of this code is tested here

moolitayer · 2017-02-13T18:23:25Z

Integration failure seems unrelated

    Failure/Error: cv1 = FactoryGirl.create(:cloud_volume_amazon, :attachments => [disk])
     
     NameError:
       uninitialized constant ManageIQ::Providers::Amazon::CloudManager::CloudVolume
     # ./spec/models/cloud_volume_spec.rb:4:in `block (2 levels) in <top (required)>'

...
    rspec ./spec/models/cloud_volume_spec.rb:2 # CloudVolume .available

durandom

lgtm

once master #13886 is merged, you can re-kick the CI?

@miq-bot assign @blomquisg

durandom · 2017-02-13T19:05:08Z

app/models/ems_event.rb

@@ -196,6 +196,7 @@ def get_target(target_type)
    target_type = "src_vm_or_template"  if target_type == "src_vm"
    target_type = "dest_vm_or_template" if target_type == "dest_vm"
    target_type = "middleware_server"   if event.event_type == "hawkular_alert"
+    target_type = "container_node"      if event.event_type == "datawarehouse_alert"


ok, I guess you are the only provider that sends datawarehouse_alert anyway

durandom · 2017-02-14T07:45:23Z

@miq-bot assign @blomquisg
@blomquisg please merge

miq-bot added enhancement providers/containers labels Feb 8, 2017

moolitayer mentioned this pull request Feb 8, 2017

Event collection for the datawarehouse provider #12773

Merged

chessbyte assigned simon3z Feb 8, 2017

moolitayer commented Feb 9, 2017

View reviewed changes

cben reviewed Feb 12, 2017

View reviewed changes

moolitayer force-pushed the node_alerts branch from 4737214 to 0307b70 Compare February 12, 2017 19:29

durandom suggested changes Feb 13, 2017

View reviewed changes

Support alerts on container nodes

92cc359

moolitayer force-pushed the node_alerts branch from 0307b70 to 92cc359 Compare February 13, 2017 17:53

durandom approved these changes Feb 13, 2017

View reviewed changes

miq-bot assigned durandom and unassigned simon3z Feb 13, 2017

moolitayer closed this Feb 13, 2017

moolitayer reopened this Feb 13, 2017

miq-bot assigned blomquisg and unassigned durandom Feb 14, 2017

blomquisg merged commit 7506b85 into ManageIQ:master Feb 14, 2017

blomquisg added this to the Sprint 55 Ending Feb 27, 2017 milestone Feb 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support alerts on container nodes #13812

Support alerts on container nodes #13812

moolitayer commented Feb 8, 2017 •

edited by miq-bot

Loading

moolitayer commented Feb 8, 2017

moolitayer commented Feb 8, 2017

moolitayer commented Feb 8, 2017

moolitayer commented Feb 8, 2017

moolitayer commented Feb 9, 2017

simon3z commented Feb 9, 2017

moolitayer Feb 9, 2017

cben Feb 12, 2017

moolitayer Feb 12, 2017

moolitayer commented Feb 12, 2017

cben Feb 12, 2017

moolitayer Feb 12, 2017

cben Feb 12, 2017

moolitayer Feb 12, 2017

cben Feb 12, 2017

cben Feb 12, 2017

moolitayer Feb 12, 2017

moolitayer commented Feb 12, 2017 •

edited

Loading

moolitayer commented Feb 13, 2017

simon3z commented Feb 13, 2017

cben commented Feb 13, 2017

simon3z commented Feb 13, 2017

durandom left a comment

durandom Feb 13, 2017

moolitayer Feb 13, 2017

durandom Feb 13, 2017

durandom Feb 13, 2017 •

edited

Loading

miq-bot commented Feb 13, 2017

moolitayer commented Feb 13, 2017

moolitayer commented Feb 13, 2017 •

edited

Loading

durandom left a comment

durandom Feb 13, 2017

durandom commented Feb 14, 2017

Support alerts on container nodes #13812

Support alerts on container nodes #13812

Conversation

moolitayer commented Feb 8, 2017 • edited by miq-bot Loading

moolitayer commented Feb 8, 2017

moolitayer commented Feb 8, 2017

moolitayer commented Feb 8, 2017

moolitayer commented Feb 8, 2017

moolitayer commented Feb 9, 2017

simon3z commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moolitayer commented Feb 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

moolitayer commented Feb 12, 2017 • edited Loading

moolitayer commented Feb 13, 2017

simon3z commented Feb 13, 2017

cben commented Feb 13, 2017

simon3z commented Feb 13, 2017

durandom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durandom Feb 13, 2017 • edited Loading

Choose a reason for hiding this comment

miq-bot commented Feb 13, 2017

moolitayer commented Feb 13, 2017

moolitayer commented Feb 13, 2017 • edited Loading

durandom left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

durandom commented Feb 14, 2017

moolitayer commented Feb 8, 2017 •

edited by miq-bot

Loading

moolitayer commented Feb 12, 2017 •

edited

Loading

durandom Feb 13, 2017 •

edited

Loading

moolitayer commented Feb 13, 2017 •

edited

Loading