Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle exception when a metrics target doesn't have an ext_management_system #14718

Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Apr 10, 2017

If an invalid target finds its way into Metric::Capture.queue_captures the exception thrown will prevent all further targets from being queued. This will catch and log the exception for that target then continue to queue the remaining targets.

Also since Unsupported type ManageIQ::Providers::Kubernetes::ContainerManager::Container (id: 28453) isn't very descriptive of the real issue (the target type is fine, the specific target doesn't have an ems) this will raise an ArgumentError saying "target does not have an ExtManagementSystem" if the target doesn't have an ems_id or an old_ems_id.

https://bugzilla.redhat.com/show_bug.cgi?id=1439888

Prevent a single target.perf_capture_queue exception from preventing the
rest of the targets from being queued.  If an invalid target made its
way into the targets list and perf_capture_queue throws an exception
then currently the remaining list of targets won't be queued.

https://bugzilla.redhat.com/show_bug.cgi?id=1439888
@agrare
Copy link
Member Author

agrare commented Apr 10, 2017

cc @blomquisg @gtanzillo

@agrare
Copy link
Member Author

agrare commented Apr 10, 2017

If this is a common issue we should also fix https://github.com/ManageIQ/manageiq/blob/master/app/models/metric/targets.rb#L56 to not add invalid containers to the metric targets, cc @simon3z

@agrare agrare force-pushed the bz_1439888_metrics_collection_nil_queue_name branch from fd8a7ba to be1cf3d Compare April 10, 2017 19:30
@miq-bot
Copy link
Member

miq-bot commented Apr 10, 2017

Checked commits agrare/manageiq@48c8a95~...be1cf3d with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
3 files checked, 3 offenses detected

app/models/metric/ci_mixin/capture.rb

@Fryguy Fryguy merged commit d7a82f2 into ManageIQ:master Apr 10, 2017
@Fryguy Fryguy added this to the Sprint 58 Ending Apr 10, 2017 milestone Apr 10, 2017
@Fryguy Fryguy self-assigned this Apr 10, 2017
simaishi pushed a commit that referenced this pull request Apr 11, 2017
…il_queue_name

Handle exception when a metrics target doesn't have an ext_management_system
(cherry picked from commit d7a82f2)

https://bugzilla.redhat.com/show_bug.cgi?id=1441271
@agrare agrare deleted the bz_1439888_metrics_collection_nil_queue_name branch April 11, 2017 14:18
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 9f9249495efd797fca310cfaa887db52b302d3bd
Author: Jason Frey <fryguy9@gmail.com>
Date:   Mon Apr 10 17:50:10 2017 -0400

    Merge pull request #14718 from agrare/bz_1439888_metrics_collection_nil_queue_name
    
    Handle exception when a metrics target doesn't have an ext_management_system
    (cherry picked from commit d7a82f2b0ccf7ca3394fe73bfa768b4417698ae5)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1441271

simaishi pushed a commit that referenced this pull request Apr 11, 2017
…il_queue_name

Handle exception when a metrics target doesn't have an ext_management_system
(cherry picked from commit d7a82f2)

https://bugzilla.redhat.com/show_bug.cgi?id=1441272
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit add1b9465d0b8c292b5b6e17cc8a3a4ce6c54d17
Author: Jason Frey <fryguy9@gmail.com>
Date:   Mon Apr 10 17:50:10 2017 -0400

    Merge pull request #14718 from agrare/bz_1439888_metrics_collection_nil_queue_name
    
    Handle exception when a metrics target doesn't have an ext_management_system
    (cherry picked from commit d7a82f2b0ccf7ca3394fe73bfa768b4417698ae5)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1441272

agrare pushed a commit to agrare/manageiq that referenced this pull request Apr 19, 2017
…ection_nil_queue_name

Handle exception when a metrics target doesn't have an ext_management_system
(cherry picked from commit d7a82f2)

https://bugzilla.redhat.com/show_bug.cgi?id=1441272
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants