[Ovirt-devel] [PATCH server] Fixes to the managed_node_configuration code.

[Darryl L. Pierce]

When returning a configuration for the node, a bridge is first created,
using the networking definition for the device. The network interface is
then bridged over that device.

Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
 src/lib/managed_node_configuration.rb              |   26 +++++++------------
 src/test/fixtures/nics.yml                         |    2 +-
 .../functional/managed_node_configuration_test.rb  |    9 ++++---
 3 files changed, 16 insertions(+), 21 deletions(-)

diff --git a/src/lib/managed_node_configuration.rb b/src/lib/managed_node_configuration.rb
index c412917..2a8d0b6 100644
--- a/src/lib/managed_node_configuration.rb
+++ b/src/lib/managed_node_configuration.rb
@@ -77,23 +77,15 @@ class ManagedNodeConfiguration
       result.puts "#{entry}|ONBOOT=yes"
 
       bonding.nics.each do |nic|
-        process_nic result, nic, macs, bonding
+        process_nic result, nic, macs, false, bonding
       end
     end
 
-    has_bridge = false
     host.nics.each do |nic|
-      # only process this nic if it doesn't have a bonding
-      # TODO remove the hack to force a bridge into the picture
       if nic.bondings.empty?
-        process_nic result, nic, macs, nil, false, true
-
-	# TODO remove this when bridges are properly supported
-	unless has_bridge
-	  macs[nic.mac] = "breth0"
-	  process_nic result, nic, macs, nil, true, false
-	  has_bridge = true
-	end
+        # process the nic twice: first to create the bridge, then to create the interface
+        process_nic result, nic, macs, true
+        process_nic result, nic, macs, false
       end
     end
 
@@ -102,11 +94,14 @@ class ManagedNodeConfiguration
 
   private
 
-  def self.process_nic(result, nic, macs, bonding = nil, is_bridge = false, bridged = true)
+  def self.process_nic(result, nic, macs, is_bridge, bonding=nil)
+    # get the interface name from the mac hash
+    # if no iterface name is found, then that means the NIC is no longer on the node
     iface_name = macs[nic.mac]
 
     if iface_name
-      entry = "ifcfg=#{nic.mac}|#{iface_name}"
+      entry = "ifcfg=#{nic.mac}|#{iface_name}" unless is_bridge
+      entry = "ifcfg=#{nic.mac}|br#{iface_name}" if is_bridge
 
       if bonding
         entry += "|MASTER=#{bonding.interface_name}|SLAVE=yes"
@@ -116,8 +111,7 @@ class ManagedNodeConfiguration
           ip = nic.ip_addresses[0]
           entry += "|IPADDR=#{ip.address}|NETMASK=#{ip.netmask}|BROADCAST=#{ip.broadcast}"
         end
-        entry += "|BRIDGE=#{nic.bridge}" if nic.bridge && !is_bridge
-        entry += "|BRIDGE=breth0" if !nic.bridge && !is_bridge
+        entry += "|BRIDGE=br#{iface_name}" unless is_bridge
         entry += "|TYPE=bridge" if is_bridge
       end
       entry += "|ONBOOT=yes"
diff --git a/src/test/fixtures/nics.yml b/src/test/fixtures/nics.yml
index b8bb6c7..cdf1c3e 100644
--- a/src/test/fixtures/nics.yml
+++ b/src/test/fixtures/nics.yml
@@ -23,7 +23,7 @@ ldapserver_nic_one:
   mac: 00:03:02:00:09:06
   usage_type: 1
   bandwidth: 100
-  bridge: breth0
+  bridge:
   host: ldapserver_managed_node
   physical_network: static_physical_network_one
 
diff --git a/src/test/functional/managed_node_configuration_test.rb b/src/test/functional/managed_node_configuration_test.rb
index 6ce4885..cf50308 100644
--- a/src/test/functional/managed_node_configuration_test.rb
+++ b/src/test/functional/managed_node_configuration_test.rb
@@ -48,8 +48,8 @@ class ManagedNodeConfigurationTest < Test::Unit::TestCase
 
     expected = <<-HERE
 # THIS FILE IS GENERATED!
-ifcfg=#{nic.mac}|eth0|BOOTPROTO=dhcp|BRIDGE=breth0|ONBOOT=yes
 ifcfg=#{nic.mac}|breth0|BOOTPROTO=dhcp|TYPE=bridge|ONBOOT=yes
+ifcfg=#{nic.mac}|eth0|BOOTPROTO=dhcp|BRIDGE=breth0|ONBOOT=yes
     HERE
 
     result = ManagedNodeConfiguration.generate(
@@ -67,8 +67,8 @@ ifcfg=#{nic.mac}|breth0|BOOTPROTO=dhcp|TYPE=bridge|ONBOOT=yes
 
     expected = <<-HERE
 # THIS FILE IS GENERATED!
-ifcfg=#{nic.mac}|eth0|BOOTPROTO=#{nic.physical_network.boot_type.proto}|IPADDR=#{nic.ip_addresses.first.address}|NETMASK=#{nic.ip_addresses.first.netmask}|BROADCAST=#{nic.ip_addresses.first.broadcast}|BRIDGE=#{nic.bridge}|ONBOOT=yes
 ifcfg=#{nic.mac}|breth0|BOOTPROTO=#{nic.physical_network.boot_type.proto}|IPADDR=#{nic.ip_addresses.first.address}|NETMASK=|BROADCAST=#{nic.ip_addresses.first.netmask}|TYPE=bridge|ONBOOT=yes
+ifcfg=#{nic.mac}|eth0|BOOTPROTO=#{nic.physical_network.boot_type.proto}|IPADDR=#{nic.ip_addresses.first.address}|NETMASK=#{nic.ip_addresses.first.netmask}|BROADCAST=#{nic.ip_addresses.first.broadcast}|BRIDGE=breth0|ONBOOT=yes
     HERE
 
     result = ManagedNodeConfiguration.generate(
@@ -87,9 +87,10 @@ ifcfg=#{nic.mac}|breth0|BOOTPROTO=#{nic.physical_network.boot_type.proto}|IPADDR
 
     expected = <<-HERE
 # THIS FILE IS GENERATED!
-ifcfg=#{nic1.mac}|eth0|BOOTPROTO=#{nic1.physical_network.boot_type.proto}|IPADDR=#{nic1.ip_addresses.first.address}|NETMASK=#{nic1.ip_addresses.first.netmask}|BROADCAST=#{nic1.ip_addresses.first.broadcast}|BRIDGE=breth0|ONBOOT=yes
 ifcfg=#{nic1.mac}|breth0|BOOTPROTO=#{nic1.physical_network.boot_type.proto}|IPADDR=#{nic1.ip_addresses.first.address}|NETMASK=#{nic1.ip_addresses.first.netmask}|BROADCAST=#{nic1.ip_addresses.first.broadcast}|TYPE=bridge|ONBOOT=yes
-ifcfg=#{nic2.mac}|eth1|BOOTPROTO=#{nic2.physical_network.boot_type.proto}|BRIDGE=breth0|ONBOOT=yes
+ifcfg=#{nic1.mac}|eth0|BOOTPROTO=#{nic1.physical_network.boot_type.proto}|IPADDR=#{nic1.ip_addresses.first.address}|NETMASK=#{nic1.ip_addresses.first.netmask}|BROADCAST=#{nic1.ip_addresses.first.broadcast}|BRIDGE=breth0|ONBOOT=yes
+ifcfg=#{nic2.mac}|breth1|BOOTPROTO=#{nic2.physical_network.boot_type.proto}|TYPE=bridge|ONBOOT=yes
+ifcfg=#{nic2.mac}|eth1|BOOTPROTO=#{nic2.physical_network.boot_type.proto}|BRIDGE=breth1|ONBOOT=yes
     HERE
 
     result = ManagedNodeConfiguration.generate(
-- 
1.6.0.6