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

Extend NetworkManager inventory #9

Merged
merged 1 commit into from
Sep 20, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,4 +1,12 @@
class ManageIQ::Providers::AzureStack::Inventory::Collector::NetworkManager < ManageIQ::Providers::AzureStack::Inventory::Collector
def networks
azure_network.virtual_networks.list_all
end

def network_ports
azure_network.network_interfaces.list_all
end

def security_groups
azure_network.network_security_groups.list_all
end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,59 @@ def parse
log_header = "Collecting data for EMS : [#{collector.manager.name}] id: [#{collector.manager.id}]"
$azure_stack_log.info("#{log_header}...")

cloud_networks
network_ports
security_groups

$azure_stack_log.info("#{log_header}...Complete")
end

def cloud_networks
collector.networks.each do |network|
cloud_network = persister.cloud_networks.build(
:name => network.name,
:ems_ref => network.id.downcase
)

cloud_subnets(network, cloud_network) if network.subnets
end
end

def cloud_subnets(network, cloud_network)
network.subnets.each do |subnet|
persister.cloud_subnets.build(
:name => subnet.name,
:ems_ref => subnet.id.downcase,
:cloud_network => cloud_network,
:security_groups => build_security_groups(subnet)
)
end
end

def network_ports
collector.network_ports.each do |port|
persister.network_ports.build(
:name => port.name,
:ems_ref => port.id.downcase,
:mac_address => port.mac_address,
:device => persister.vms.lazy_find(port.virtual_machine&.id&.downcase),
:security_groups => build_security_groups(port)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure we need utility function here because we're just filling in a simple relation. I'd go with a simple IF statement at the top instead:

security_groups = []
if (security_group_id = port.network_security_group&.id&.downcase)
  security_groups = [persister.security_groups.lazy_find(security_group_id)]
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is exactly how I used to have it initially, when I hooked security_groups to network ports only. But then I needed the same logic for subnets and I thought it made sense to extract it to a helper function (note that this is also mirrored by the assert_security_groups_binding which is used in both assert_specific_subnet and assert_specific_network_port).

I did modify the helper itself to make use of &. when accessing entity's security group though 👍

)
end
end

# helper method that returns either an empty array or a one-element
# array containing the entity's security group.
def build_security_groups(entity)
security_groups = []
security_group_id = entity.network_security_group&.id&.downcase
if security_group_id
security_groups << persister.security_groups.lazy_find(security_group_id)
end

security_groups
end

def security_groups
collector.security_groups.each do |security_group|
persister.security_groups.build(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,18 @@ module ManageIQ::Providers::AzureStack::Inventory::Persister::Definitions::Netwo
extend ActiveSupport::Concern

def initialize_network_inventory_collections
add_collection(network, :security_groups)
%i[cloud_networks
cloud_subnets
network_ports
security_groups].each do |name|
add_collection(network, name)
end

add_collection(cloud, :vms) do |builder|
builder.add_properties(
:parent => manager.parent_manager,
:strategy => :local_db_find_references
)
end
end
end
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
class ManageIQ::Providers::AzureStack::NetworkManager < ManageIQ::Providers::NetworkManager
require_nested :RefreshWorker
require_nested :Refresher
require_nested :CloudNetwork
require_nested :CloudSubnet
require_nested :NetworkPort
require_nested :SecurityGroup

delegate :authentication_check,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class ManageIQ::Providers::AzureStack::NetworkManager::CloudNetwork < ::CloudNetwork
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class ManageIQ::Providers::AzureStack::NetworkManager::CloudSubnet < ::CloudSubnet
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
class ManageIQ::Providers::AzureStack::NetworkManager::NetworkPort < ::NetworkPort
end
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,9 @@
"type": "Microsoft.Network/virtualNetworks",
"name": "[variables('virtualNetworkName')]",
"location": "[variables('location')]",
"dependsOn": [
"[variables('networkSecurityGroupName')]"
],
"properties": {
"addressSpace": {
"addressPrefixes": [
Expand All @@ -110,7 +113,10 @@
{
"name": "[variables('subnetName')]",
"properties": {
"addressPrefix": "[variables('subnetPrefix')]"
"addressPrefix": "[variables('subnetPrefix')]",
"networkSecurityGroup": {
"id": "[resourceId('Microsoft.Network/networkSecurityGroups', variables('networkSecurityGroupName'))]"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome 👍

}
}
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,10 @@
let(:security_group) { SecurityGroup.find_by(:name => 'demoSecurityGroup') }
let(:flavor) { Flavor.find_by(:ems_ref => 'standard_a1') }

let(:network) { CloudNetwork.find_by(:name => 'demoNetwork') }
let(:subnet) { CloudSubnet.find_by(:name => 'demoSubnet') }
let(:network_port) { NetworkPort.find_by(:name => 'demoNic0') }

let(:saving_strategy) { :recursive }
let(:saver_strategy) { :default }
let(:use_ar) { true }
Expand Down Expand Up @@ -67,6 +71,9 @@ def assert_inventory
assert_specific_flavor
assert_specific_vm
assert_specific_orchestration_stack
assert_specific_network
assert_specific_subnet
assert_specific_network_port
assert_security_group
end

Expand All @@ -76,6 +83,9 @@ def assert_table_counts
expect(AvailabilityZone.count).to eq(1)
expect(Vm.count).to eq(1)
expect(Flavor.count).to eq(70)
expect(CloudNetwork.count).to eq(1)
expect(CloudSubnet.count).to eq(1)
expect(NetworkPort.count).to eq(1)
expect(SecurityGroup.count).to eq(1)
expect(OrchestrationStack.count).to eq(1)
end
Expand Down Expand Up @@ -143,6 +153,39 @@ def assert_specific_orchestration_stack
)
end

def assert_specific_network
expect(network).not_to be_nil
expect(ems_ref_suffix(network.ems_ref)).to match(%r{^/providers/microsoft.network/virtualnetworks/[^/]+$})

expect(network.cloud_subnets).not_to be_nil
expect(network.cloud_subnets.size).to eq(1)
end

def assert_specific_subnet
expect(subnet).not_to be_nil
expect(ems_ref_suffix(subnet.ems_ref)).to match(%r{^/providers/microsoft.network/virtualnetworks/[^/]+/subnets/[^/]+$})

expect(subnet.cloud_network).to eq(network)

assert_security_groups_binding(subnet)
end

def assert_specific_network_port
expect(network_port).not_to be_nil
expect(ems_ref_suffix(network_port.ems_ref)).to match(%r{^/providers/microsoft.network/networkinterfaces/[^/]+$})
expect(network_port.mac_address).to eq('001DD8B70047')

expect(network_port.device).to eq(vm)

assert_security_groups_binding(network_port)
end

def assert_security_groups_binding(entity)
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this DRY approach you took here 👍

expect(entity.security_groups).not_to be_nil
expect(entity.security_groups.size).to eq(1)
expect(entity.security_groups.first).to eq(security_group)
end

def assert_security_group
expect(security_group).not_to be_nil
expect(ems_ref_suffix(security_group.ems_ref)).to match(%r{^/providers/microsoft.network/networksecuritygroups/[^/]+$})
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading