Stop nils from appearing in list of hosts to test OSP ssh keys against

[mansam]

This fixes a scenario when validating a new SSH key where Openstack Infra hosts that were powered off would cause validation to throw an exception. The enumeration used to collection hosts was returning nil for powered off hosts instead of excluding them from the list, resulting in an attempt to call verify_credentials against nil.

This fixes the issue described in https://bugzilla.redhat.com/show_bug.cgi?id=1376605

@petrblaho
@tzumainn

[petrblaho]

@mansam you are right about that nil that can appear. This is bug in that code I wrote :-)

But I am not happy with removing that sorting and slicing by cluster_id. This code caused that only 1 host of each cluster will be tried. In real life scenario it can take a lot of time to try sshing into all powered hosts so we settled with using only 1 host from each cluster. Did this requirement changed, @tzumainn ?

If we settle on using all hosts I am happy to ack this. If we still need to use only 1 host from each cluster I think appending .compact to remove nils should work.

[mansam]

@petrblaho Ah, I see. Yes, compact would work as well if that is a requirement to test only one host per cluster.

[tzumainn]

@miq-bot add_label euwe/yes
@miq-bot add_label blocker

[petrblaho]

Works for me 👍

[tzumainn]

@miq-bot add_label euwe/yes
@miq-bot add_label blocker

[tzumainn]

@petrblaho thanks for taking a look! looks good to me as well.

[tzumainn]

@miq-bot assign @blomquisg

[blomquisg]

@mansam can you throw a test together for this? I think I understand what the problem was, but would be nice to see a test validate it.

[mansam]

@blomquisg tests added. :)

[miq-bot]

Some comments on commits mansam/manageiq@3c80a31~...96c7e28

spec/models/manageiq/providers/openstack/infra_manager_spec.rb

• ⚠️ - 21 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
• ⚠️ - 28 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.
• ⚠️ - 35 - Detected allow_any_instance_of. This RSpec method is highly discouraged, please only use when absolutely necessary.

[miq-bot]

Checked commits mansam/manageiq@3c80a31~...96c7e28 with ruby 2.2.5, rubocop 0.37.2, and haml-lint 0.16.1
2 files checked, 3 offenses detected

spec/models/manageiq/providers/openstack/infra_manager_spec.rb

• ❕ - Line 21, Col 121 - Metrics/LineLength - Line is too long. [128/120]
• ❕ - Line 28, Col 121 - Metrics/LineLength - Line is too long. [129/120]
• ❕ - Line 35, Col 121 - Metrics/LineLength - Line is too long. [129/120]

[petrblaho]

Code from this PR is included in #13445

@mansam you can close this PR.

[mansam]

Thanks @petrblaho!