Fix call to get_targets_for_source to be correct scope name #16824

d-m-u · 2018-01-15T21:10:50Z

PR #16811 and #16688 broke openstack provisioning because the call needs the all_cloud_networks which was taken out in #16688. The all_cloud_networks includes the shared networks set up in the openstack provider specs here.

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)

Related to:

ManageIQ/manageiq-providers-openstack#202
ManageIQ/manageiq-providers-google#43

related external provider tests:

(which, since this and the above two are blockers, should maybe wait until the code is merged?)
ManageIQ/manageiq-providers-amazon#394
ManageIQ/manageiq-providers-azure#199

d-m-u · 2018-01-15T21:11:48Z

@miq-bot assign @gmcculloug
@miq-bot add_label provisioning
@syncrou I think this fixes our issue with openstack specs

gmcculloug · 2018-01-15T23:11:58Z

app/models/manageiq/providers/cloud_manager/provision_workflow.rb

@@ -27,7 +27,7 @@ def allowed_cloud_subnets(_options = {})

  def allowed_cloud_networks(_options = {})
    return {} unless (src = provider_or_tenant_object)
-    targets = get_targets_for_source(src, :cloud_filter, CloudNetwork, 'cloud_networks')
+    targets = get_targets_for_source(src, :cloud_filter, CloudNetwork, 'all_cloud_networks')


@Ladas This should resolve the issue of returning shared cloud_networks when only the provider or tenant are known. Can you provide some guidance (or recommend someone) for what to do when an availability_zone has been selected in the provisioning UI?

Do we have a way to determine what shared cloud_networks are available then? See the availability_zone_to_cloud_network method below which will be called from the allowed_ci method to limit the cloud_networks.

Cool, I've missed this one.

So looking at miq_provision dialogs, for Google and OpenStack, we can select AvailabilityZone when provisioning, but we do not refresh subnet <- availability_zone relationships. That means we need to disable allowed_ci for them for now (and put into todo list to check if the relation should be there).

For Vcloud, I am not sure, since I don't see provision dialog for it, but we refresh AZ without the relation to subnet.

So answer to Do we have a way to determine what shared cloud_networks are available then? is no. We need to offer all networks, until we can refresh the subnet <- availability_zone relationships in these providers.

Azure and Amazon should work ok with allowed_ci filtering.

That being said, allowed_ci filter is new here, so I think we should do manual test, for each provider, just for sure. And probably add small spec to each provider, something like https://github.com/Ladas/manageiq-providers-openstack/blob/bd5b69a68cf29f41a582d377a544597423586068/spec/models/manageiq/providers/openstack/cloud_manager/provision_workflow_spec.rb#L298

d-m-u · 2018-01-16T19:22:18Z

@miq-bot add_label gaprindashvili/yes

d-m-u · 2018-01-17T15:43:00Z

@Ladas @syncrou GM says "we can address the external repo tests as a follow up" so can we please get this reviewed ASAP?

syncrou

👍 LGTM

d-m-u · 2018-01-18T19:10:02Z

@miq-bot add_label blocker

miq-bot · 2018-01-18T20:43:07Z

Checked commit d-m-u@3eb218e with ruby 2.3.3, rubocop 0.52.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

d-m-u · 2018-01-18T20:45:16Z

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

Ladas

Looks good 👍

syncrou

👍 Looks gooder

gmcculloug · 2018-01-18T22:22:41Z

@Ladas After reviewing your comments and the PRs made so far I think this PR, along with the related provider PRs (ManageIQ/manageiq-providers-openstack#202 and ManageIQ/manageiq-providers-google#43), get us closer to the correct solution.

Now the cloud_workflow can still call allowed_ci and pass a RBAC filtered list so all providers honor RBAC restrictions. Then the providers that do not support the subnet <- availability_zone relationships can override the availability_zone_to_cloud_network method to return the full set.

In the future when that relationship is supported the external repos can remove this override logic and use the base class method.

Thoughts?

Ladas · 2018-01-19T13:52:24Z

@gmcculloug yeah, overriding availability_zone_to_cloud_network to return full set in google and openstack sounds good.

Fix call to get_targets_for_source to be correct scope name (cherry picked from commit f5f9a83) https://bugzilla.redhat.com/show_bug.cgi?id=1536509

simaishi · 2018-01-19T14:45:52Z

Gaprindashvili backport details:

$ git log -1
commit 345484871740c733b99443012e028043f0605a7c
Author: Greg McCullough <gmccullo@redhat.com>
Date:   Fri Jan 19 08:57:57 2018 -0500

    Merge pull request #16824 from d-m-u/fixing_openstack_cloud_network_call
    
    Fix call to get_targets_for_source to be correct scope name
    (cherry picked from commit f5f9a83a1bf7bbb0e66344f5dd30c5922d569d42)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1536509

d-m-u changed the title ~~Add method for cloud_tenant to cloud_network and fix call~~ [WIP] Add method for cloud_tenant to cloud_network and fix call Jan 15, 2018

miq-bot assigned gmcculloug Jan 15, 2018

miq-bot added lifecycle/provisioning wip labels Jan 15, 2018

d-m-u force-pushed the fixing_openstack_cloud_network_call branch 3 times, most recently from 5cc4de5 to 2929e96 Compare January 15, 2018 22:33

gmcculloug reviewed Jan 15, 2018

View reviewed changes

Ladas mentioned this pull request Jan 16, 2018

Ensure that subnets are dissociated from routers in the ManageIQ inventory when their interfaces are removed on the OSP side ManageIQ/manageiq-providers-openstack#182

Merged

chessbyte added bug test labels Jan 16, 2018

This was referenced Jan 16, 2018

Extend allowed_cloud_network for providers that don't support allowed_ci ManageIQ/manageiq-providers-google#41

Closed

Extend allowed_cloud_network for providers that don't support allowed_ci ManageIQ/manageiq-providers-openstack#197

Merged

d-m-u changed the title ~~[WIP] Add method for cloud_tenant to cloud_network and fix call~~ Add method for cloud_tenant to cloud_network and fix call Jan 16, 2018

miq-bot removed the wip label Jan 16, 2018

miq-bot added the gaprindashvili/yes label Jan 16, 2018

d-m-u changed the title ~~Add method for cloud_tenant to cloud_network and fix call~~ Fix call to get_targets_for_source to be correct scope name Jan 16, 2018

This was referenced Jan 16, 2018

Tests cloud_networks with RBAC filtering from super ManageIQ/manageiq-providers-amazon#394

Merged

Add tests to check that cloud network list is dependent on zone ManageIQ/manageiq-providers-azure#199

Merged

syncrou approved these changes Jan 17, 2018

View reviewed changes

miq-bot added the blocker label Jan 18, 2018

Fix call to get_targets_for_source in allowed_cloud_networks

3eb218e

mansam mentioned this pull request Jan 18, 2018

Override az_zone_to_cloud_network in openstack prov ManageIQ/manageiq-providers-openstack#202

Merged

d-m-u force-pushed the fixing_openstack_cloud_network_call branch from 2929e96 to 3eb218e Compare January 18, 2018 20:33

d-m-u mentioned this pull request Jan 18, 2018

Override az_zone_to_cloud_network method in google prov ManageIQ/manageiq-providers-google#43

Merged

Ladas approved these changes Jan 18, 2018

View reviewed changes

syncrou approved these changes Jan 18, 2018

View reviewed changes

gmcculloug merged commit f5f9a83 into ManageIQ:master Jan 19, 2018

gmcculloug added this to the Sprint 78 Ending Jan 29, 2018 milestone Jan 19, 2018

d-m-u deleted the fixing_openstack_cloud_network_call branch January 19, 2018 14:03

simaishi added gaprindashvili/backported and removed gaprindashvili/yes labels Jan 19, 2018

This was referenced Jan 19, 2018

Test availability_zone_to_cn overriding in google prov ManageIQ/manageiq-providers-google#44

Merged

Test az_to_cn method in vmware ManageIQ/manageiq-providers-vmware#177

Merged

Test az_to_cn method from super ManageIQ/manageiq-providers-openstack#203

Merged

d-m-u mentioned this pull request Feb 12, 2018

Allowed cloud network without a zone should return everything ManageIQ/manageiq-providers-azure#207

Merged

simaishi mentioned this pull request Mar 19, 2018

Change get_targets_for_source to use correct params #16811

Merged

This pull request was closed.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix call to get_targets_for_source to be correct scope name #16824

Fix call to get_targets_for_source to be correct scope name #16824

d-m-u commented Jan 15, 2018 •

edited

Loading

d-m-u commented Jan 15, 2018 •

edited

Loading

gmcculloug Jan 15, 2018

Ladas Jan 16, 2018

d-m-u commented Jan 16, 2018

d-m-u commented Jan 17, 2018

syncrou left a comment

d-m-u commented Jan 18, 2018

miq-bot commented Jan 18, 2018

d-m-u commented Jan 18, 2018

Ladas left a comment

syncrou left a comment

gmcculloug commented Jan 18, 2018

Ladas commented Jan 19, 2018

simaishi commented Jan 19, 2018

Fix call to get_targets_for_source to be correct scope name #16824

Fix call to get_targets_for_source to be correct scope name #16824

Conversation

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

Related to:

related external provider tests:

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

gmcculloug Jan 15, 2018

Choose a reason for hiding this comment

Ladas Jan 16, 2018

Choose a reason for hiding this comment

d-m-u commented Jan 16, 2018

d-m-u commented Jan 17, 2018

syncrou left a comment

Choose a reason for hiding this comment

d-m-u commented Jan 18, 2018

miq-bot commented Jan 18, 2018

d-m-u commented Jan 18, 2018

Ladas left a comment

Choose a reason for hiding this comment

syncrou left a comment

Choose a reason for hiding this comment

gmcculloug commented Jan 18, 2018

Ladas commented Jan 19, 2018

simaishi commented Jan 19, 2018

d-m-u commented Jan 15, 2018 •

edited

Loading

d-m-u commented Jan 15, 2018 •

edited

Loading