thr3ads.net - Ovirt devel - [Ovirt-devel] [PATCH server] Fixes to the managed_node

If this information is useful, please help other people find it:
Share via:

Darryl L. Pierce

2009-Mar-31 19:33 UTC

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

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

Darryl L. Pierce

2009-Mar-31 21:05 UTC

head link

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

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/lib/managed_node_configuration.rb              |   77 +++++++++-----------
 src/test/fixtures/nics.yml                         |    6 +-
 .../functional/managed_node_configuration_test.rb  |   15 +++--
 3 files changed, 48 insertions(+), 50 deletions(-)

diff --git a/src/lib/managed_node_configuration.rb
b/src/lib/managed_node_configuration.rb
index c412917..f864c46 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"
+      add_bridge(result,
+                 "none",
+                 bonding.interface_name,
+                 (bonding.ip_addresses.empty? ? "dhcp" :
"static"),
+                 (bonding.ip_addresses.empty? ? nil : bonding.ip_addresses[0]))
 
       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
+        iface_name = macs[nic.mac]
+        if iface_name
+          add_bridge(result,
+                     nic.mac,
+                     iface_name,
+                     nic.physical_network.boot_type.proto,
+                     nic.ip_addresses[0])
+          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)
+    entry = "ifcfg=#{mac}|br#{iface_name}|BOOTPROTO=#{bootproto}"
+    if ipaddress
+      entry +=
"|IPADDR=#{ipaddress.address}|NETMASK=#{ipaddress.netmask}|BROADCAST=#{ipaddress.broadcast}"
     end
+    entry += "|TYPE=bridge|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/nics.yml b/src/test/fixtures/nics.yml
index b8bb6c7..048d80c 100644
--- a/src/test/fixtures/nics.yml
+++ b/src/test/fixtures/nics.yml
@@ -10,7 +10,7 @@ mailserver_nic_two:
   usage_type: 1
   bandwidth: 100
   host: mailservers_managed_node
-  physical_network: dhcp_physical_network_one
+  physical_network: dhcp_physical_network_two
 
 fileserver_nic_one:
   mac: 00:99:00:99:13:07
@@ -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
 
@@ -53,4 +53,4 @@ mediaserver_nic_two:
   usage_type: 1
   bandwidth: 100
   host: mediaserver_managed_node
-  physical_network: dhcp_physical_network_one
+  physical_network: dhcp_physical_network_two
diff --git a/src/test/functional/managed_node_configuration_test.rb
b/src/test/functional/managed_node_configuration_test.rb
index 6ce4885..b6e591c 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|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|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|BRIDGE=breth0|ONBOOT=yes
+ifcfg=#{nic2.mac}|breth1|BOOTPROTO=#{nic2.physical_network.boot_type.proto}|TYPE=bridge|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=static|IPADDR=#{bonding.ip_addresses.first.address}|NETMASK=#{bonding.ip_addresses.first.netmask}|BROADCAST=#{bonding.ip_addresses.first.broadcast}|TYPE=bridge|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|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

Mohammed Morsi

2009-Apr-01 00:17 UTC

head link

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

Looks good for most part, save a few specifics, comments below.

Darryl L. Pierce wrote:> 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/lib/managed_node_configuration.rb              |   77
+++++++++-----------
>  src/test/fixtures/nics.yml                         |    6 +-
>  .../functional/managed_node_configuration_test.rb  |   15 +++--
>  3 files changed, 48 insertions(+), 50 deletions(-)
>
> diff --git a/src/lib/managed_node_configuration.rb
b/src/lib/managed_node_configuration.rb
> index c412917..f864c46 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"
> +      add_bridge(result,
> +                 "none",
> +                 bonding.interface_name,
> +                 (bonding.ip_addresses.empty? ? "dhcp" :
"static"),
> +                 (bonding.ip_addresses.empty? ? nil :
bonding.ip_addresses[0]))
>  
>   Are you sure bonding bridges don't need to have an assigned mac address?

Also instead of checking for bonding.ip_addresses.empty? shouldn't we
utilize bonding.vlan.nil?. Furthermore bonding.vlan.proto should be
employed similar to nic.physical_network.proto below

Furthermore I believe simply passing in / using the bonding ip address
for netmask / broadcast won't work as explained below.
>        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
> +        iface_name = macs[nic.mac]
> +        if iface_name
> +          add_bridge(result,
> +                     nic.mac,
> +                     iface_name,
> +                     nic.physical_network.boot_type.proto,
> +                     nic.ip_addresses[0])
>   I believe we should have the same ip_addresses.empty? check here as we
do w/ bondings as if a nic can be assigned to a dhcp network and have no
explicitly associated ip addresses.

Furthermore nic.physical_network should be checked to see if null, as a
nic doesn't have to be assigned to a network. (actually I believe one of
the reqs is that if it a nic isn't assigned to a network, it shouldn't
be included in the config returned to the node here, but we should check
w/ Hugh to make sure)

Also the same issue w/ passing in / using the nic ip address for netmask
/ broadcast won't work as explained below.
> +          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}"
>   While for static networks the ip associated w/ the nic / bonding will
contain the assigned ip address, it will not contain the netmask and the
broadcast. Rather this data is contained in the ip_address associated w/
the network which the nic / bonding is assigned to (physical net or
vlan). Essentially you configure the gateway / netmask / broadcast when
configuring the network and simply assign an ip address to the actual
device.

> -        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)
> +    entry =
"ifcfg=#{mac}|br#{iface_name}|BOOTPROTO=#{bootproto}"
> +    if ipaddress
> +      entry +=
"|IPADDR=#{ipaddress.address}|NETMASK=#{ipaddress.netmask}|BROADCAST=#{ipaddress.broadcast}"
>      end
> +    entry += "|TYPE=bridge|ONBOOT=yes"
>   We also need 'PEERDNS=no' associated w/ bridge configuration here.

> +    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/nics.yml b/src/test/fixtures/nics.yml
> index b8bb6c7..048d80c 100644
> --- a/src/test/fixtures/nics.yml
> +++ b/src/test/fixtures/nics.yml
> @@ -10,7 +10,7 @@ mailserver_nic_two:
>    usage_type: 1
>    bandwidth: 100
>    host: mailservers_managed_node
> -  physical_network: dhcp_physical_network_one
> +  physical_network: dhcp_physical_network_two
>  
>  fileserver_nic_one:
>    mac: 00:99:00:99:13:07
> @@ -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
>  
> @@ -53,4 +53,4 @@ mediaserver_nic_two:
>    usage_type: 1
>    bandwidth: 100
>    host: mediaserver_managed_node
> -  physical_network: dhcp_physical_network_one
> +  physical_network: dhcp_physical_network_two
> diff --git a/src/test/functional/managed_node_configuration_test.rb
b/src/test/functional/managed_node_configuration_test.rb
> index 6ce4885..b6e591c 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|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|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|BRIDGE=breth0|ONBOOT=yes
>
+ifcfg=#{nic2.mac}|breth1|BOOTPROTO=#{nic2.physical_network.boot_type.proto}|TYPE=bridge|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=static|IPADDR=#{bonding.ip_addresses.first.address}|NETMASK=#{bonding.ip_addresses.first.netmask}|BROADCAST=#{bonding.ip_addresses.first.broadcast}|TYPE=bridge|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|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
>       -Mo

Mohammed Morsi

2009-Apr-01 00:19 UTC

head link

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

Looks good for most part, save a few specifics, comments below.

Darryl L. Pierce wrote:> 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/lib/managed_node_configuration.rb              |   77
+++++++++-----------
>  src/test/fixtures/nics.yml                         |    6 +-
>  .../functional/managed_node_configuration_test.rb  |   15 +++--
>  3 files changed, 48 insertions(+), 50 deletions(-)
>
> diff --git a/src/lib/managed_node_configuration.rb
b/src/lib/managed_node_configuration.rb
> index c412917..f864c46 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"
> +      add_bridge(result,
> +                 "none",
> +                 bonding.interface_name,
> +                 (bonding.ip_addresses.empty? ? "dhcp" :
"static"),
> +                 (bonding.ip_addresses.empty? ? nil :
bonding.ip_addresses[0]))
>  
>   Are you sure bonding bridges don't need to have an assigned mac address?

Also instead of checking for bonding.ip_addresses.empty? shouldn't we
utilize bonding.vlan.nil?. Furthermore bonding.vlan.proto should be
employed similar to nic.physical_network.proto below

Furthermore I believe simply passing in / using the bonding ip address
for netmask / broadcast won't work as explained below.
>        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
> +        iface_name = macs[nic.mac]
> +        if iface_name
> +          add_bridge(result,
> +                     nic.mac,
> +                     iface_name,
> +                     nic.physical_network.boot_type.proto,
> +                     nic.ip_addresses[0])
>   I believe we should have the same ip_addresses.empty? check here as we
do w/ bondings as if a nic can be assigned to a dhcp network and have no
explicitly associated ip addresses.

Furthermore nic.physical_network should be checked to see if null, as a
nic doesn't have to be assigned to a network. (actually I believe one of
the reqs is that if it a nic isn't assigned to a network, it shouldn't
be included in the config returned to the node here, but we should check
w/ Hugh to make sure)

Also the same issue w/ passing in / using the nic ip address for netmask
/ broadcast won't work as explained below.
> +          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}"
>   While for static networks the ip associated w/ the nic / bonding will
contain the assigned ip address, it will not contain the netmask and the
broadcast. Rather this data is contained in the ip_address associated w/
the network which the nic / bonding is assigned to (physical net or
vlan). Essentially you configure the gateway / netmask / broadcast when
configuring the network and simply assign an ip address to the actual
device.

> -        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)
> +    entry =
"ifcfg=#{mac}|br#{iface_name}|BOOTPROTO=#{bootproto}"
> +    if ipaddress
> +      entry +=
"|IPADDR=#{ipaddress.address}|NETMASK=#{ipaddress.netmask}|BROADCAST=#{ipaddress.broadcast}"
>      end
> +    entry += "|TYPE=bridge|ONBOOT=yes"
>   We also need 'PEERDNS=no' associated w/ bridge configuration here.

> +    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/nics.yml b/src/test/fixtures/nics.yml
> index b8bb6c7..048d80c 100644
> --- a/src/test/fixtures/nics.yml
> +++ b/src/test/fixtures/nics.yml
> @@ -10,7 +10,7 @@ mailserver_nic_two:
>    usage_type: 1
>    bandwidth: 100
>    host: mailservers_managed_node
> -  physical_network: dhcp_physical_network_one
> +  physical_network: dhcp_physical_network_two
>  
>  fileserver_nic_one:
>    mac: 00:99:00:99:13:07
> @@ -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
>  
> @@ -53,4 +53,4 @@ mediaserver_nic_two:
>    usage_type: 1
>    bandwidth: 100
>    host: mediaserver_managed_node
> -  physical_network: dhcp_physical_network_one
> +  physical_network: dhcp_physical_network_two
> diff --git a/src/test/functional/managed_node_configuration_test.rb
b/src/test/functional/managed_node_configuration_test.rb
> index 6ce4885..b6e591c 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|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|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|BRIDGE=breth0|ONBOOT=yes
>
+ifcfg=#{nic2.mac}|breth1|BOOTPROTO=#{nic2.physical_network.boot_type.proto}|TYPE=bridge|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=static|IPADDR=#{bonding.ip_addresses.first.address}|NETMASK=#{bonding.ip_addresses.first.netmask}|BROADCAST=#{bonding.ip_addresses.first.broadcast}|TYPE=bridge|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|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
>       -Mo

Darryl L. Pierce

2009-Apr-01 15:35 UTC

head link

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

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/lib/managed_node_configuration.rb              |   77 +++++++++-----------
 src/test/fixtures/ip_addresses.yml                 |    9 +++
 src/test/fixtures/nics.yml                         |    4 +-
 .../functional/managed_node_configuration_test.rb  |   21 +++---
 4 files changed, 59 insertions(+), 52 deletions(-)

diff --git a/src/lib/managed_node_configuration.rb
b/src/lib/managed_node_configuration.rb
index c412917..61ee09e 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
+        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

Darryl L. Pierce

2009-Apr-01 19:52 UTC

head link

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

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

Darryl L. Pierce

2009-Apr-02 13:50 UTC

head link

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

Added public APIs to Bonding and Nic to retrieve networking details for
the respective interface.

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     |    6 +-
 src/app/models/bonding.rb                          |   40 +++++++++
 src/app/models/nic.rb                              |   39 +++++++++
 src/lib/managed_node_configuration.rb              |   86 +++++++++----------
 src/test/fixtures/ip_addresses.yml                 |    9 ++
 src/test/fixtures/nics.yml                         |    4 +-
 .../functional/managed_node_configuration_test.rb  |   21 +++--
 7 files changed, 146 insertions(+), 59 deletions(-)

diff --git a/src/app/controllers/managed_node_controller.rb
b/src/app/controllers/managed_node_controller.rb
index f5948be..0922d92 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
@@ -54,10 +54,10 @@ class ManagedNodeController < ApplicationController
     if mac_text != nil
       mac_text.scan(/([^,]+)\,?/).each do |line|
         key, value = line.first.split("=")
-        @macs[key] = value
+        @macs[key] = value.upcase
       end
     end
 
-    render :nothing => true, :status => :error if @macs.empty?
+    render :nothing => true if @macs.empty?
   end
 end
diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
index 32b9a37..d87f3a0 100644
--- a/src/app/models/bonding.rb
+++ b/src/app/models/bonding.rb
@@ -72,6 +72,46 @@ class Bonding < ActiveRecord::Base
      :greater_than_or_equal_to => 0,
      :unless => Proc.new { |bonding| bonding.arp_interval.nil? }
 
+  # Returns whether networking is defined for this interface.
+  def networking?
+    # there is network if there's a vlan defined.
+    (vlan.nil? == false)
+  end
+
+  # Returns the boot protocol for the interface, or +nil+ if networking
+  # is not defined.
+  def boot_protocol
+    return vlan.boot_type.proto if networking?
+    return nil
+  end
+
+  # Returns the ip address assigned to this bonded interface, or +nil+
+  # if no networking is defined.
+  def ip_address
+    return ip_addresses.first.address unless ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the netmask assigned to this bonded interface, or +nil+
+  # if no networking is defined.
+  def netmask
+    return vlan.ip_addresses.first.netmask if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the broadcast address assigned to this bonded interface,
+  # or +nil if no networking is defined.
+  def broadcast
+    return vlan.ip_addresses.first.broadcast if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the gateway address for the bonded interface if networking is
defined.
+  def gateway
+    return vlan.ip_addresses.first.gateway if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
  protected
   def validate
     if ! vlan.nil? and
diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
index e26c110..8e19c26 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -43,6 +43,45 @@ class Nic < ActiveRecord::Base
      :scope => :host_id,
      :unless => Proc.new { |nic| nic.physical_network_id.nil? }
 
+  # Returns whether the nic has networking defined.
+  def networking?
+    (physical_network != nil)
+  end
+
+  # Returns the boot protocol for the nic if networking is defined.
+  def boot_protocol
+    return physical_network.boot_type.proto if networking?
+  end
+
+  # Returns whether the nic is enslaved by a bonded interface.
+  def bonded?
+    !bondings.empty?
+  end
+
+  # Returns the ip address for the nic if networking is defined.
+  def ip_address
+    return ip_addresses.first.address if networking? &&
!ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the netmask for the nic if networking is defined.
+  def netmask
+    return physical_network.ip_addresses.first.netmask if networking?
&& !ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the broadcast address for the nic if networking is defined.
+  def broadcast
+    return physical_network.ip_addresses.first.broadcast if networking?
&& !ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the gateway address fo rthe nic if networking is defined.
+  def gateway
+    return physical_network.ip_addresses.first.gateway if networking?
&& !ip_addresses.empty?
+    return nil
+  end
+
   # validate 'bridge' or 'usage_type' attribute ?
 
   protected
diff --git a/src/lib/managed_node_configuration.rb
b/src/lib/managed_node_configuration.rb
index c412917..028573b 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
-
-      result.puts "#{entry}|ONBOOT=yes"
-
-      bonding.nics.each do |nic|
-        process_nic result, nic, macs, bonding
+      if bonding.networking?
+        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
+
+        add_bridge(result,"none",bonding.interface_name,
+                   bonding.boot_protocol, bonding.ip_address,
+                   bonding.netmask, bonding.broadcast,
+                   bonding.gateway)
+
+        bonding.nics.each do |nic|
+          iface_name = macs[nic.mac]
+          if iface_name
+            add_slave(result, nic.mac, iface_name, bonding.interface_name)
+          end
+        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.networking? && !nic.bonded?
+        iface_name = macs[nic.mac]
+        if iface_name
+          add_bridge(result, nic.mac, iface_name,
+                     nic.boot_protocol, nic.ip_address,
+                     nic.netmask, nic.broadcast,
+                     nic.gateway)
+          add_nic(result, nic.mac, iface_name)
+        end
       end
     end
 
@@ -102,27 +104,21 @@ 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, gateway)
+    entry = "ifcfg=#{mac}|br#{iface_name}|BOOTPROTO=#{bootproto}"
+    if bootproto == "static"
+      entry +=
"|IPADDR=#{ipaddress}|NETMASK=#{netmask}|BROADCAST=#{broadcast}|GATEWAY=#{gateway}"
     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..b66705e 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=#{nic.boot_protocol}|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.boot_protocol}|IPADDR=#{nic.ip_address}|NETMASK=#{nic.netmask}|BROADCAST=#{nic.broadcast}|GATEWAY=#{nic.gateway}|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.boot_protocol}|IPADDR=#{nic1.ip_address}|NETMASK=#{nic1.netmask}|BROADCAST=#{nic1.broadcast}|GATEWAY=#{nic1.gateway}|TYPE=bridge|PEERDNS=no|ONBOOT=yes
+ifcfg=#{nic1.mac}|eth0|BRIDGE=breth0|ONBOOT=yes
+ifcfg=#{nic2.mac}|breth1|BOOTPROTO=#{nic2.boot_protocol}|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=#{bonding.boot_protocol}|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

Darryl L. Pierce

2009-Apr-02 15:53 UTC

head link

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

Added public APIs to Bonding and Nic to retrieve networking details for
the respective interface.

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.

If a hostname is unknown, or it does not send up any macs, then it is
returned an empty configuration payload and not an error status code.

Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
 src/app/controllers/managed_node_controller.rb     |    6 +-
 src/app/models/bonding.rb                          |   40 ++++++++++
 src/app/models/nic.rb                              |   39 ++++++++++
 src/lib/managed_node_configuration.rb              |   80 +++++++++----------
 src/test/fixtures/ip_addresses.yml                 |    9 ++
 src/test/fixtures/nics.yml                         |    4 +-
 .../functional/managed_node_configuration_test.rb  |   21 +++--
 .../functional/managed_node_controller_test.rb     |   21 -----
 8 files changed, 143 insertions(+), 77 deletions(-)

diff --git a/src/app/controllers/managed_node_controller.rb
b/src/app/controllers/managed_node_controller.rb
index f5948be..062b3d4 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
@@ -54,10 +54,10 @@ class ManagedNodeController < ApplicationController
     if mac_text != nil
       mac_text.scan(/([^,]+)\,?/).each do |line|
         key, value = line.first.split("=")
-        @macs[key] = value
+        @macs[key.upcase] = value
       end
     end
 
-    render :nothing => true, :status => :error if @macs.empty?
+    render :nothing => true if @macs.empty?
   end
 end
diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
index 32b9a37..2dd84fa 100644
--- a/src/app/models/bonding.rb
+++ b/src/app/models/bonding.rb
@@ -72,6 +72,46 @@ class Bonding < ActiveRecord::Base
      :greater_than_or_equal_to => 0,
      :unless => Proc.new { |bonding| bonding.arp_interval.nil? }
 
+  # Returns whether networking is defined for this interface.
+  def networking?
+    # there is network if there's a vlan defined.
+    (vlan.nil? == false)
+  end
+
+  # Returns the boot protocol for the interface, or +nil+ if networking
+  # is not defined.
+  def boot_protocol
+    return vlan.boot_type.proto if networking?
+    return nil
+  end
+
+  # Returns the ip address assigned to this bonded interface, or +nil+
+  # if no networking is defined.
+  def ip_address
+    return ip_addresses.first.address if networking? &&
!ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the netmask assigned to this bonded interface, or +nil+
+  # if no networking is defined.
+  def netmask
+    return vlan.ip_addresses.first.netmask if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the broadcast address assigned to this bonded interface,
+  # or +nil if no networking is defined.
+  def broadcast
+    return vlan.ip_addresses.first.broadcast if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the gateway address for the bonded interface if networking is
defined.
+  def gateway
+    return vlan.ip_addresses.first.gateway if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
  protected
   def validate
     if ! vlan.nil? and
diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
index e26c110..8e19c26 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -43,6 +43,45 @@ class Nic < ActiveRecord::Base
      :scope => :host_id,
      :unless => Proc.new { |nic| nic.physical_network_id.nil? }
 
+  # Returns whether the nic has networking defined.
+  def networking?
+    (physical_network != nil)
+  end
+
+  # Returns the boot protocol for the nic if networking is defined.
+  def boot_protocol
+    return physical_network.boot_type.proto if networking?
+  end
+
+  # Returns whether the nic is enslaved by a bonded interface.
+  def bonded?
+    !bondings.empty?
+  end
+
+  # Returns the ip address for the nic if networking is defined.
+  def ip_address
+    return ip_addresses.first.address if networking? &&
!ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the netmask for the nic if networking is defined.
+  def netmask
+    return physical_network.ip_addresses.first.netmask if networking?
&& !ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the broadcast address for the nic if networking is defined.
+  def broadcast
+    return physical_network.ip_addresses.first.broadcast if networking?
&& !ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the gateway address fo rthe nic if networking is defined.
+  def gateway
+    return physical_network.ip_addresses.first.gateway if networking?
&& !ip_addresses.empty?
+    return nil
+  end
+
   # validate 'bridge' or 'usage_type' attribute ?
 
   protected
diff --git a/src/lib/managed_node_configuration.rb
b/src/lib/managed_node_configuration.rb
index c412917..889869a 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}"
+      if bonding.networking?
+        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
+
+        add_bridge(result,"none",bonding.interface_name,
+                   bonding.boot_protocol, bonding.ip_address,
+                   bonding.netmask, bonding.broadcast,
+                   bonding.gateway)
       end
 
-      result.puts "#{entry}|ONBOOT=yes"
-
       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.networking? && !nic.bonded?
+        iface_name = macs[nic.mac]
+        if iface_name
+          add_bridge(result, nic.mac, iface_name,
+                     nic.boot_protocol, nic.ip_address,
+                     nic.netmask, nic.broadcast,
+                     nic.gateway)
+          add_nic(result, nic.mac, iface_name)
+        end
       end
     end
 
@@ -102,27 +104,21 @@ 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, gateway)
+    entry = "ifcfg=#{mac}|br#{iface_name}|BOOTPROTO=#{bootproto}"
+    if bootproto == "static"
+      entry +=
"|IPADDR=#{ipaddress}|NETMASK=#{netmask}|BROADCAST=#{broadcast}|GATEWAY=#{gateway}"
     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..b66705e 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=#{nic.boot_protocol}|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.boot_protocol}|IPADDR=#{nic.ip_address}|NETMASK=#{nic.netmask}|BROADCAST=#{nic.broadcast}|GATEWAY=#{nic.gateway}|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.boot_protocol}|IPADDR=#{nic1.ip_address}|NETMASK=#{nic1.netmask}|BROADCAST=#{nic1.broadcast}|GATEWAY=#{nic1.gateway}|TYPE=bridge|PEERDNS=no|ONBOOT=yes
+ifcfg=#{nic1.mac}|eth0|BRIDGE=breth0|ONBOOT=yes
+ifcfg=#{nic2.mac}|breth1|BOOTPROTO=#{nic2.boot_protocol}|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=#{bonding.boot_protocol}|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
diff --git a/src/test/functional/managed_node_controller_test.rb
b/src/test/functional/managed_node_controller_test.rb
index a6d2667..dd93e80 100644
--- a/src/test/functional/managed_node_controller_test.rb
+++ b/src/test/functional/managed_node_controller_test.rb
@@ -26,27 +26,6 @@ class ManagedNodeControllerTest <
ActionController::TestCase
   fixtures :hosts
   fixtures :nics
 
-  # Ensures that the request fails if it's missing a hostname, or if the
-  # hostname is invalid.
-  #
-  def test_config_with_invalid_hostname
-    get :config
-
-    assert_response :error
-
-    get :config, {:host => 'invalid.prod.com'}
-
-    assert_response :error
-  end
-
-  # Ensures the request fails if no mac addresses are supplied.
-  #
-  def test_config_without_macs
-    get :config, {:host => hosts(:mailservers_managed_node).hostname}
-
-    assert_response :error
-  end
-
   # Ensures the request succeeds if it is well-formed.
   #
   def test_config
-- 
1.6.0.6

Darryl L. Pierce

2009-Apr-02 16:15 UTC

head link

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

NOTE: with feedback from mmorsi dated 02 April @ 12:12:47 -0400

Added public APIs to Bonding and Nic to retrieve networking details for
the respective interface.

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.

If a hostname is unknown, or it does not send up any macs, then it is
returned an empty configuration payload and not an error status code.

Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
---
 src/app/controllers/managed_node_controller.rb     |    6 +-
 src/app/models/bonding.rb                          |   40 ++++++++++
 src/app/models/nic.rb                              |   39 ++++++++++
 src/lib/managed_node_configuration.rb              |   78 +++++++++----------
 src/test/fixtures/ip_addresses.yml                 |    9 ++
 src/test/fixtures/nics.yml                         |    4 +-
 .../functional/managed_node_configuration_test.rb  |   21 +++--
 .../functional/managed_node_controller_test.rb     |   21 -----
 8 files changed, 142 insertions(+), 76 deletions(-)

diff --git a/src/app/controllers/managed_node_controller.rb
b/src/app/controllers/managed_node_controller.rb
index f5948be..062b3d4 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
@@ -54,10 +54,10 @@ class ManagedNodeController < ApplicationController
     if mac_text != nil
       mac_text.scan(/([^,]+)\,?/).each do |line|
         key, value = line.first.split("=")
-        @macs[key] = value
+        @macs[key.upcase] = value
       end
     end
 
-    render :nothing => true, :status => :error if @macs.empty?
+    render :nothing => true if @macs.empty?
   end
 end
diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
index 32b9a37..2dd84fa 100644
--- a/src/app/models/bonding.rb
+++ b/src/app/models/bonding.rb
@@ -72,6 +72,46 @@ class Bonding < ActiveRecord::Base
      :greater_than_or_equal_to => 0,
      :unless => Proc.new { |bonding| bonding.arp_interval.nil? }
 
+  # Returns whether networking is defined for this interface.
+  def networking?
+    # there is network if there's a vlan defined.
+    (vlan.nil? == false)
+  end
+
+  # Returns the boot protocol for the interface, or +nil+ if networking
+  # is not defined.
+  def boot_protocol
+    return vlan.boot_type.proto if networking?
+    return nil
+  end
+
+  # Returns the ip address assigned to this bonded interface, or +nil+
+  # if no networking is defined.
+  def ip_address
+    return ip_addresses.first.address if networking? &&
!ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the netmask assigned to this bonded interface, or +nil+
+  # if no networking is defined.
+  def netmask
+    return vlan.ip_addresses.first.netmask if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the broadcast address assigned to this bonded interface,
+  # or +nil if no networking is defined.
+  def broadcast
+    return vlan.ip_addresses.first.broadcast if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the gateway address for the bonded interface if networking is
defined.
+  def gateway
+    return vlan.ip_addresses.first.gateway if networking? &&
!vlan.ip_addresses.empty?
+    return nil
+  end
+
  protected
   def validate
     if ! vlan.nil? and
diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
index e26c110..e8b7768 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -43,6 +43,45 @@ class Nic < ActiveRecord::Base
      :scope => :host_id,
      :unless => Proc.new { |nic| nic.physical_network_id.nil? }
 
+  # Returns whether the nic has networking defined.
+  def networking?
+    (physical_network != nil)
+  end
+
+  # Returns the boot protocol for the nic if networking is defined.
+  def boot_protocol
+    return physical_network.boot_type.proto if networking?
+  end
+
+  # Returns whether the nic is enslaved by a bonded interface.
+  def bonded?
+    !bondings.empty?
+  end
+
+  # Returns the ip address for the nic if networking is defined.
+  def ip_address
+    return ip_addresses.first.address if networking? &&
!ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the netmask for the nic if networking is defined.
+  def netmask
+    return physical_network.ip_addresses.first.netmask if networking?
&& !physical_network.ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the broadcast address for the nic if networking is defined.
+  def broadcast
+    return physical_network.ip_addresses.first.broadcast if networking?
&& !physical_network.ip_addresses.empty?
+    return nil
+  end
+
+  # Returns the gateway address fo rthe nic if networking is defined.
+  def gateway
+    return physical_network.ip_addresses.first.gateway if networking?
&& !physical_network.ip_addresses.empty?
+    return nil
+  end
+
   # validate 'bridge' or 'usage_type' attribute ?
 
   protected
diff --git a/src/lib/managed_node_configuration.rb
b/src/lib/managed_node_configuration.rb
index c412917..f93bccd 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\""
+      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
 
-      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}"
+      if bonding.networking?
+        add_bridge(result,"none",bonding.interface_name,
+                   bonding.boot_protocol, bonding.ip_address,
+                   bonding.netmask, bonding.broadcast,
+                   bonding.gateway)
       end
 
-      result.puts "#{entry}|ONBOOT=yes"
-
       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.networking? && !nic.bonded?
+        iface_name = macs[nic.mac]
+        if iface_name
+          add_bridge(result, nic.mac, iface_name,
+                     nic.boot_protocol, nic.ip_address,
+                     nic.netmask, nic.broadcast,
+                     nic.gateway)
+          add_nic(result, nic.mac, iface_name)
+        end
       end
     end
 
@@ -102,27 +104,21 @@ 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, gateway)
+    entry = "ifcfg=#{mac}|br#{iface_name}|BOOTPROTO=#{bootproto}"
+    if bootproto == "static"
+      entry +=
"|IPADDR=#{ipaddress}|NETMASK=#{netmask}|BROADCAST=#{broadcast}|GATEWAY=#{gateway}"
     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..b66705e 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=#{nic.boot_protocol}|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.boot_protocol}|IPADDR=#{nic.ip_address}|NETMASK=#{nic.netmask}|BROADCAST=#{nic.broadcast}|GATEWAY=#{nic.gateway}|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.boot_protocol}|IPADDR=#{nic1.ip_address}|NETMASK=#{nic1.netmask}|BROADCAST=#{nic1.broadcast}|GATEWAY=#{nic1.gateway}|TYPE=bridge|PEERDNS=no|ONBOOT=yes
+ifcfg=#{nic1.mac}|eth0|BRIDGE=breth0|ONBOOT=yes
+ifcfg=#{nic2.mac}|breth1|BOOTPROTO=#{nic2.boot_protocol}|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=#{bonding.boot_protocol}|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
diff --git a/src/test/functional/managed_node_controller_test.rb
b/src/test/functional/managed_node_controller_test.rb
index a6d2667..dd93e80 100644
--- a/src/test/functional/managed_node_controller_test.rb
+++ b/src/test/functional/managed_node_controller_test.rb
@@ -26,27 +26,6 @@ class ManagedNodeControllerTest <
ActionController::TestCase
   fixtures :hosts
   fixtures :nics
 
-  # Ensures that the request fails if it's missing a hostname, or if the
-  # hostname is invalid.
-  #
-  def test_config_with_invalid_hostname
-    get :config
-
-    assert_response :error
-
-    get :config, {:host => 'invalid.prod.com'}
-
-    assert_response :error
-  end
-
-  # Ensures the request fails if no mac addresses are supplied.
-  #
-  def test_config_without_macs
-    get :config, {:host => hosts(:mailservers_managed_node).hostname}
-
-    assert_response :error
-  end
-
   # Ensures the request succeeds if it is well-formed.
   #
   def test_config
-- 
1.6.0.6

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

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

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

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

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

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

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

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

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

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