Adding connection b/w physical servers and physical switches

[douglasgabriel]

This PR is able to:

• Bind the connected physical network ports;
• Bind the connected physical servers and physical switches;

It is prerequisite to:

• Adding connections between servers and switches on topology page manageiq-ui-classic#3799

Depends on:

• Adding parsing steps for switches manageiq-providers-lenovo#132 - Merged
• Adding PhysicalNetworkPort model #17268 - Merged
• Refactoring the Network Devices Parser to save ports information on physical_network_ports table manageiq-providers-lenovo#152 - Merged
• Adding column on physical_network_ports table to connect other ports manageiq-schema#208 - Merged

[miq-bot]

This pull request is not mergeable. Please rebase and repush.

[douglasgabriel]

@miq-bot remove_label unmergeable

[douglasgabriel]

@miq-bot assign @agrare

[agrare]

Hm not a fan of doing the queries every time we call #connected_port. Is this something we can do during refresh? E.g. could we set a peer_port_id?

[douglasgabriel]

Hmm, good point, I can see the problem. I will do it in the refresh cycle. Thanks

[agrare]

I like how this came out much better 👍

[agrare]

You should do this through rails associations instead of loops and finds.

Give this a shot:

diff --git a/app/models/guest_device.rb b/app/models/guest_device.rb
index cd4791532a..09db2943cc 100644
--- a/app/models/guest_device.rb
+++ b/app/models/guest_device.rb
@@ -16,6 +16,7 @@ class GuestDevice < ApplicationRecord
   has_many :child_devices, -> { where(:parent_device_id => ids) }, :foreign_key => "parent_device_id", :class_name => "GuestDevice", :dependent => :destroy
 
   has_many :physical_network_ports, :dependent => :destroy
+  has_many :connected_physical_switches, :through => :physical_network_ports
 
   alias_attribute :name, :device_name
 
diff --git a/app/models/hardware.rb b/app/models/hardware.rb
index af0d5d155e..4168d913ba 100644
--- a/app/models/hardware.rb
+++ b/app/models/hardware.rb
@@ -22,6 +22,7 @@ class Hardware < ApplicationRecord
   has_many    :nics, -> { where("device_type = 'ethernet'") }, :class_name => "GuestDevice", :foreign_key => :hardware_id
   has_many    :ports, -> { where("device_type != 'storage'") }, :class_name => "GuestDevice", :foreign_key => :hardware_id
   has_many    :physical_ports, -> { where("device_type = 'physical_port'") }, :class_name => "GuestDevice", :foreign_key => :hardware_id
+  has_many    :connected_physical_switches, :through => :guest_devices
 
   virtual_column :ipaddresses,   :type => :string_set, :uses => :networks
   virtual_column :hostnames,     :type => :string_set, :uses => :networks
diff --git a/app/models/physical_network_port.rb b/app/models/physical_network_port.rb
index 3b43db5b82..cb627d900f 100644
--- a/app/models/physical_network_port.rb
+++ b/app/models/physical_network_port.rb
@@ -1,6 +1,7 @@
 class PhysicalNetworkPort < ApplicationRecord
   belongs_to :guest_device
-  belongs_to :physical_switch
+  belongs_to :physical_switch, :foreign_key => :switch_id
 
   has_one :connected_port, :foreign_key => "connected_port_uid", :primary_key => "uid_ems", :class_name => "PhysicalNetworkPort", :dependent => :nullify
+  has_one :connected_physical_switch, :through => :connected_port, :source => :physical_switch

diff --git a/app/models/physical_server.rb b/app/models/physical_server.rb
index 275637078d..1d598fce2c 100644
--- a/app/models/physical_server.rb
+++ b/app/models/physical_server.rb
@@ -106,23 +106,5 @@ class PhysicalServer < ApplicationRecord
     host.try(:vmm_product).nil? ? N_("") : host.vmm_product
   end
 
+  has_many :physical_switches, :through => :hardware, :source => :connected_physical_switches

This way I was able to do:

>> server.physical_switches
  PhysicalSwitch Load (1.0ms)  SELECT "switches".* FROM "switches" INNER JOIN "physical_network_ports" ON "switches"."id" = "physical_network_ports"."switch_id" INNER JOIN "physical_network_ports" "physical_network_ports_physical_switches" ON "physical_network_ports"."connected_port_uid" = "physical_network_ports_physical_switches"."uid_ems" INNER JOIN "guest_devices" ON "physical_network_ports_physical_switches"."guest_device_id" = "guest_devices"."id" INNER JOIN "hardwares" ON "guest_devices"."hardware_id" = "hardwares"."id" INNER JOIN "computer_systems" ON "hardwares"."computer_system_id" = "computer_systems"."id" WHERE "switches"."type" IN ('PhysicalSwitch', 'ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch') AND "computer_systems"."managed_entity_id" = $1 AND "computer_systems"."managed_entity_type" = $2  [["managed_entity_id", 1], ["managed_entity_type", "PhysicalServer"]]
  PhysicalSwitch Inst Including Associations (16.0ms - 1rows)
=> #<ActiveRecord::Associations::CollectionProxy [#<PhysicalSwitch id: 1, name: nil, ports: nil, created_on: "2018-06-22 07:35:54", updated_on: "2018-06-22 07:35:54", uid_ems: nil, allow_promiscuous: nil, forged_transmits: nil, mac_changes: nil, switch_uuid: nil, shared: nil, mtu: nil, ems_id: nil, type: "PhysicalSwitch", health_state: nil, power_state: nil>]>

[douglasgabriel]

These changes seems to works fine, but, if I try to call (which is what the topology page do)

PhysicalServer.includes(:physical_switches)

I get the following error:

PhysicalServer Load (0.8ms)  SELECT "physical_servers".* FROM "physical_servers"
  PhysicalServer Inst Including Associations (587.0ms - 3rows)
  ComputerSystem Load (0.5ms)  SELECT "computer_systems".* FROM "computer_systems" WHERE "computer_systems"."managed_entity_type" = $1 AND "computer_systems"."managed_entity_id" IN (1, 2, 3)  [["managed_entity_type", "PhysicalServer"]]
  ComputerSystem Inst Including Associations (3.5ms - 3rows)
  Hardware Load (0.6ms)  SELECT "hardwares".* FROM "hardwares" WHERE "hardwares"."computer_system_id" IN (1, 2, 3)
  Hardware Inst Including Associations (16.4ms - 3rows)
  GuestDevice Load (0.4ms)  SELECT "guest_devices".* FROM "guest_devices" WHERE "switches"."type" IN ('PhysicalSwitch', 'ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch') AND "guest_devices"."hardware_id" IN (1, 2, 3)
ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR:  missing FROM-clause entry for table "switches"
LINE 1: ...LECT "guest_devices".* FROM "guest_devices" WHERE "switches"...

[douglasgabriel]

@agrare regarding the error that I reported before, I solved it with a workaround solution, I implemented a method and call it on the topology service to mount the relationships. It is not the prettier solution, but it works. What do you think?

[agrare]

So i see you're calling this here but why not just call :physical_switches there? I'm not sure why this would fix anything

[douglasgabriel]

Not sure why, but I get the error that I said before:

PhysicalServer Load (0.8ms)  SELECT "physical_servers".* FROM "physical_servers"
  PhysicalServer Inst Including Associations (587.0ms - 3rows)
  ComputerSystem Load (0.5ms)  SELECT "computer_systems".* FROM "computer_systems" WHERE "computer_systems"."managed_entity_type" = $1 AND "computer_systems"."managed_entity_id" IN (1, 2, 3)  [["managed_entity_type", "PhysicalServer"]]
  ComputerSystem Inst Including Associations (3.5ms - 3rows)
  Hardware Load (0.6ms)  SELECT "hardwares".* FROM "hardwares" WHERE "hardwares"."computer_system_id" IN (1, 2, 3)
  Hardware Inst Including Associations (16.4ms - 3rows)
  GuestDevice Load (0.4ms)  SELECT "guest_devices".* FROM "guest_devices" WHERE "switches"."type" IN ('PhysicalSwitch', 'ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch') AND "guest_devices"."hardware_id" IN (1, 2, 3)
ActiveRecord::StatementInvalid: PG::UndefinedTable: ERROR:  missing FROM-clause entry for table "switches"
LINE 1: ...LECT "guest_devices".* FROM "guest_devices" WHERE "switches"...

This error happens when you call PhysicalServer.includes(:physical_switches) which is what the topology service does.

With this method, I force the #includes method to call physical_server.physical_switches and then it works.

Basically, the #includes method tries to create separated queries for each join table, but #physical_switches does a single query joining every tables. This is why:

physical_server_instance.physical_switches # works
PhysicalServer.includes(:physical_switches) # doesn't works

[agrare]

@douglasgabriel looks like the assoc from physical_server needs to go through computer system, try this:

diff --git a/app/models/computer_system.rb b/app/models/computer_system.rb
index 062d9bb3cb..7f8ebc722d 100644
--- a/app/models/computer_system.rb
+++ b/app/models/computer_system.rb
@@ -3,4 +3,6 @@ class ComputerSystem < ApplicationRecord
 
   has_one :operating_system, :dependent => :destroy
   has_one :hardware, :dependent => :destroy
+
+  has_many :connected_physical_switches, :through => :hardware
 end
diff --git a/app/models/physical_server.rb b/app/models/physical_server.rb
index d309c35b31..ccbf8f7a3b 100644
--- a/app/models/physical_server.rb
+++ b/app/models/physical_server.rb
@@ -33,7 +33,7 @@ class PhysicalServer < ApplicationRecord
   virtual_column :v_availability, :type => :string, :uses => :host
   virtual_column :v_host_os, :type => :string, :uses => :host
 
-  has_many :physical_switches, :through => :hardware, :source => :connected_physical_switches
+  has_many :physical_switches, :through => :computer_system, :source => :connected_physical_switches
 
   def name_with_details
     details % {
@@ -107,8 +107,4 @@ class PhysicalServer < ApplicationRecord
   def v_host_os
     host.try(:vmm_product).nil? ? N_("") : host.vmm_product
   end
-
-  def connected_switches
-    physical_switches
-  end
 end

>> PhysicalServer.includes(:physical_switches)
PostgreSQLAdapter#log_after_checkout, connection_pool: size: 5, connections: 1, in use: 1, waiting_in_queue: 0
  PhysicalServer Load (0.4ms)  SELECT "physical_servers".* FROM "physical_servers"
  PhysicalServer Inst Including Associations (21.1ms - 1rows)
  SQL (1.7ms)  SELECT "computer_systems"."id" AS t0_r0, "computer_systems"."managed_entity_type" AS t0_r1, "computer_systems"."managed_entity_id" AS t0_r2, "computer_systems"."created_at" AS t0_r3, "computer_systems"."updated_at" AS t0_r4, "switches"."id" AS t1_r0, "switches"."name" AS t1_r1, "switches"."ports" AS t1_r2, "switches"."created_on" AS t1_r3, "switches"."updated_on" AS t1_r4, "switches"."uid_ems" AS t1_r5, "switches"."allow_promiscuous" AS t1_r6, "switches"."forged_transmits" AS t1_r7, "switches"."mac_changes" AS t1_r8, "switches"."switch_uuid" AS t1_r9, "switches"."shared" AS t1_r10, "switches"."mtu" AS t1_r11, "switches"."ems_id" AS t1_r12, "switches"."type" AS t1_r13, "switches"."health_state" AS t1_r14, "switches"."power_state" AS t1_r15 FROM "computer_systems" LEFT OUTER JOIN "hardwares" ON "hardwares"."computer_system_id" = "computer_systems"."id" LEFT OUTER JOIN "guest_devices" ON "guest_devices"."hardware_id" = "hardwares"."id" LEFT OUTER JOIN "physical_network_ports" "physical_network_ports_computer_systems_join" ON "physical_network_ports_computer_systems_join"."guest_device_id" = "guest_devices"."id" LEFT OUTER JOIN "physical_network_ports" ON "physical_network_ports"."connected_port_uid" = "physical_network_ports_computer_systems_join"."uid_ems" LEFT OUTER JOIN "switches" ON "switches"."id" = "physical_network_ports"."switch_id" AND "switches"."type" IN ('PhysicalSwitch', 'ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch') WHERE "computer_systems"."managed_entity_type" = $1 AND "switches"."type" IN ('PhysicalSwitch', 'ManageIQ::Providers::Lenovo::PhysicalInfraManager::PhysicalSwitch') AND "computer_systems"."managed_entity_id" = 1  [["managed_entity_type", "PhysicalServer"]]
  ComputerSystem Inst Including Associations (23.8ms - 1rows)
=> #<ActiveRecord::Relation [#<PhysicalServer id: 1, ems_id: nil, name: nil, type: nil, uid_ems: nil, ems_ref: nil, created_at: "2018-06-29 18:36:15", updated_at: "2018-06-29 18:36:15", health_state: nil, power_state: nil, hostname: nil, product_name: nil, manufacturer: nil, machine_type: nil, model: nil, serial_number: nil, field_replaceable_unit: nil, raw_power_state: nil, vendor: nil, location_led_state: nil, physical_rack_id: nil, ems_compliance_name: nil, ems_compliance_status: nil, physical_chassis_id: nil>]>

[douglasgabriel]

Yeah, it works!

[miq-bot]

Checked commit douglasgabriel@af1e882 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
5 files checked, 0 offenses detected
Everything looks fine. 🍪

[agrare]

LGTM