[Ovirt-devel] [PATCH server] Fixes to the managed_node_configuration code.
Darryl L. Pierce
dpierce at redhat.com
Thu Apr 2 15:53:20 UTC 2009
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
More information about the ovirt-devel
mailing list