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

Darryl L. Pierce dpierce at redhat.com
Wed Apr 1 19:52:34 UTC 2009


Refactored how interfaces, bonded interfaces and bridges are defined.

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.

When a bonded interface is created, a bridge is created for it as well.

Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
 src/app/controllers/managed_node_controller.rb     |    4 +-
 src/lib/managed_node_configuration.rb              |   79 +++++++++----------
 src/test/fixtures/ip_addresses.yml                 |    9 ++
 src/test/fixtures/nics.yml                         |    4 +-
 .../functional/managed_node_configuration_test.rb  |   21 +++--
 5 files changed, 62 insertions(+), 55 deletions(-)

diff --git a/src/app/controllers/managed_node_controller.rb b/src/app/controllers/managed_node_controller.rb
index f5948be..29e4a4f 100644
--- a/src/app/controllers/managed_node_controller.rb
+++ b/src/app/controllers/managed_node_controller.rb
@@ -44,7 +44,7 @@ class ManagedNodeController < ApplicationController
   def load_host
     @host = Host.find_by_hostname(params[:host])
 
-    render :nothing => true, :status => :error unless @host
+    render :nothing => true unless @host
   end
 
   def load_macs
@@ -58,6 +58,6 @@ class ManagedNodeController < ApplicationController
       end
     end
 
-    render :nothing => true, :status => :error if @macs.empty?
+    render :nothing => true if @macs.empty?
   end
 end
diff --git a/src/lib/managed_node_configuration.rb b/src/lib/managed_node_configuration.rb
index c412917..054836f 100644
--- a/src/lib/managed_node_configuration.rb
+++ b/src/lib/managed_node_configuration.rb
@@ -65,35 +65,37 @@ class ManagedNodeConfiguration
     # now process the network interfaces  and bondings
 
     host.bondings.each do |bonding|
-      entry = "ifcfg=none|#{bonding.interface_name}|BONDING_OPTS=\"mode=#{bonding.bonding_type.mode} miimon=100\""
-
-      if bonding.ip_addresses.empty?
-        entry += "|BOOTPROTO=dhcp"
-      else
-        ip = bonding.ip_addresses[0]
-        entry += "|BOOTPROTO=static|IPADDR=#{ip.address}|NETMASK=#{ip.netmask}|BROADCAST=#{ip.broadcast}"
-      end
+      entry  = "ifcfg=none|#{bonding.interface_name}"
+      entry += "|BONDING_OPTS=\"mode=#{bonding.bonding_type.mode} miimon=100\""
+      entry += "|BRIDGE=br#{bonding.interface_name}"
+      entry += "|ONBOOT=yes"
+      result.puts entry
 
-      result.puts "#{entry}|ONBOOT=yes"
+      ipaddress=(bonding.ip_addresses.empty?      ? nil : bonding.ip_addresses.first.address)
+      netmask  =(bonding.vlan.ip_addresses.empty? ? nil : bonding.vlan.ip_addresses.first.netmask)
+      broadcast=(bonding.vlan.ip_addresses.empty? ? nil : bonding.vlan.ip_addresses.first.broadcast)
+      add_bridge(result,"none",bonding.interface_name,
+                 bonding.vlan.boot_type.proto, ipaddress, netmask, broadcast)
 
       bonding.nics.each do |nic|
-        process_nic result, nic, macs, bonding
+        iface_name = macs[nic.mac]
+        if iface_name
+          add_slave(result, nic.mac, iface_name, bonding.interface_name)
+        end
       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
+      if nic.bondings.empty? && nic.physical_network
+        iface_name = macs[nic.mac]
+        ipaddress=(nic.physical_network.ip_addresses.empty? ? nil : nic.physical_network.ip_addresses.first.address)
+        netmask  =(nic.ip_addresses.empty?                  ? nil : nic.ip_addresses.first.netmask)
+        broadcast=(nic.ip_addresses.empty?                  ? nil : nic.ip_addresses.first.broadcast)
+        if iface_name
+          add_bridge(result,nic.mac,iface_name,
+                     nic.physical_network.boot_type.proto,ipaddress,netmask,broadcast)
+          add_nic(result, nic.mac, iface_name)
+        end
       end
     end
 
@@ -102,27 +104,20 @@ class ManagedNodeConfiguration
 
   private
 
-  def self.process_nic(result, nic, macs, bonding = nil, is_bridge = false, bridged = true)
-    iface_name = macs[nic.mac]
-
-    if iface_name
-      entry = "ifcfg=#{nic.mac}|#{iface_name}"
-
-      if bonding
-        entry += "|MASTER=#{bonding.interface_name}|SLAVE=yes"
-      else
-        entry += "|BOOTPROTO=#{nic.physical_network.boot_type.proto}"
-        if nic.physical_network.boot_type.proto == 'static'
-          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 += "|TYPE=bridge" if is_bridge
-      end
-      entry += "|ONBOOT=yes"
+  def self.add_bridge(result, mac, iface_name, bootproto, ipaddress, netmask, broadcast)
+    entry = "ifcfg=#{mac}|br#{iface_name}|BOOTPROTO=#{bootproto}"
+    if bootproto == "static"
+      entry += "|IPADDR=#{ipaddress}|NETMASK=#{netmask}|BROADCAST=#{broadcast}"
     end
+    entry += "|TYPE=bridge|PEERDNS=no|ONBOOT=yes"
+    result.puts entry
+  end
+
+  def self.add_nic(result, mac, iface_name)
+    result.puts "ifcfg=#{mac}|#{iface_name}|BRIDGE=br#{iface_name}|ONBOOT=yes"
+  end
 
-    result.puts entry if defined? entry
+  def self.add_slave(result, mac, iface_name, master)
+    result.puts "ifcfg=#{mac}|#{iface_name}|MASTER=#{master}|SLAVE=yes|ONBOOT=yes"
   end
 end
diff --git a/src/test/fixtures/ip_addresses.yml b/src/test/fixtures/ip_addresses.yml
index b19e9e1..bca0f11 100644
--- a/src/test/fixtures/ip_addresses.yml
+++ b/src/test/fixtures/ip_addresses.yml
@@ -10,6 +10,15 @@ ip_v4_ldapserver_nic_one:
   nic: ldapserver_nic_one
   type: IpV4Address
   address: 172.31.0.25
+  netmask: 255.255.255.0
+  broadcast: 172.31.0.255
+  gateway: 172.31.0.1
+
+ip_v4_ldapserver_physical_nic_one:
+  network: static_physical_network_one
+  address: 172.31.0.26
+  netmask: 255.255.255.0
+  broadcast: 172.31.0.255
   gateway: 172.31.0.1
 
 ip_v4_buildserver_nic_one:
diff --git a/src/test/fixtures/nics.yml b/src/test/fixtures/nics.yml
index 1cf3223..97397cd 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
 
@@ -39,7 +39,7 @@ buildserver_nic_two:
   usage_type: 1
   bandwidth: 100
   host: buildserver_managed_node
-  physical_network: static_physical_network_two
+  physical_network: static_physical_network_one
 
 mediaserver_nic_one:
   mac: 07:17:19:65:03:32
diff --git a/src/test/functional/managed_node_configuration_test.rb b/src/test/functional/managed_node_configuration_test.rb
index 6ce4885..2393227 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}|breth0|BOOTPROTO=dhcp|TYPE=bridge|PEERDNS=no|ONBOOT=yes
+ifcfg=#{nic.mac}|eth0|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}|breth0|BOOTPROTO=#{nic.physical_network.boot_type.proto}|IPADDR=#{nic.physical_network.ip_addresses.first.address}|NETMASK=#{nic.ip_addresses.first.netmask}|BROADCAST=#{nic.ip_addresses.first.broadcast}|TYPE=bridge|PEERDNS=no|ONBOOT=yes
+ifcfg=#{nic.mac}|eth0|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}|breth0|BOOTPROTO=#{nic1.physical_network.boot_type.proto}|IPADDR=#{nic1.physical_network.ip_addresses.first.address}|NETMASK=#{nic1.ip_addresses.first.netmask}|BROADCAST=#{nic1.ip_addresses.first.broadcast}|TYPE=bridge|PEERDNS=no|ONBOOT=yes
+ifcfg=#{nic1.mac}|eth0|BRIDGE=breth0|ONBOOT=yes
+ifcfg=#{nic2.mac}|breth1|BOOTPROTO=#{nic2.physical_network.boot_type.proto}|TYPE=bridge|PEERDNS=no|ONBOOT=yes
+ifcfg=#{nic2.mac}|eth1|BRIDGE=breth1|ONBOOT=yes
     HERE
 
     result = ManagedNodeConfiguration.generate(
@@ -114,7 +115,8 @@ ifcfg=#{nic2.mac}|eth1|BOOTPROTO=#{nic2.physical_network.boot_type.proto}|BRIDGE
     expected = <<-HERE
 # THIS FILE IS GENERATED!
 bonding=#{bonding.interface_name}
-ifcfg=none|#{bonding.interface_name}|BONDING_OPTS="mode=#{bonding.bonding_type.mode} miimon=100"|BOOTPROTO=static|IPADDR=#{bonding.ip_addresses.first.address}|NETMASK=#{bonding.ip_addresses.first.netmask}|BROADCAST=#{bonding.ip_addresses.first.broadcast}|ONBOOT=yes
+ifcfg=none|#{bonding.interface_name}|BONDING_OPTS="mode=#{bonding.bonding_type.mode} miimon=100"|BRIDGE=br#{bonding.interface_name}|ONBOOT=yes
+ifcfg=none|br#{bonding.interface_name}|BOOTPROTO=dhcp|TYPE=bridge|PEERDNS=no|ONBOOT=yes
 ifcfg=#{nic1.mac}|eth0|MASTER=#{bonding.interface_name}|SLAVE=yes|ONBOOT=yes
 ifcfg=#{nic2.mac}|eth1|MASTER=#{bonding.interface_name}|SLAVE=yes|ONBOOT=yes
 HERE
@@ -140,7 +142,8 @@ HERE
     expected = <<-HERE
 # THIS FILE IS GENERATED!
 bonding=#{bonding.interface_name}
-ifcfg=none|#{bonding.interface_name}|BONDING_OPTS="mode=#{bonding.bonding_type.mode} miimon=100"|BOOTPROTO=dhcp|ONBOOT=yes
+ifcfg=none|#{bonding.interface_name}|BONDING_OPTS="mode=#{bonding.bonding_type.mode} miimon=100"|BRIDGE=br#{bonding.interface_name}|ONBOOT=yes
+ifcfg=none|br#{bonding.interface_name}|BOOTPROTO=dhcp|TYPE=bridge|PEERDNS=no|ONBOOT=yes
 ifcfg=#{nic1.mac}|eth0|MASTER=#{bonding.interface_name}|SLAVE=yes|ONBOOT=yes
 ifcfg=#{nic2.mac}|eth1|MASTER=#{bonding.interface_name}|SLAVE=yes|ONBOOT=yes
 HERE
-- 
1.6.0.6




More information about the ovirt-devel mailing list