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

Filter out duplicates during inventory collection #212

Merged
merged 3 commits into from
Jan 31, 2018

Conversation

mansam
Copy link
Contributor

@mansam mansam commented Jan 29, 2018

Initial fix for https://bugzilla.redhat.com/show_bug.cgi?id=1528663

There are some disadvantages that make it difficult to fix the bug by changing pagination in openstack_handle::pagination::marker.rb, since pagination can be different for each service and collection, Fog can make it difficult to get the raw response, and some Fog collections behave differently from others. To avoid these problems, this fix implements a filter for uniqueness at the point where the inventory is collected.

Long term the correct fix is to totally reimplement the way that paging is done in OpenstackHandle/Fog.

@@ -15,7 +15,7 @@ def availability_zones

def cloud_services
return @cloud_services if @cloud_services.any?
@cloud_services = compute_service.handled_list(:services, {}, openstack_admin?)
@cloud_services = uniques(compute_service.handled_list(:services, {}, openstack_admin?))
Copy link
Contributor

Choose a reason for hiding this comment

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

we can ignore graph refresh, its indexes will not allow duplicate records to be passed to persistor

@miq-bot miq-bot added the wip label Jan 29, 2018
@mansam mansam force-pushed the filter-out-uniques-from-fog-response branch from caa95f5 to 7a752b8 Compare January 29, 2018 19:53
@miq-bot
Copy link
Member

miq-bot commented Jan 29, 2018

This pull request is not mergeable. Please rebase and repush.

@Ladas
Copy link
Contributor

Ladas commented Jan 29, 2018

@mansam we will need to rerecord the VCRs, conflict with the storage targeted refresh?

@mansam
Copy link
Contributor Author

mansam commented Jan 29, 2018

@Ladas Probably. They're currently rerecording.

@mansam mansam force-pushed the filter-out-uniques-from-fog-response branch from 7a752b8 to 90584e7 Compare January 29, 2018 22:35
@miq-bot
Copy link
Member

miq-bot commented Jan 29, 2018

Checked commits mansam/manageiq-providers-openstack@02631e1~...90584e7 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
30 files checked, 4 offenses detected

**

  • 💣 💥 🔥 🚒 - Linter/Yaml - missing config files

app/models/manageiq/providers/openstack/refresh_parser_common/helper_methods.rb

@mansam mansam changed the title [WIP] Filter out duplicates during inventory collection Filter out duplicates during inventory collection Jan 30, 2018
@miq-bot miq-bot removed the wip label Jan 30, 2018
Copy link
Contributor

@Ladas Ladas left a comment

Choose a reason for hiding this comment

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

@aufi looks good to me and it passes specs. So I am good with merging.

@mansam We should also add spec reproducing the original issue? So we know it'gets backported right?

@aufi
Copy link
Member

aufi commented Jan 31, 2018

Looks good to me, merging!

@aufi aufi merged commit 1bd67ea into ManageIQ:master Jan 31, 2018
@aufi aufi added this to the Sprint 79 Ending Feb 12, 2018 milestone Jan 31, 2018
@mansam
Copy link
Contributor Author

mansam commented Jan 31, 2018

@Ladas Yeah, specs to reproduce this are on my log of things to do.

@mansam
Copy link
Contributor Author

mansam commented Feb 1, 2018

@miq-bot add_label fine/yes

@simaishi
Copy link
Contributor

simaishi commented Feb 1, 2018

@mansam For backporting to Gaprindashvili branch, all VCR files are conflicting on cherry-pick. If I should take all VCR files from this PR ignoring the conflict, I can handle that. Otherwise, please create a PR for Gaprindashvili.

For backporting to Fine branch, there are only 3 files that are clean cherry-pick. Please create a PR for Fine branch.

@simaishi
Copy link
Contributor

simaishi commented Feb 1, 2018

Backported to Gaprindashvili via #219

@simaishi
Copy link
Contributor

simaishi commented Feb 2, 2018

Backported to Fine via ManageIQ/manageiq#16934

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.

5 participants