[Ovirt-devel] [PATCH server] Introduces a new IP address model. Moves IP addressing to a separate

Mohammed Morsi mmorsi at redhat.com
Tue Oct 21 20:54:30 UTC 2008


NACK. Applies nicely (albeit with whitespace complaints) and everything
seems to work with a few caveats as described below / inline.

Darryl L. Pierce wrote:
> You will need to run a migration after this patch to create the
> ip_addresses table and massage the bondings and nics tables to
> reference it.
>
> NEW CLASSES:
>
> IpAddress:   base class for addresses
> IpV4Address: represents an IPv4 address
> IpV6Address: represents an IPv6 address
>
> Signed-off-by: Darryl L. Pierce <dpierce at redhat.com>
> ---
>  src/app/models/bonding.rb                          |    1 +
>  src/app/models/ip_address.rb                       |   25 +++++
>  src/app/models/ip_v4_address.rb                    |   46 +++++++++
>  src/app/models/ip_v6_address.rb                    |   40 ++++++++
>  src/app/models/nic.rb                              |    2 +
>  src/db/migrate/025_create_ip_addresses.rb          |   46 +++++++++
>   
It seems since you've sent this out, migration 025 has been committed,
so you'll need to start at 026

>  ...026_move_nic_addresses_to_ip_addresses_table.rb |   56 +++++++++++
>  ...7_move_bonding_address_to_ip_addresses_table.rb |   54 +++++++++++
>  src/host-browser/host-browser.rb                   |   17 ++--
>  src/lib/managed_node_configuration.rb              |    8 +-
>  src/test/fixtures/bondings.yml                     |    4 +-
>  src/test/fixtures/ip_addresses.yml                 |   55 +++++++++++
>  src/test/fixtures/nics.yml                         |   82 ++++++++--------
>  .../functional/managed_node_configuration_test.rb  |   26 +++---
>  src/test/unit/ip_address_test.rb                   |    8 ++
>  src/test/unit/ip_v4_address_test.rb                |   99 ++++++++++++++++++++
>  src/test/unit/ip_v6_address_test.rb                |   82 ++++++++++++++++
>  src/test/unit/nic_test.rb                          |    1 +
>  18 files changed, 584 insertions(+), 68 deletions(-)
>  create mode 100644 src/app/models/ip_address.rb
>  create mode 100644 src/app/models/ip_v4_address.rb
>  create mode 100644 src/app/models/ip_v6_address.rb
>  create mode 100644 src/db/migrate/025_create_ip_addresses.rb
>  create mode 100644 src/db/migrate/026_move_nic_addresses_to_ip_addresses_table.rb
>  create mode 100644 src/db/migrate/027_move_bonding_address_to_ip_addresses_table.rb
>  create mode 100644 src/test/fixtures/ip_addresses.yml
>  create mode 100644 src/test/unit/ip_address_test.rb
>  create mode 100644 src/test/unit/ip_v4_address_test.rb
>  create mode 100644 src/test/unit/ip_v6_address_test.rb
>
> diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
> index 006c261..2fa4aa8 100644
> --- a/src/app/models/bonding.rb
> +++ b/src/app/models/bonding.rb
> @@ -51,6 +51,7 @@ class Bonding < ActiveRecord::Base
>    belongs_to :host
>    belongs_to :bonding_type
>    belongs_to :boot_type
> +  belongs_to :ip_address
>  
>    has_and_belongs_to_many :nics,
>      :join_table  => 'bondings_nics',
> diff --git a/src/app/models/ip_address.rb b/src/app/models/ip_address.rb
> new file mode 100644
> index 0000000..f48cd4e
> --- /dev/null
> +++ b/src/app/models/ip_address.rb
> @@ -0,0 +1,25 @@
> +#
> +# ip_address.rb
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Darryl L. Pierce <dpierce at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +# +IpAddress+ is the base class for all address related classes.
> +#
> +class IpAddress < ActiveRecord::Base
> +
> +end
>   
Why isn't there a "has_many :nics" and a "has_many :bonding"
relationship in the IpAddress table. We wont be able to lookup nics /
bondings from an ip address otherwise.


> diff --git a/src/app/models/ip_v4_address.rb b/src/app/models/ip_v4_address.rb
> new file mode 100644
> index 0000000..4495a13
> --- /dev/null
> +++ b/src/app/models/ip_v4_address.rb
> @@ -0,0 +1,46 @@
> +# ip_v4_address.rb
> +#
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Darryl L. Pierce <dpierce at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +# +IpV4Address+ represents a single IPv4 address.
> +#
> +class IpV4Address < IpAddress
> +  ADDRESS_TEST = %r{^(\d{1,3}\.){3}\d{1,3}$}
> +
> +  validates_presence_of :address,
> +    :message => 'An address must be supplied.'
> +  validates_format_of :address,
> +    :with => ADDRESS_TEST
> +
> +  validates_presence_of :netmask,
> +    :message => 'A netmask must be supplied.'
> +  validates_format_of :netmask,
> +    :with => ADDRESS_TEST
> +
> +  validates_presence_of :gateway,
> +    :message => 'A gateway address must be supplied.'
> +  validates_format_of :gateway,
> +    :with => ADDRESS_TEST
> +
> +  validates_presence_of :broadcast,
> +    :message => 'A broadcast address must be supplied.'
> +  validates_format_of :broadcast,
> +    :with => ADDRESS_TEST
> +
> +end
> diff --git a/src/app/models/ip_v6_address.rb b/src/app/models/ip_v6_address.rb
> new file mode 100644
> index 0000000..1395886
> --- /dev/null
> +++ b/src/app/models/ip_v6_address.rb
> @@ -0,0 +1,40 @@
> +# ip_v6_address.rb
> +#
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Darryl L. Pierce <dpierce at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +# +IpV6Address+ represents a single IPv6 address.
> +#
> +class IpV6Address < IpAddress
> +  ADDRESS_TEST = %r{^([0-9a-fA-F]{0,4}|0)(\:([0-9a-fA-F]{0,4}|0)){7}$}
> +
> +  validates_presence_of :address,
> +    :message => 'An address must be provided.'
> +  validates_format_of :address,
> +    :with => ADDRESS_TEST
> +
> +  validates_presence_of :gateway,
> +    :message => 'A gateway address must be provided.'
> +  validates_format_of :gateway,
> +    :with => ADDRESS_TEST
> +
> +  validates_presence_of :prefix,
> +    :message => 'A prefix must be provided.'
> +  validates_format_of :prefix,
> +    :with => ADDRESS_TEST
> +end
> diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
> index baf7095..69568a9 100644
> --- a/src/app/models/nic.rb
> +++ b/src/app/models/nic.rb
> @@ -22,4 +22,6 @@ class Nic < ActiveRecord::Base
>    belongs_to :boot_type
>  
>    has_and_belongs_to_many :bonding, :join_table => 'bondings_nics'
> +
> +  belongs_to :ip_address
>  end
> diff --git a/src/db/migrate/025_create_ip_addresses.rb b/src/db/migrate/025_create_ip_addresses.rb
> new file mode 100644
> index 0000000..0f528f4
> --- /dev/null
> +++ b/src/db/migrate/025_create_ip_addresses.rb
> @@ -0,0 +1,46 @@
> +#
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Darryl L. Pierce <dpierce at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +# Creates a single-inheritance table for both IPv4 and IPv6
> +# addresses.
> +#
> +class CreateIpAddresses < ActiveRecord::Migration
> +  def self.up
> +    create_table :ip_addresses do |t|
> +      t.string :type
> +
> +      # common attributes
> +      t.string :address,   :limit => 39
> +      t.string :gateway,   :limit => 39
> +
> +      # attributes for IPv4 (type=IpV4Address)
> +      t.string :netmask,   :limit => 15
> +      t.string :broadcast, :limit => 15
> +
> +      # attributes for IPv6 (type=IpV6Address)
> +      t.string :prefix,    :limit => 39
> +
> +      t.timestamps
> +    end
> +  end
> +
> +  def self.down
> +    drop_table :ip_addresses
> +  end
> +end
> diff --git a/src/db/migrate/026_move_nic_addresses_to_ip_addresses_table.rb b/src/db/migrate/026_move_nic_addresses_to_ip_addresses_table.rb
> new file mode 100644
> index 0000000..b873e4d
> --- /dev/null
> +++ b/src/db/migrate/026_move_nic_addresses_to_ip_addresses_table.rb
> @@ -0,0 +1,56 @@
> +#
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Darryl L. Pierce <dpierce at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +class MoveNicAddressesToIpAddressesTable < ActiveRecord::Migration
> +  def self.up
> +    add_column :nics, :ip_address_id, :integer, :null => true
> +
> +    execute 'alter table nics add constraint fk_nic_ip_address
> +             foreign key (ip_address_id) references ip_addresses(id)'
> +
> +    Nic.find(:all).each do |nic|
> +      address = IpV4Address.new(:address   => nic.ip_addr,
> +				:netmask   => nic.netmask,
> +				:broadcast => nic.broadcast,
> +				:gateway   => nic.ip_addr)
> +      nic.ip_address = address
> +
> +      nic.save!
> +      address.save!
> +    end
> +
> +    remove_column :nics, :ip_addr
> +    remove_column :nics, :netmask
> +    remove_column :nics, :broadcast
> +  end
> +
> +  def self.down
> +    add_column :nics, :ip_addr,   :string, :limit => 16
> +    add_column :nics, :netmask,   :string, :limit => 16
> +    add_column :nics, :broadcast, :string, :limit => 16
> +
> +    Nic.find(:all).each do |nic|
> +      nic.ip_addr   = nic.ip_address.address
> +      nic.netmask   = nic.ip_address.netmask
> +      nic.broadcast = nic.ip_address.broadcast
> +    end
> +
> +    remove_column :nics, :ip_address_id
> +  end
> +end
> diff --git a/src/db/migrate/027_move_bonding_address_to_ip_addresses_table.rb b/src/db/migrate/027_move_bonding_address_to_ip_addresses_table.rb
> new file mode 100644
> index 0000000..53f7058
> --- /dev/null
> +++ b/src/db/migrate/027_move_bonding_address_to_ip_addresses_table.rb
> @@ -0,0 +1,54 @@
> +#
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Darryl L. Pierce <dpierce at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +class MoveBondingAddressToIpAddressesTable < ActiveRecord::Migration
> +  def self.up
> +    add_column :bondings, :ip_address_id, :integer, :null => true
> +
> +    execute 'alter table bondings add constraint fk_bonding_ip_address
> +             foreign key (ip_address_id) references ip_addresses(id)'
> +
> +    Bonding.find(:all).each do |bonding|
> +      ip_address = IpV4Address.new(:address   => bonding.ip_addr,
> +				   :netmask   => bonding.netmask,
> +				   :broadcast => bonding.broadcast,
> +				   :gateway   => bonding.ip_addr)
> +    end
> +
> +    remove_column :bondings, :ip_addr
> +    remove_column :bondings, :netmask
> +    remove_column :bondings, :broadcast
> +  end
> +
> +  def self.down
> +    t.string  :ip_addr,         :null => true, :limit => 15
> +    t.string  :netmask,         :null => true, :limit => 15
> +    t.string  :broadcast,       :null => true, :limit => 15
> +    
> +    Bonding.each do |bonding|
> +      bonding.ip_addr   = bonding.ip_address.address
> +      bonding.netmask   = bonding.ip_address.netmask
> +      bonding.broadcast = bonding.ip_address.broadcast
> +
> +      bonding.save!
> +    end
> +
> +    remove_column :bondings, :ip_address_id
> +  end
> +end
> diff --git a/src/host-browser/host-browser.rb b/src/host-browser/host-browser.rb
> index 79d34e8..bbfbf3b 100755
> --- a/src/host-browser/host-browser.rb
> +++ b/src/host-browser/host-browser.rb
> @@ -283,10 +283,10 @@ class HostBrowser
>  
>                      updated_nic = Nic.find_by_id(nic.id)
>  
> -                    updated_nic.bandwidth = detail['BANDWIDTH']
>   
Did you really mean to remove bandwidth here? I still see it in the db
table after the migrations and this seems to be the only place that a
change to it occurs.

> -                    updated_nic.ip_addr   = detail['IP_ADDRESS']
> -                    updated_nic.netmask   = detail['NETMASK']
> -                    updated_nic.broadcast = detail['BROADCAST']
> +                    updated_nic.bandwidth            = detail['BANDWIDTH']
> +                    updated_nic.ip_address.address   = detail['IP_ADDRESS']
> +                    updated_nic.ip_address.netmask   = detail['NETMASK']
> +                    updated_nic.ip_address.broadcast = detail['BROADCAST']
>  
>                      updated_nic.save!
>                      found=true
> @@ -310,11 +310,14 @@ class HostBrowser
>                  'mac'          => nic['MAC'].upcase,
>                  'bandwidth'    => nic['BANDWIDTH'],
>                  'usage_type'   => 1,
> -                'ip_addr'      => nic['IP_ADDRESS'],
> -                'netmask'      => nic['NETMASK'],
> -                'broadcast'    => nic['BROADCAST'],
>                  'boot_type_id' => boot_type.id)
>  
> +	    ip_address = IpV4Address.new('address'   => nic['IP_ADDRESS'],
> +					 'netmask'   => nic['NETMASK'],
> +					 'broadcast' => nic['BROADCAST'],
> +					 'gateway'   => nic['GATEWAY'])
> +	    detail.ip_address = ip_address
> +
>              host.nics << detail
>          end
>  
> diff --git a/src/lib/managed_node_configuration.rb b/src/lib/managed_node_configuration.rb
> index 93f8448..fff7914 100644
> --- a/src/lib/managed_node_configuration.rb
> +++ b/src/lib/managed_node_configuration.rb
> @@ -51,7 +51,7 @@ class ManagedNodeConfiguration
>      host.bondings.each do |bonding|
>        result.puts "rm #{NIC_ENTRY_PREFIX}/ifcfg-#{bonding.interface_name}"
>        result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{bonding.interface_name}/DEVICE #{bonding.interface_name}"
> -      result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{bonding.interface_name}/IPADDR #{bonding.ip_addr}" if bonding.ip_addr
> +      result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{bonding.interface_name}/IPADDR #{bonding.ip_address.address}" if bonding.ip_address
>        result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{bonding.interface_name}/ONBOOT yes"
>  
>        bonding.nics.each do |nic|
> @@ -97,9 +97,9 @@ class ManagedNodeConfiguration
>          result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{iface_name}/BOOTPROTO #{nic.boot_type.proto}"
>  
>          if nic.boot_type.proto == 'static'
> -          result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{iface_name}/IPADDR #{nic.ip_addr}"
> -          result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{iface_name}/NETMASK #{nic.netmask}"
> -          result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{iface_name}/BROADCAST #{nic.broadcast}"
> +          result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{iface_name}/IPADDR #{nic.ip_address.address}"
> +          result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{iface_name}/NETMASK #{nic.ip_address.netmask}"
> +          result.puts "set #{NIC_ENTRY_PREFIX}/ifcfg-#{iface_name}/BROADCAST #{nic.ip_address.broadcast}"
>          end
>  
>  	if bridged
>   
The host-browser and managed_node_configuration are the bits I'm least
familiar with. That being said, after applying your patch and rebooting,
everything seems to work. I also went through the changes above and
everything looks good (besides the aformentioned note on bandwidth).


> diff --git a/src/test/fixtures/bondings.yml b/src/test/fixtures/bondings.yml
> index 29473c7..1a74f42 100644
> --- a/src/test/fixtures/bondings.yml
> +++ b/src/test/fixtures/bondings.yml
> @@ -4,8 +4,6 @@ mailservers_managed_node_bonding:
>      bonding_type_id: <%= Fixtures.identify(:link_aggregation_bonding_type) %>
>      host_id: <%= Fixtures.identify(:mailservers_managed_node) %>
>      boot_type_id: <%= Fixtures.identify(:boot_type_static) %>
> -    ip_addr: 172.31.0.15
> -    netmask: 255.255.255.
> -    broadcast: 172.31.0.255
> +    ip_address_id: <%= Fixtures.identify(:ip_v4_mailservers_managed_node_bonding) %>
>      arp_ping_address: 172.31.0.100
>      arp_interval: 0
> diff --git a/src/test/fixtures/ip_addresses.yml b/src/test/fixtures/ip_addresses.yml
> new file mode 100644
> index 0000000..0472669
> --- /dev/null
> +++ b/src/test/fixtures/ip_addresses.yml
> @@ -0,0 +1,55 @@
> +ip_v4_one:
> +  type: IpV4Address
> +  address: 1.2.3.4
> +  netmask: 255.255.255.0
> +  gateway: 1.2.3.1
> +  broadcast: 1.2.3.255
> +
> +ip_v4_two:
> +  type: IpV4Address
> +  address: 2.3.4.5
> +  netmask: 255.255.255.0
> +  gateway: 1.2.3.1
> +  broadcast: 2.3.4.255
> +
> +ip_v4_three:
> +  type: IpV4Address
> +  address: 3.4.5.6
> +  netmask: 255.255.255.0
> +  gateway: 3.4.5.1
> +  broadcast: 3.4.5.255
> +
> +ip_v4_four:
> +  type: IpV4Address
> +  address: 3.4.5.6
> +  netmask: 255.255.255.0
> +  gateway: 3.4.5.1
> +  broadcast: 3.4.5.255
> +
> +ip_v4_mailserver_nic_one:
> +  type: IpV4Address
> +  address: 172.31.0.15
> +  netmask: 255.255.255.0
> +  gateway: 172.31.0.1
> +  broadcast: 172.31.0.255
> +
> +ip_v4_ldapserver_nic_one:
> +  type: IpV4Address
> +  address: 172.31.0.25
> +  netmask: 255.255.255.0
> +  gateway: 172.31.0.1
> +  broadcast: 172.31.0.255
> +
> +ip_v4_buildserver_nic_two:
> +  type: IpV4Address
> +  address: 172.31.0.31
> +  netmask: 255.255.255.0
> +  gateway: 172.31.0.1
> +  broadcast: 172.31.0.255
> +
> +ip_v4_mailservers_managed_node_bonding:
> +  type: IpV4Address
> +  address: 192.168.50.100
> +  netmask: 255.255.255.0
> +  gateway: 192.168.50.1
> +  broadcast: 192.168.50.255
> diff --git a/src/test/fixtures/nics.yml b/src/test/fixtures/nics.yml
> index ccf71d2..dbbc511 100644
> --- a/src/test/fixtures/nics.yml
> +++ b/src/test/fixtures/nics.yml
> @@ -1,80 +1,80 @@
>  one:
>    id: 1
>    mac: '00:11:22:33:44:55'
> -  ip_addr: '1.2.3.4'
> -  usage_type: '1'
> +  ip_address_id: <%= Fixtures.identify(:ip_v4_one) %>
>    bandwidth: 100
>    host_id: 10
>    boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
> +
>  two:
>    id: 2
>    mac: 'AA:BB:CC:DD:EE:FF'
> -  ip_addr: '2.3.4.5'
> +  ip_address_id: <%= Fixtures.identify(:ip_v4_two) %>
>    usage_type: '2'
>    bandwidth: 1000
>    host_id: 10
>    boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
> +
>  three:
>    id: 3
>    mac: '00:FF:11:EE:22:DD'
> -  ip_addr: '3.4.5.6'
> +  ip_address_id: <%= Fixtures.identify(:ip_v4_three) %>
>    usage_type: '1'
>    bandwidth: 10
>    host_id: 10
>    boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
> +
>  four:
>    id: 4
>    mac: '00:FF:11:EE:22:DD'
> -  ip_addr: '3.4.5.6'
> +  ip_address: <%= Fixtures.identify(:ip_v4_four) %>
>    usage_type: '1'
>    bandwidth: 10
>    host_id: 10
>    boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
>  
>  mailserver_nic_one:
> -    mac: '00:11:22:33:44:55'
> -    usage_type: '1'
> -    bandwidth: 100
> -    ip_addr: '172.31.0.15'
> -    host_id: <%= Fixtures.identify(:mailservers_managed_node) %>
> -    boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
> +  mac: '00:11:22:33:44:55'
> +  ip_address: <%= Fixtures.identify(:ip_v4_mailserver_nic_one) %>
> +  usage_type: '1'
> +  bandwidth: 100
> +  host_id: <%= Fixtures.identify(:mailservers_managed_node) %>
> +  boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
>  
>  mailserver_nic_two:
> -    mac: '22:11:33:66:44:55'
> -    usage_type: '1'
> -    bandwidth: 100
> -    host_id: <%= Fixtures.identify(:mailservers_managed_node) %>
> -    boot_type_id: <%= Fixtures.identify(:boot_type_dhcp) %>
> +  mac: '22:11:33:66:44:55'
> +  usage_type: '1'
> +  bandwidth: 100
> +  host_id: <%= Fixtures.identify(:mailservers_managed_node) %>
> +  boot_type_id: <%= Fixtures.identify(:boot_type_dhcp) %>
>  
>  fileserver_nic_one:
> -    mac: '00:99:00:99:13:07'
> -    usage_type: '1'
> -    bandwidth: 100
> -    host_id: <%= Fixtures.identify(:fileserver_managed_node) %>
> -    boot_type_id: <%= Fixtures.identify(:boot_type_dhcp) %>
> +  mac: '00:99:00:99:13:07'
> +  usage_type: '1'
> +  bandwidth: 100
> +  host_id: <%= Fixtures.identify(:fileserver_managed_node) %>
> +  boot_type_id: <%= Fixtures.identify(:boot_type_dhcp) %>
>  
>  ldapserver_nic_one:
> -    mac: '00:03:02:00:09:06'
> -    usage_type: '1'
> -    bandwidth: 100
> -    bridge: 'ovirtbr0'
> -    ip_addr: '172.31.0.25'
> -    host_id: <%= Fixtures.identify(:ldapserver_managed_node) %>
> -    boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
> +  mac: '00:03:02:00:09:06'
> +  usage_type: '1'
> +  bandwidth: 100
> +  bridge: 'ovirtbr0'
> +  ip_address_id: <%= Fixtures.identify(:ip_v4_ldapserver_nic_one) %>
> +  host_id: <%= Fixtures.identify(:ldapserver_managed_node) %>
> +  boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
>  
>  buildserver_nic_one:
> -    mac: '07:17:19:65:03:38'
> -    usage_type: '1'
> -    bandwidth: 100
> -    host_id: <%= Fixtures.identify(:buildserver_managed_node) %>
> -    boot_type_id: <%= Fixtures.identify(:boot_type_dhcp) %>
> +  mac: '07:17:19:65:03:38'
> +  usage_type: '1'
> +  bandwidth: 100
> +  host_id: <%= Fixtures.identify(:buildserver_managed_node) %>
> +  boot_type_id: <%= Fixtures.identify(:boot_type_dhcp) %>
>  
>  buildserver_nic_two:
> -    mac: '07:17:19:65:03:39'
> -    usage_type: '1'
> -    bandwidth: 100
> -    ip_addr: '172.31.0.31'
> -    netmask: '255.255.255.0'
> -    broadcast: '172.31.0.255'
> -    host_id: <%= Fixtures.identify(:buildserver_managed_node) %>
> -    boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
> +  mac: '07:17:19:65:03:39'
> +  usage_type: '1'
> +  bandwidth: 100
> +  ip_address_id: <%= Fixtures.identify(:ip_v4_buildserver_nic_two) %>
> +  host_id: <%= Fixtures.identify(:buildserver_managed_node) %>
> +  boot_type_id: <%= Fixtures.identify(:boot_type_static_ip) %>
> diff --git a/src/test/functional/managed_node_configuration_test.rb b/src/test/functional/managed_node_configuration_test.rb
> index d0d8aa3..2158c8d 100644
> --- a/src/test/functional/managed_node_configuration_test.rb
> +++ b/src/test/functional/managed_node_configuration_test.rb
> @@ -83,17 +83,17 @@ cat <<\EOF > /var/tmp/node-augtool
>  rm /files/etc/sysconfig/network-scripts/ifcfg-eth0
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/DEVICE eth0
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/BOOTPROTO #{nic.boot_type.proto}
> -set /files/etc/sysconfig/network-scripts/ifcfg-eth0/IPADDR #{nic.ip_addr}
> -set /files/etc/sysconfig/network-scripts/ifcfg-eth0/NETMASK #{nic.netmask}
> -set /files/etc/sysconfig/network-scripts/ifcfg-eth0/BROADCAST #{nic.broadcast}
> +set /files/etc/sysconfig/network-scripts/ifcfg-eth0/IPADDR #{nic.ip_address.address}
> +set /files/etc/sysconfig/network-scripts/ifcfg-eth0/NETMASK #{nic.ip_address.netmask}
> +set /files/etc/sysconfig/network-scripts/ifcfg-eth0/BROADCAST #{nic.ip_address.broadcast}
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/BRIDGE ovirtbr0
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/ONBOOT yes
>  rm /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/DEVICE ovirtbr0
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/BOOTPROTO #{nic.boot_type.proto}
> -set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/IPADDR #{nic.ip_addr}
> -set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/NETMASK #{nic.netmask}
> -set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/BROADCAST #{nic.broadcast}
> +set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/IPADDR #{nic.ip_address.address}
> +set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/NETMASK #{nic.ip_address.netmask}
> +set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/BROADCAST #{nic.ip_address.broadcast}
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/TYPE bridge
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/PEERNTP yes
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/DELAY 0
> @@ -123,17 +123,17 @@ cat <<\EOF > /var/tmp/node-augtool
>  rm /files/etc/sysconfig/network-scripts/ifcfg-eth0
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/DEVICE eth0
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/BOOTPROTO #{nic1.boot_type.proto}
> -set /files/etc/sysconfig/network-scripts/ifcfg-eth0/IPADDR #{nic1.ip_addr}
> -set /files/etc/sysconfig/network-scripts/ifcfg-eth0/NETMASK #{nic1.netmask}
> -set /files/etc/sysconfig/network-scripts/ifcfg-eth0/BROADCAST #{nic1.broadcast}
> +set /files/etc/sysconfig/network-scripts/ifcfg-eth0/IPADDR #{nic1.ip_address.address}
> +set /files/etc/sysconfig/network-scripts/ifcfg-eth0/NETMASK #{nic1.ip_address.netmask}
> +set /files/etc/sysconfig/network-scripts/ifcfg-eth0/BROADCAST #{nic1.ip_address.broadcast}
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/BRIDGE ovirtbr0
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/ONBOOT yes
>  rm /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/DEVICE ovirtbr0
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/BOOTPROTO #{nic1.boot_type.proto}
> -set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/IPADDR #{nic1.ip_addr}
> -set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/NETMASK #{nic1.netmask}
> -set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/BROADCAST #{nic1.broadcast}
> +set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/IPADDR #{nic1.ip_address.address}
> +set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/NETMASK #{nic1.ip_address.netmask}
> +set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/BROADCAST #{nic1.ip_address.broadcast}
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/TYPE bridge
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/PEERNTP yes
>  set /files/etc/sysconfig/network-scripts/ifcfg-ovirtbr0/DELAY 0
> @@ -177,7 +177,7 @@ EOF
>  cat <<\EOF > /var/tmp/node-augtool
>  rm /files/etc/sysconfig/network-scripts/ifcfg-#{bonding.interface_name}
>  set /files/etc/sysconfig/network-scripts/ifcfg-#{bonding.interface_name}/DEVICE #{bonding.interface_name}
> -set /files/etc/sysconfig/network-scripts/ifcfg-#{bonding.interface_name}/IPADDR 172.31.0.15
> +set /files/etc/sysconfig/network-scripts/ifcfg-#{bonding.interface_name}/IPADDR #{bonding.ip_address.address}
>  set /files/etc/sysconfig/network-scripts/ifcfg-#{bonding.interface_name}/ONBOOT yes
>  rm /files/etc/sysconfig/network-scripts/ifcfg-eth0
>  set /files/etc/sysconfig/network-scripts/ifcfg-eth0/DEVICE eth0
> diff --git a/src/test/unit/ip_address_test.rb b/src/test/unit/ip_address_test.rb
> new file mode 100644
> index 0000000..152578e
> --- /dev/null
> +++ b/src/test/unit/ip_address_test.rb
> @@ -0,0 +1,8 @@
> +require 'test_helper'
> +
> +class IpAddressTest < ActiveSupport::TestCase
> +  # Replace this with your real tests.
> +  def test_truth
> +    assert true
> +  end
> +end
> diff --git a/src/test/unit/ip_v4_address_test.rb b/src/test/unit/ip_v4_address_test.rb
> new file mode 100644
> index 0000000..14f0175
> --- /dev/null
> +++ b/src/test/unit/ip_v4_address_test.rb
> @@ -0,0 +1,99 @@
> +#
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Darryl L. Pierce <dpierce at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +require File.dirname(__FILE__) + '/../test_helper'
> +
> +class IpV4AddressTest < ActiveSupport::TestCase
> +  def setup
> +    @address = IpV4Address.new(:address   => '192.168.50.2',
> +			       :netmask   => '255.255.255.0',
> +			       :gateway   => '192.168.50.1',
> +			       :broadcast => '192.168.50.255')
> +  end
> +
> +  # Ensures that an address must be supplied.
> +  #
> +  def test_valid_fails_without_address
> +    @address.address = nil
> +
> +    flunk "An address must be present." if @address.valid?
> +  end
> +
> +  # Ensures that an address has to be in the correct format.
> +  #
> +  def test_valid_fails_with_bad_address
> +    @address.address = '192.168'
> +
> +    flunk "An address must be in the format ##.##.##.##." if @address.valid?
> +  end
> +
> +  # Ensures that a netmask must be supplied.
> +  #
> +  def test_valid_fails_without_netmask
> +    @address.netmask = nil
> +
> +    flunk "An address must have a netmask." if @address.valid?
> +  end
> +
> +  # Ensures that a netmask must have the correct format.
> +  #
> +  def test_valid_fails_with_bad_netmask
> +    @address.netmask = 'farkle'
> +
> +    flunk "A netmask must be in the format ##.##.##.##." if @address.valid?
> +  end
> +
> +  # Ensures that a gateway must be supplied.
> +  #
> +  def test_valid_fails_without_gateway
> +    @address.gateway = nil
> +
> +    flunk "A gateway address must be supplied." if @address.valid?
> +  end
> +
> +  # Ensures that a gateway must be in the correct format.
> +  #
> +  def test_valid_fails_with_bad_gateway
> +    @address.gateway = '-3.a2.0.8'
> +
> +    flunk "The gateway address must be in the format ##.##.##.##." if @address.valid?
> +  end
> +
> +  # Ensures that a broadcast must be supplied.
> +  #
> +  def test_valid_fails_without_broadcast
> +    @address.broadcast = nil
> +
> +    flunk "A broadcast addres must be supplied." if @address.valid?
> +  end
> +
> +  # Ensures that a broadcast must be in the correct format.
> +  #
> +  def test_valid_fails_with_bad_broadcast
> +    @address.broadcast = '-3.2.0.8'
> +
> +    flunk "The broadcast address must be in the format ##.##.##.##." if @address.valid?
> +  end
> +
> +  # Ensures that a well-formed address is valid.
> +  #
> +  def test_valid
> +    flunk "There is an error with validation." unless @address.valid?
> +  end
> +end
> diff --git a/src/test/unit/ip_v6_address_test.rb b/src/test/unit/ip_v6_address_test.rb
> new file mode 100644
> index 0000000..a2be85f
> --- /dev/null
> +++ b/src/test/unit/ip_v6_address_test.rb
> @@ -0,0 +1,82 @@
> +#
> +# Copyright (C) 2008 Red Hat, Inc.
> +# Written by Darryl L. Pierce <dpierce at redhat.com>
> +#
> +# This program is free software; you can redistribute it and/or modify
> +# it under the terms of the GNU General Public License as published by
> +# the Free Software Foundation; version 2 of the License.
> +#
> +# This program is distributed in the hope that it will be useful,
> +# but WITHOUT ANY WARRANTY; without even the implied warranty of
> +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> +# GNU General Public License for more details.
> +#
> +# You should have received a copy of the GNU General Public License
> +# along with this program; if not, write to the Free Software
> +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston,
> +# MA  02110-1301, USA.  A copy of the GNU General Public License is
> +# also available at http://www.gnu.org/copyleft/gpl.html.
> +
> +require File.dirname(__FILE__) + '/../test_helper'
> +
> +class IpV6AddressTest < ActiveSupport::TestCase
> +  def setup
> +    @address = IpV6Address.new(:address => 'fe80:0:0:0:200:f8ff:fe21:67cf',
> +			       :gateway => ':::::::717',
> +			       :prefix  => '0000:0000:0000:0000:1234:1234:1234:1234')
> +  end
> +
> +  # Ensures that the address must be provided.
> +  #
> +  def test_valid_fails_without_address
> +    @address.address = nil
> +
> +    flunk "An address must be provided." if @address.valid?
> +  end
> +
> +  # Ensures that the address must be in the correct format.
> +  #
> +  def test_valid_fails_with_bad_address
> +    @address.address = "farkle"
> +
> +    flunk "The address must be in the correct format." if @address.valid?
> +  end
> +
> +  # Ensures that the gateway must be provided.
> +  #
> +  def test_valid_fails_without_gateway
> +    @address.gateway = nil
> +
> +    flunk "The gateway address must be provided." if @address.valid?
> +  end
> +
> +  # Ensures that the gateway address is in the correct format.
> +  #
> +  def test_valid_fails_with_bad_gateway
> +    @address.gateway = '0-:::::::717'
> +
> +    flunk "The gateway address must be in the correct format." if @address.valid?
> +  end
> +
> +  # Ensures that the prefix must be provided.
> +  #
> +  def test_valid_fails_without_prefix
> +    @address.prefix = nil
> +
> +    flunk "The prefix must be provided." if @address.valid?
> +  end
> +
> +  # Ensures that the prefix is in the correct format.
> +  #
> +  def test_valid_fails_with_invalid_prefix
> +    @address.prefix = 'whoops'
> +
> +    flunk "The prefix must be in the correct format." if @address.valid?
> +  end
> +
> +  # Ensures that a well-formed address is considered valid.
> +  #
> +  def test_valid
> +    flunk "There is an problem with address validation." unless @address.valid?
> +  end
> +end
> diff --git a/src/test/unit/nic_test.rb b/src/test/unit/nic_test.rb
> index a0776a2..1de1e00 100644
> --- a/src/test/unit/nic_test.rb
> +++ b/src/test/unit/nic_test.rb
> @@ -20,6 +20,7 @@
>  require File.dirname(__FILE__) + '/../test_helper'
>  
>  class NicTest < Test::Unit::TestCase
> +  fixtures :ip_addresses
>    fixtures :nics
>  
>    # Replace this with your real tests.
>   
I ran these tests and everything succeeded flawlessly. For the most
part, besides the mentioned items, everything seemed to come together
and work nicely. Good job.

    -Mo





More information about the ovirt-devel mailing list