Bug 1439888

Summary: queue_name_for_metrics_collection raises an exception when ems is nil
Product: Red Hat CloudForms Management Engine Reporter: Adam Grare <agrare>
Component: C&U Capacity and UtilizationAssignee: Adam Grare <agrare>
Status: CLOSED CURRENTRELEASE QA Contact: Einat Pacifici <epacific>
Severity: high Docs Contact:
Priority: high    
Version: 5.7.0CC: cpelland, fsimonce, jhardy, obarenbo, simaishi
Target Milestone: GAKeywords: TestOnly
Target Release: 5.9.0   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: 5.9.0.1 Doc Type: If docs needed, set a value
Doc Text:
Story Points: ---
Clone Of:
: 1441271 1441272 (view as bug list) Environment:
Last Closed: 2018-03-06 14:38:54 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: Container Management Target Upstream Version:
Embargoed:
Bug Depends On:    
Bug Blocks: 1441271, 1441272    

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