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

Fix disable CloudTenant Vm targeted refresh #213

Merged
merged 1 commit into from
Jan 29, 2018

Conversation

aufi
Copy link
Member

@aufi aufi commented Jan 29, 2018

Vm targeted refresh queued related Cloud Tenant refresh, that could fail due to
keystone v3 backend in fog or an issue in fog/openstack method get_project_by_id.

Queueing CloudTenant doesn't seem to be actually needed - disabled now and we should
make a cleanup in ongoing PR.

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

@mansam
Copy link
Contributor

mansam commented Jan 29, 2018

@aufi thanks for the fix! also the targeted refresh vcr needs to be updated :)

@aufi aufi force-pushed the fix_vm_targeted_refresh_tenant branch from 539c2cf to 0152f96 Compare January 29, 2018 17:11
Vm tagreted refresh queued related Cloud Tenant refresh, that could fail due to
keystone v3 backend in fog or an issue in fog/openstack method get_project_by_id.

Adding custom method to get_project derived from list_project.

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1538741
@aufi aufi force-pushed the fix_vm_targeted_refresh_tenant branch from 0152f96 to 34de6c3 Compare January 29, 2018 17:51
@miq-bot
Copy link
Member

miq-bot commented Jan 29, 2018

Checked commit aufi@34de6c3 with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 2 offenses detected

app/models/manageiq/providers/openstack/inventory/collector/target_collection.rb

@aufi
Copy link
Member Author

aufi commented Jan 29, 2018

@mansam Yes, thanks, based on @Ladas feedback, I trying find a way with performing custom get_project query, but didn't reach working auth (still getting 403 with https://gist.github.com/aufi/d014cc327081a709fcdef328b3444cb7). So decided to use list_projects and filter it in ruby, planning to fix it in more nice way updating fog with fog/fog-openstack#358

@@ -254,7 +254,7 @@ def infer_related_cloud_tenant_ems_refs_db!

def infer_related_cloud_tenant_ems_refs_api!
tenants.each do |tenant|
add_simple_target(:cloud_tenants, tenant.try(:parent_id)) unless tenant.try(:parent_id).blank?
add_simple_target!(:cloud_tenants, tenant.try(:parent_id)) unless tenant.try(:parent_id).blank?
Copy link
Member Author

Choose a reason for hiding this comment

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

@mansam I've got error here without exclamation mark, so added it, is it ok?

Copy link
Contributor

Choose a reason for hiding this comment

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

@aufi yep!

@aufi aufi requested review from mansam and Ladas January 29, 2018 17:58
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.

Looks good, we can't have a better fix now, since it;s broken inside fog-openstack

@Ladas Ladas merged commit e34789f into ManageIQ:master Jan 29, 2018
simaishi pushed a commit that referenced this pull request Jan 29, 2018
Fix disable CloudTenant Vm targeted refresh
(cherry picked from commit e34789f)

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

Gaprindashvili backport details:

$ git log -1
commit a071dd63fb167be49bc008d5a664442e71ee9217
Author: Ladislav Smola <[email protected]>
Date:   Mon Jan 29 19:12:59 2018 +0100

    Merge pull request #213 from aufi/fix_vm_targeted_refresh_tenant
    
    Fix disable CloudTenant Vm targeted refresh
    (cherry picked from commit e34789f7d6ee50a5768b6e9fa71f5ede1b98e394)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1539874

@aufi aufi added this to the Sprint 78 Ending Jan 29, 2018 milestone Jan 30, 2018
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