From 57114d7cab118b38d32f44e35d529210edba88e1 Mon Sep 17 00:00:00 2001 From: Ladislav Smola Date: Thu, 27 Apr 2017 12:47:08 +0200 Subject: [PATCH] Allow all networks to be attached directly to a VM Allow all networks to be attached directly to a VM Fixes BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1431370 --- .../openstack/cloud_manager/cloud_tenant.rb | 4 ---- .../cloud_manager/provision_workflow.rb | 9 --------- .../providers/openstack/network_manager.rb | 1 - .../network_manager/cloud_network/public.rb | 3 --- .../cloud_manager/provision_workflow_spec.rb | 18 +++++++++++------- .../cloud_manager/refresh_spec_common.rb | 19 ++++++++++++------- 6 files changed, 23 insertions(+), 31 deletions(-) diff --git a/app/models/manageiq/providers/openstack/cloud_manager/cloud_tenant.rb b/app/models/manageiq/providers/openstack/cloud_manager/cloud_tenant.rb index 03324dfc4..6e7b6e86f 100644 --- a/app/models/manageiq/providers/openstack/cloud_manager/cloud_tenant.rb +++ b/app/models/manageiq/providers/openstack/cloud_manager/cloud_tenant.rb @@ -8,10 +8,6 @@ class ManageIQ::Providers::Openstack::CloudManager::CloudTenant < ::CloudTenant has_many :private_networks, :class_name => "ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Private" - def all_private_networks - private_networks + (try(:ext_management_system).try(:private_networks).try(:where, :shared => true) || []) - end - def self.raw_create_cloud_tenant(ext_management_system, options) tenant = nil ext_management_system.with_provider_connection(connection_options) do |service| diff --git a/app/models/manageiq/providers/openstack/cloud_manager/provision_workflow.rb b/app/models/manageiq/providers/openstack/cloud_manager/provision_workflow.rb index 594742988..4de03225c 100644 --- a/app/models/manageiq/providers/openstack/cloud_manager/provision_workflow.rb +++ b/app/models/manageiq/providers/openstack/cloud_manager/provision_workflow.rb @@ -50,15 +50,6 @@ def prepare_volumes_fields(values) volumes end - def allowed_cloud_networks(_options = {}) - # We want only non external networks to be connectable directly to the Vm - return {} unless (src_obj = provider_or_tenant_object) - - src_obj.all_private_networks.each_with_object({}) do |cn, hash| - hash[cn.id] = cn.cidr.blank? ? cn.name : "#{cn.name} (#{cn.cidr})" - end - end - def allowed_floating_ip_addresses(_options = {}) # We want to show only floating IPs connected to the cloud_network via router, respecting the owner tenant of the # floating ip diff --git a/app/models/manageiq/providers/openstack/network_manager.rb b/app/models/manageiq/providers/openstack/network_manager.rb index 86184ac6e..00ec885f8 100644 --- a/app/models/manageiq/providers/openstack/network_manager.rb +++ b/app/models/manageiq/providers/openstack/network_manager.rb @@ -23,7 +23,6 @@ class ManageIQ::Providers::Openstack::NetworkManager < ManageIQ::Providers::Netw :class_name => "ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Public" has_many :private_networks, :foreign_key => :ems_id, :dependent => :destroy, :class_name => "ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Private" - alias_method :all_private_networks, :private_networks # Auth and endpoints delegations, editing of this type of manager must be disabled delegate :authentication_check, diff --git a/app/models/manageiq/providers/openstack/network_manager/cloud_network/public.rb b/app/models/manageiq/providers/openstack/network_manager/cloud_network/public.rb index 8285541af..3043db682 100644 --- a/app/models/manageiq/providers/openstack/network_manager/cloud_network/public.rb +++ b/app/models/manageiq/providers/openstack/network_manager/cloud_network/public.rb @@ -1,5 +1,2 @@ class ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Public < ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork - has_many :vms, -> { distinct }, :through => :network_routers - has_many :network_routers, :foreign_key => :cloud_network_id - has_many :private_networks, -> { distinct }, :through => :network_routers, :source => :cloud_networks end diff --git a/spec/models/manageiq/providers/openstack/cloud_manager/provision_workflow_spec.rb b/spec/models/manageiq/providers/openstack/cloud_manager/provision_workflow_spec.rb index f3a29a9da..661223710 100644 --- a/spec/models/manageiq/providers/openstack/cloud_manager/provision_workflow_spec.rb +++ b/spec/models/manageiq/providers/openstack/cloud_manager/provision_workflow_spec.rb @@ -279,27 +279,31 @@ :cloud_tenant => @ct2, :ext_management_system => provider.network_manager) - @cn_shared = FactoryGirl.create(:cloud_network_private_openstack, - :shared => true, - :cloud_tenant => @ct2, - :ext_management_system => provider.network_manager) + @cn_shared = FactoryGirl.create(:cloud_network_private_openstack, + :shared => true, + :cloud_tenant => @ct2, + :ext_management_system => provider.network_manager) + @cn_public_shared = FactoryGirl.create(:cloud_network_public_openstack, + :shared => true, + :cloud_tenant => @ct2, + :ext_management_system => provider.network_manager) end it "#allowed_cloud_networks with tenant selected" do workflow.values.merge!(:cloud_tenant => @ct2.id) cns = workflow.allowed_cloud_networks - expect(cns.keys).to match_array [@cn2.id, @cn_shared.id] + expect(cns.keys).to match_array [@cn2.id, @cn3.id, @cn_shared.id, @cn_public_shared.id] end it "#allowed_cloud_networks with another tenant selected" do workflow.values[:cloud_tenant] = @ct1.id cns = workflow.allowed_cloud_networks - expect(cns.keys).to match_array [@cn1.id, @cn_shared.id] + expect(cns.keys).to match_array [@cn1.id, @cn_shared.id, @cn_public_shared.id] end it "#allowed_cloud_networks with tenant not selected" do cns = workflow.allowed_cloud_networks - expect(cns.keys).to match_array [@cn2.id, @cn1.id, @cn_shared.id] + expect(cns.keys).to match_array [@cn2.id, @cn3.id, @cn1.id, @cn_shared.id, @cn_public_shared.id] end end diff --git a/spec/models/manageiq/providers/openstack/cloud_manager/refresh_spec_common.rb b/spec/models/manageiq/providers/openstack/cloud_manager/refresh_spec_common.rb index c2fff1df4..eddb68759 100644 --- a/spec/models/manageiq/providers/openstack/cloud_manager/refresh_spec_common.rb +++ b/spec/models/manageiq/providers/openstack/cloud_manager/refresh_spec_common.rb @@ -486,9 +486,9 @@ def assert_networks if network.external_facing? vms = network.private_networks.map { |x| (network_vms(x) + network_stacks(x)) }.flatten.uniq - expect(network.vms.count).to eq vms.count + expect(network.public_network_vms.count).to eq vms.count - non_stack_vms = network.vms.select { |x| x.orchestration_stack.blank? } + non_stack_vms = network.public_network_vms.select { |x| x.orchestration_stack.blank? } non_stack_expected_vms = network.private_networks.map { |x| (network_vms(x)) }.flatten.uniq expect(non_stack_vms.map(&:name)).to match_array(non_stack_expected_vms.map { |x| x[:name] }) else @@ -516,20 +516,25 @@ def assert_specific_networks expect(network.ext_management_system).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager) expect(network.cloud_subnets.count).to be > 0 expect(network.cloud_subnets.first).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::CloudSubnet) - expect(network.vms.count).to be > 0 - expect(network.vms.first).to be_kind_of(ManageIQ::Providers::Openstack::CloudManager::Vm) - expect(network.network_routers.count).to be > 0 - expect(network.network_routers.first).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::NetworkRouter) + if network.external_facing? # It's a public network + expect(network.public_network_vms.count).to be > 0 + expect(network.public_network_vms.first).to be_kind_of(ManageIQ::Providers::Openstack::CloudManager::Vm) + expect(network.public_network_routers.count).to be > 0 + expect(network.public_network_routers.first).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::NetworkRouter) expect(network).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Public) expect(network.private_networks.count).to be > 0 expect(network.floating_ips.count).to be > 0 expect(network.private_networks.first).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Private) - assert_objects_with_hashes(network.network_routers, network_data.routers(network.name)) + assert_objects_with_hashes(network.public_network_routers, network_data.routers(network.name)) else # It's a private network + expect(network.vms.count).to be > 0 + expect(network.vms.first).to be_kind_of(ManageIQ::Providers::Openstack::CloudManager::Vm) + expect(network.network_routers.count).to be > 0 + expect(network.network_routers.first).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::NetworkRouter) expect(network).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Private) expect(network.public_networks.count).to be > 0 expect(network.public_networks.first).to be_kind_of(ManageIQ::Providers::Openstack::NetworkManager::CloudNetwork::Public)