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

Use the host_geust_devices inventory collection #360

Merged

Conversation

agrare
Copy link
Member

@agrare agrare commented Apr 25, 2019

Prevent duplication of host guest_devices by using the correct
association.

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

Prevent duplication of host guest_devices by using the correct
association.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1691109
@agrare agrare added the bug label Apr 25, 2019
@miq-bot
Copy link
Member

miq-bot commented Apr 25, 2019

Checked commit agrare@d4f2903 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. 🍰

Copy link
Contributor

@borod108 borod108 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! How would you say we should test this?
(We can do it in a separate PR thought)

@borod108 borod108 merged commit 49ac642 into ManageIQ:master Apr 25, 2019
@agrare agrare deleted the bz_1691109_use_host_guest_device_association branch April 25, 2019 15:49
@agrare
Copy link
Member Author

agrare commented Apr 25, 2019

@borod108 what we should do is update all the refresh specs to do 2.times {} around the refresh and expect blocks (e.g. https://github.com/ManageIQ/manageiq-providers-vmware/blob/master/spec/models/manageiq/providers/vmware/infra_manager/inventory/collector_spec.rb#L22-L37).

This catches any issues like this where two refreshes run back to back don't yield the same result.

I tried doing this but was getting vcr errors:

bundle exec rspec spec/models/manageiq/providers/redhat/infra_manager/refresh//refresher_4_async_graph_spec.rb
  1) ManageIQ::Providers::Redhat::InfraManager::Refresh::Refresher will perform a full refresh on v4.1
     Failure/Error: raise "connection_vcr could not find a response for: #{req_key}"
     
     RuntimeError:
       connection_vcr could not find a response for: https://localhost:8443/ovirt-engine/api/clusters{}GET
     # ./spec/support/ovirt_sdk/connection_vcr.rb:32:in `wait'
     # /var/lib/gems/2.5.0/gems/ovirt-engine-sdk-4.2.5/lib/ovirtsdk4/service.rb:44:in `wait'

cc @slemrmartin

@agrare
Copy link
Member Author

agrare commented Apr 25, 2019

I tested this against a live RHV system by doing:

EmsRefresh.refresh(ManageIQ::Providers::Redhat::InfraManager.first)
GuestDevice.count

Twice

@agrare
Copy link
Member Author

agrare commented Apr 29, 2019

@simaishi before you backport this we'll need ManageIQ/manageiq@b528ade1615 which adds the host_guest_devices inventory collection

simaishi pushed a commit that referenced this pull request Jul 22, 2019
…association

Use the host_geust_devices inventory collection

(cherry picked from commit 49ac642)

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

Hammer backport details:

$ git log -1
commit 38522a8832afa14e218472b140caca0b54e36ff1
Author: Boris Od <bodnopoz@redhat.com>
Date:   Thu Apr 25 18:48:40 2019 +0300

    Merge pull request #360 from agrare/bz_1691109_use_host_guest_device_association
    
    Use the host_geust_devices inventory collection
    
    (cherry picked from commit 49ac6425ecbcc868d6676a0a9d534120b6de73f0)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1731237

@simaishi
Copy link
Contributor

@agrare thank you for the note, confirmed host_guest_devices is in hammer branch.

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.

None yet

4 participants