Fixing middleware servers alert handling #16048
Conversation
* miq_alert: MiddlewareServers model has STI enabled. It's not enough to test against the class name to detect if target is a mw-server. Using kind_of?. * middleware_server: The format of the alert ids changed. Testing against the new id format in addition to the old one to check if alert should be handled.
|
@miq-bot add_label bug, alerts, providers/hawkular |
|
Checked commit israel-hdez@68cb336 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0 |
|
@lucasponce could you please review this? |
I am not familiar with STI, I understand that this fixes the previous logic to identify the class.
That sounds good. LGTM |
|
LGTM |
| @@ -33,7 +33,8 @@ def evaluate_alert(alert_id, event) | |||
| s_start = event.full_data.index("id=\"") + 4 | |||
| s_end = event.full_data.index("\"", s_start + 4) - 1 | |||
| event_id = event.full_data[s_start..s_end] | |||
| if event_id.start_with?("MiQ-#{alert_id}") && event.middleware_server_id == id | |||
| if event.middleware_server_id == id && ( | |||
| event_id.start_with?("MiQ-#{alert_id}") || event_id.start_with?(ext_management_system.miq_id_prefix)) | |||
There was a problem hiding this comment.
how about moving that logic to your subclass?
ext_management_system.miq_id_prefix is a method that is only avail in Hawkular::MiddlewareManager
There was a problem hiding this comment.
Yes, indeed evaluate_alert should be made abstract. All its code is specific to Hawkular. May I move the code in a follow-up PR?
BTW, the asignee didn't change.
There was a problem hiding this comment.
I'm good with a followup PR for this, I'm not a fan of checking event_id.start_with? maybe you can try checking the source?
Fixing middleware servers alert handling
This PR is part of a collection of PRs solving #15756.