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.

Sign up for GitHub

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

Jump to bottom

Override az_zone_to_cloud_network in openstack prov #202

Merged
merged 2 commits into from
Jan 19, 2018
Merged

Override az_zone_to_cloud_network in openstack prov #202

merged 2 commits into from
Jan 19, 2018

Conversation

d-m-u
Copy link
Contributor

@d-m-u d-m-u commented Jan 18, 2018 •
edited
Loading

I messed up here should be reverted because the logic in main is wrong. That else case should not include cloud_subnets: fixed in ManageIQ/manageiq#16824. Per comment we need to override this method in google and openstack because the availability zone to subnet relationship isn't set up yet.

Fixes issue caused by fix of https://bugzilla.redhat.com/show_bug.cgi?id=1533277
(Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1535189)

Depends on:

ManageIQ/manageiq#16824

Related to:

ManageIQ/manageiq-providers-google#43

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 18, 2018

@miq-bot assign @gmcculloug
@miq-bot add_label bug
@miq-bot add_label blocker

@d-m-u d-m-u mentioned this pull request Jan 18, 2018
Merged
@mansam
Copy link
Contributor

mansam commented Jan 18, 2018

So once this and ManageIQ/manageiq#16824 are merged are we good, or is there another follow-up PR coming?

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 18, 2018 •
edited
Loading

We have two more changes coming because both openstack and google need to override the def availability_zone_to_cloud_network(src) method that's in the main repo PR

@d-m-u d-m-u mentioned this pull request Jan 18, 2018
Merged
@miq-bot
Copy link
Member

miq-bot commented Jan 18, 2018

Checked commits d-m-u/manageiq-providers-openstack@ede176b~...444347c with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
2 files checked, 0 offenses detected
Everything looks fine. ⭐

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 18, 2018

@syncrou @gmcculloug can you two please give this (hopefully) a final look?

@d-m-u d-m-u changed the title Remove changes introduced [here](https://github.com/ManageIQ/manageiq-providers-openstack/pull/197) Override az_zone_to_cloud_network in openstack prov Jan 18, 2018
@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 18, 2018

@miq-bot add_label gaprindashvili/yes

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 18, 2018 •
edited
Loading

The test that's failing is expected because the call references a line that got changed in the linked main pr here

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 18, 2018

@mansam if you don't mind, I'd like GM to review before this has anything happen to it, please

@gmcculloug gmcculloug mentioned this pull request Jan 18, 2018
Merged
syncrou
syncrou approved these changes Jan 18, 2018
View reviewed changes
Copy link
Contributor

@syncrou syncrou left a comment

Choose a reason for hiding this comment

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

👍 LGTM

@d-m-u d-m-u closed this Jan 19, 2018
@d-m-u d-m-u reopened this Jan 19, 2018
@gmcculloug
Copy link
Member

@mansam Can you review? This is a followup to PR #197.

@d-m-u
Copy link
Contributor Author

d-m-u commented Jan 19, 2018

Kicked, but expecting to fail with https://travis-ci.org/ManageIQ/manageiq-providers-openstack/jobs/330414913#L2076 error because all of openstack is red at the moment

@mansam mansam merged commit a21cd5e into ManageIQ:master Jan 19, 2018
@d-m-u d-m-u deleted the fixing_cloud_network_thingy branch January 19, 2018 14:15
simaishi pushed a commit that referenced this pull request Jan 19, 2018
@mansam @simaishi
Merge pull request #202 from d-m-u/fixing_cloud_network_thingy
086f4fd 
Override az_zone_to_cloud_network in openstack prov
(cherry picked from commit a21cd5e)

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

Gaprindashvili backport details:

$ git log -1
commit 086f4fd41f2f7a333c3abd23e34d0a443c8dd903
Author: Samuel Lucidi <mansam@csh.rit.edu>
Date:   Fri Jan 19 09:15:15 2018 -0500

    Merge pull request #202 from d-m-u/fixing_cloud_network_thingy
    
    Override az_zone_to_cloud_network in openstack prov
    (cherry picked from commit a21cd5e1f53d2940bbd95d29ed07265a9ebfc800)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1536509

@aufi aufi added this to the Sprint 78 Ending Jan 29, 2018 milestone Jan 30, 2018
@gmcculloug gmcculloug mentioned this pull request Mar 19, 2018
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gmcculloug gmcculloug gmcculloug approved these changes

@syncrou syncrou syncrou approved these changes

Assignees

@gmcculloug gmcculloug

Labels
blocker bug gaprindashvili/backported
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@d-m-u @mansam @miq-bot @gmcculloug @simaishi @syncrou @aufi