Bug 1439888 - queue_name_for_metrics_collection raises an exception when ems is nil
Summary: queue_name_for_metrics_collection raises an exception when ems is nil
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: Red Hat CloudForms Management Engine
Classification: Red Hat
Component: C&U Capacity and Utilization
Version: 5.7.0
Hardware: Unspecified
OS: Unspecified
high
high
Target Milestone: GA
: 5.9.0
Assignee: Adam Grare
QA Contact: Einat Pacifici
URL:
Whiteboard:
Depends On:
Blocks: 1441271 1441272
TreeView+ depends on / blocked
 
Reported: 2017-04-06 18:59 UTC by Adam Grare
Modified: 2020-06-11 13:32 UTC (History)
5 users (show)

Fixed In Version: 5.9.0.1
Doc Type: If docs needed, set a value
Doc Text:
Clone Of:
: 1441271 1441272 (view as bug list)
Environment:
Last Closed: 2018-03-06 14:38:54 UTC
Category: ---
Cloudforms Team: Container Management
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Adam Grare 2017-04-06 18:59:46 UTC
Description of problem:
If metrics collection is attempted for an object that doesn't have an ems_id or an old_ems_id then queue_name_for_metrics_collection raises an exception "Unsupported type ManageIQ::Providers::Kubernetes::ContainerManager::Container (id: 1234)"

Version-Release number of selected component (if applicable):
5.7.1.3

Comment 4 Adam Grare 2017-04-10 18:33:01 UTC
This got through the Metric::Targets.capture_container_targets because even though the container has no ems_id/old_ems_id it still is related to a container_group.

Ideally items which cannot have metrics collected for them should not be included in the targets list.

Frederico do you know if it is a normal for a container to have no ems_id and no old_ems_id?

Comment 6 CFME Bot 2017-04-10 21:51:29 UTC
New commit detected on ManageIQ/manageiq/master:
https://github.com/ManageIQ/manageiq/commit/48c8a9579e50920844306e2eedb1593465747bef

commit 48c8a9579e50920844306e2eedb1593465747bef
Author:     Adam Grare <agrare>
AuthorDate: Mon Apr 10 14:33:27 2017 -0400
Commit:     Adam Grare <agrare>
CommitDate: Mon Apr 10 14:33:27 2017 -0400

    Handle an exception from target.perf_capture_queue
    
    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

 app/models/metric/capture.rb | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

Comment 7 CFME Bot 2017-04-10 21:51:34 UTC
New commit detected on ManageIQ/manageiq/master:
https://github.com/ManageIQ/manageiq/commit/be1cf3d4cff88c54b28c5253c726f5e85b35aae0

commit be1cf3d4cff88c54b28c5253c726f5e85b35aae0
Author:     Adam Grare <agrare>
AuthorDate: Mon Apr 10 15:06:28 2017 -0400
Commit:     Adam Grare <agrare>
CommitDate: Mon Apr 10 15:30:30 2017 -0400

    Raise a more useful exception if target.ems is nil
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1439888

 app/models/metric/ci_mixin/capture.rb       | 23 ++++++++++++-----------
 spec/models/metric/ci_mixin/capture_spec.rb |  6 +++---
 2 files changed, 15 insertions(+), 14 deletions(-)

Comment 8 Federico Simoncelli 2017-04-10 22:20:21 UTC
(In reply to Adam Grare from comment #4)
> This got through the Metric::Targets.capture_container_targets because even
> though the container has no ems_id/old_ems_id it still is related to a
> container_group.
> 
> Ideally items which cannot have metrics collected for them should not be
> included in the targets list.

Well the container_group is still attached to an ems (since it's reached through the ems relationship):

  targets += ems.container_groups.flat_map(&:containers)

So the container is expected to be attached as well.
As you saw if both ems_id and old_ems_id are missing then something went extremely wrong.

> Frederico do you know if it is a normal for a container to have no ems_id
> and no old_ems_id?

No it's not expected at all.


The ems_id was added later on to the containers model, we could use that to create the target list, although it will just hide these issues:

https://github.com/ManageIQ/manageiq/pull/14719


Note You need to log in before you can comment on or make changes to this bug.