Mohammed Morsi
2009-Jan-15 20:21 UTC
[Ovirt-devel] [PATCH server] comprehensive / up to date validations patch (revised again)
(revisions relate to small validation bugs in host, nic, task models
and host-browser module)
this patch provides all the validation components included
in the previous 3-patch patch-set, updated to incorporate
recent changes to the rails application since they were
originally sent out and to remove extraneous whitespace.
This is the only patch that needs to be ack'd as it
contains the validations, tests, and bug fixes
Attached is additional validations, added to the rails model
and controller layers verifying:
bondings: arp related fields
boot_types: label uniqueness and proto inclusion
cpus: various fields are set and min values
help_sections: presence and uniqueness of fields
hosts: presence and format of various fields
ip_addresses: fk existance / integrity
nics: mac address format
permissions: presence / inclusion of various fields
pools: type inclusion
quotas: minimum values
smart_pool_tags: tagged_id / type fk present
storage_pools: type, state field inclusion, min values
storage_volumes: various fields, subclasses w/ various fields
tasks: various fields present and consistensy maintained
usages: label, other fields present, unique
vms: uuid of correct format, min values, various fields present
pools lft and rgt are not verified as they are added by
acts_as_nested_set _after_ the pool is inserted into the db
can only add / move host and storage volumes and pools to / from pools
when you have sufficient permissions and target entities don't have
associated vms
can only destroy storage volumes / pools if there are no attached vms
included are a few updates to the main server app and many updates
to the test suite to fix a few various bits and assert all the new
validations added.
---
src/app/controllers/hardware_controller.rb | 28 ++++++-
src/app/controllers/storage_controller.rb | 7 ++
src/app/models/bonding.rb | 15 +++-
src/app/models/boot_type.rb | 5 +
src/app/models/cpu.rb | 23 +++++
src/app/models/help_section.rb | 34 ++++++++
src/app/models/host.rb | 22 +++++-
src/app/models/ip_address.rb | 4 +
src/app/models/iscsi_storage_volume.rb | 2 +
src/app/models/lvm_storage_volume.rb | 6 ++
src/app/models/nfs_storage_pool.rb | 1 +
src/app/models/nic.rb | 15 +++-
src/app/models/permission.rb | 7 ++
src/app/models/pool.rb | 3 +
src/app/models/quota.rb | 22 +++++
src/app/models/smart_pool_tag.rb | 5 +
src/app/models/storage_pool.rb | 17 ++++
src/app/models/storage_volume.rb | 21 +++++
src/app/models/task.rb | 16 ++++
src/app/models/usage.rb | 6 ++
src/app/models/vm.rb | 36 ++++++++-
src/host-browser/host-browser.rb | 12 ++--
src/test/fixtures/help_sections.yml | 14 ++-
src/test/fixtures/hosts.yml | 45 +++++++----
src/test/fixtures/storage_pools.yml | 11 +++
src/test/fixtures/usages.yml | 12 ++-
src/test/functional/resources_controller_test.rb | 2 +-
src/test/functional/storage_controller_test.rb | 4 +-
src/test/unit/bonding_test.rb | 35 ++++++++
src/test/unit/boot_type_test.rb | 36 +++++++-
src/test/unit/cpu_test.rb | 95 +++++++++++++++++++++-
src/test/unit/help_section_test.rb | 51 +++++++++++-
src/test/unit/host_test.rb | 60 +++++++++++++-
src/test/unit/ip_address_test.rb | 5 +
src/test/unit/ip_v4_address_test.rb | 3 +-
src/test/unit/ip_v6_address_test.rb | 3 +-
src/test/unit/nic_test.rb | 58 +++++++++++++-
src/test/unit/permission_test.rb | 28 +++++++
src/test/unit/pool_test.rb | 21 +++++
src/test/unit/quota_test.rb | 37 ++++++++-
src/test/unit/storage_pool_test.rb | 45 ++++++++++
src/test/unit/storage_volume_test.rb | 86 +++++++++++++++++++
src/test/unit/task_test.rb | 14 +++
src/test/unit/usage_test.rb | 47 ++++++++++-
src/test/unit/vm_test.rb | 64 +++++++++++++++
45 files changed, 1014 insertions(+), 69 deletions(-)
diff --git a/src/app/controllers/hardware_controller.rb
b/src/app/controllers/hardware_controller.rb
index 7be67cc..5c14eec 100644
--- a/src/app/controllers/hardware_controller.rb
+++ b/src/app/controllers/hardware_controller.rb
@@ -293,15 +293,39 @@ class HardwareController < PoolController
resource_ids_str = params[:resource_ids]
resource_ids = resource_ids_str.split(",").collect {|x| x.to_i}
+ # if user doesn't have modify permission on both source and destination
+ unless @pool.can_modify(@user) and
Pool.find(params[:target_pool_id]).can_modify(@user)
+ render :json => { :success => false,
+ :alert => "Cannot #{item_action.to_s}
#{item_class.table_name.humanize} without admin permissions on both pools"
}
+ return
+ end
+
+ # relay error message if movable check fails for any resource
+ success = true
+ failed_resources = ""
+ resource_ids.each {|x|
+ unless item_class.find(x).movable?
+ success = false
+ failed_resources += x.to_s + " "
+ end
+ }
+ resource_ids.delete_if { |x| ! item_class.find(x).movable? }
+
begin
@pool.transaction do
@pool.send(item_method, resource_ids, target_pool_id)
end
+ rescue
+ success = false
+ end
+
+ if success
render :json => { :success => true,
:alert => "#{item_action.to_s}
#{item_class.table_name.humanize} successful." }
- rescue
+ else
render :json => { :success => false,
- :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed." }
+ :alert => "#{item_action.to_s}
#{item_class.table_name.humanize} failed" +
+ (failed_resources == "" ? "." : "
for " + failed_resources) }
end
end
diff --git a/src/app/controllers/storage_controller.rb
b/src/app/controllers/storage_controller.rb
index 148d1be..e4b72f1 100644
--- a/src/app/controllers/storage_controller.rb
+++ b/src/app/controllers/storage_controller.rb
@@ -310,6 +310,13 @@ class StorageController < ApplicationController
end
def destroy
+ unless @storage_pool.movable?
+ format.json { render :json => { :object => "storage_pool",
+ :success => false,
+ :alert => "Cannot delete storage with associated vms" } }
+ return
+ end
+
pool = @storage_pool.hardware_pool
if @storage_pool.destroy
alert="Storage Pool was successfully deleted."
diff --git a/src/app/models/bonding.rb b/src/app/models/bonding.rb
index c9af38c..a67b97b 100644
--- a/src/app/models/bonding.rb
+++ b/src/app/models/bonding.rb
@@ -30,6 +30,8 @@
# interface. They can be ignored if not used.
#
class Bonding < ActiveRecord::Base
+
+
belongs_to :host
belongs_to :bonding_type
belongs_to :vlan
@@ -58,9 +60,20 @@ class Bonding < ActiveRecord::Base
validates_presence_of :vlan_id,
:message => 'A vlan must be specified.'
+ # verify arp ping address to be ipv4 if set
+ validates_format_of :arp_ping_address,
+ :with => %r{^(\d{1,3}\.){3}\d{1,3}$},
+ :unless => Proc.new { |bonding| bonding.arp_ping_address.nil? }
+
+ validates_numericality_of :arp_interval,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new { |bonding| bonding.arp_interval.nil? }
+
protected
def validate
- if vlan.boot_type.proto == 'static' and ip_addresses.size == 0
+ if ! vlan.nil? and
+ vlan.boot_type.proto == 'static' and
+ ip_addresses.size == 0
errors.add("vlan_id",
"is static. Must create at least one static
ip")
end
diff --git a/src/app/models/boot_type.rb b/src/app/models/boot_type.rb
index 8ffbe67..6cfdb04 100644
--- a/src/app/models/boot_type.rb
+++ b/src/app/models/boot_type.rb
@@ -18,4 +18,9 @@
# also available at http://www.gnu.org/copyleft/gpl.html.
class BootType < ActiveRecord::Base
+ validates_presence_of :label
+ validates_uniqueness_of :label,
+ :message => 'Label must be unique'
+ validates_inclusion_of :proto,
+ :in => %w( static dhcp bootp )
end
diff --git a/src/app/models/cpu.rb b/src/app/models/cpu.rb
index 1a7d3cf..714a8ed 100644
--- a/src/app/models/cpu.rb
+++ b/src/app/models/cpu.rb
@@ -21,4 +21,27 @@
#
class Cpu < ActiveRecord::Base
belongs_to :host
+
+ validates_presence_of :host_id,
+ :message => 'A host must be specified.'
+
+ validates_numericality_of :cpu_number,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :core_number,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :number_of_cores,
+ :greater_than_or_equal_to => 1
+
+ validates_numericality_of :cpuid_level,
+ :greater_than_or_equal_to => 0
+
+ validates_numericality_of :speed,
+ :greater_than => 0
+ # also verify speed in MHz ?
+
+ validates_presence_of :vendor
+ validates_presence_of :model
+ validates_presence_of :family
end
diff --git a/src/app/models/help_section.rb b/src/app/models/help_section.rb
index a891383..bd3e27c 100644
--- a/src/app/models/help_section.rb
+++ b/src/app/models/help_section.rb
@@ -1,2 +1,36 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi <mmorsi 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.
+#
+# +HelpSection+ defines a section of the web help document to be
+# available for a specific controller / action
+#
class HelpSection < ActiveRecord::Base
+
+ validates_uniqueness_of :action,
+ :scope => :controller,
+ :message => 'Controller / Action must be unique'
+
+ validates_presence_of :controller,
+ :message => 'A controller must be specified.'
+
+ validates_presence_of :action,
+ :message => 'An action must be specified.'
+
+ validates_presence_of :section,
+ :message => 'A section must be specified.'
end
diff --git a/src/app/models/host.rb b/src/app/models/host.rb
index 813bc1d..1e3a0e6 100644
--- a/src/app/models/host.rb
+++ b/src/app/models/host.rb
@@ -60,13 +60,29 @@ class Host < ActiveRecord::Base
[ :search_users, 'U',
"search_users" ] ],
:eager_load => :smart_pools
+ validates_presence_of :hardware_pool_id,
+ :message => 'A hardware pool id must be specified.'
+
+ validates_presence_of :hostname,
+ :message => 'A hostname must be specified.'
+
+ validates_presence_of :arch,
+ :message => 'An arch must be specified.'
KVM_HYPERVISOR_TYPE = "KVM"
- HYPERVISOR_TYPES = [KVM_HYPERVISOR_TYPE]
+ QEMU_HYPERVISOR_TYPE = "QEMU"
+ HYPERVISOR_TYPES = [KVM_HYPERVISOR_TYPE, QEMU_HYPERVISOR_TYPE]
STATE_UNAVAILABLE = "unavailable"
STATE_AVAILABLE = "available"
STATES = [STATE_UNAVAILABLE, STATE_AVAILABLE]
+ validates_inclusion_of :hypervisor_type,
+ :in => HYPERVISOR_TYPES,
+ :unless => Proc.new { |host| host.hypervisor_type.nil? or
host.hypervisor_type == "" }
+
+ validates_inclusion_of :state,
+ :in => STATES + Task::COMPLETED_STATES + Task::WORKING_STATES
+
def memory_in_mb
kb_to_mb(memory)
end
@@ -108,4 +124,8 @@ class Host < ActiveRecord::Base
hardware_pool.search_users
end
+ def movable?
+ return vms.size == 0
+ end
+
end
diff --git a/src/app/models/ip_address.rb b/src/app/models/ip_address.rb
index 5d2e6af..3f246b1 100644
--- a/src/app/models/ip_address.rb
+++ b/src/app/models/ip_address.rb
@@ -24,4 +24,8 @@ class IpAddress < ActiveRecord::Base
belongs_to :network
belongs_to :nic
belongs_to :bonding
+
+ def validate
+ errors.add("id", "ip must be associated with foreign
entity") if network_id.nil? and nic_id.nil? and bonding_id.nil?
+ end
end
diff --git a/src/app/models/iscsi_storage_volume.rb
b/src/app/models/iscsi_storage_volume.rb
index 48edbd8..fe2cbf5 100644
--- a/src/app/models/iscsi_storage_volume.rb
+++ b/src/app/models/iscsi_storage_volume.rb
@@ -31,4 +31,6 @@ class IscsiStorageVolume < StorageVolume
return true
end
+ validates_presence_of :lun
+
end
diff --git a/src/app/models/lvm_storage_volume.rb
b/src/app/models/lvm_storage_volume.rb
index 8eb1f0e..38949ce 100644
--- a/src/app/models/lvm_storage_volume.rb
+++ b/src/app/models/lvm_storage_volume.rb
@@ -29,4 +29,10 @@ class LvmStorageVolume < StorageVolume
def volume_create_params
return lv_name, size, lv_owner_perms, lv_group_perms, lv_mode_perms
end
+
+ validates_presence_of :lv_name
+ validates_presence_of :lv_owner_perms
+ validates_presence_of :lv_group_perms
+ validates_presence_of :lv_mode_perms
+
end
diff --git a/src/app/models/nfs_storage_pool.rb
b/src/app/models/nfs_storage_pool.rb
index 398e3f9..3b438c8 100644
--- a/src/app/models/nfs_storage_pool.rb
+++ b/src/app/models/nfs_storage_pool.rb
@@ -29,4 +29,5 @@ class NfsStoragePool < StoragePool
def user_subdividable
true
end
+
end
diff --git a/src/app/models/nic.rb b/src/app/models/nic.rb
index 5649763..cd21bb7 100644
--- a/src/app/models/nic.rb
+++ b/src/app/models/nic.rb
@@ -24,15 +24,28 @@ class Nic < ActiveRecord::Base
has_and_belongs_to_many :bondings, :join_table => 'bondings_nics'
+ validates_presence_of :mac,
+ :message => 'A mac must be specified.'
+
+ validates_format_of :mac,
+ :with => %r{^([0-9a-fA-F]{2}([:-]|$)){6}$}
+
validates_presence_of :host_id,
:message => 'A host must be specified.'
validates_presence_of :physical_network_id,
:message => 'A network must be specified.'
+ validates_numericality_of :bandwidth,
+ :greater_than_or_equal_to => 0
+
+ # validate 'bridge' or 'usage_type' attribute ?
+
protected
def validate
- if physical_network.boot_type.proto == 'static' and
ip_addresses.size == 0
+ if ! physical_network.nil? and
+ physical_network.boot_type.proto == 'static' and
+ ip_addresses.size == 0
errors.add("physical_network_id",
"is static. Must create at least one static
ip")
end
diff --git a/src/app/models/permission.rb b/src/app/models/permission.rb
index ece3da5..b3929ad 100644
--- a/src/app/models/permission.rb
+++ b/src/app/models/permission.rb
@@ -24,9 +24,12 @@ class Permission < ActiveRecord::Base
has_many :child_permissions, :dependent => :destroy,
:class_name => "Permission", :foreign_key =>
"inherited_from_id"
+ validates_presence_of :pool_id
+ validates_presence_of :uid
validates_uniqueness_of :uid, :scope => [:pool_id, :inherited_from_id]
+
ROLE_SUPER_ADMIN = "Super Admin"
ROLE_ADMIN = "Administrator"
ROLE_USER = "User"
@@ -44,6 +47,10 @@ class Permission < ActiveRecord::Base
ROLE_USER => [PRIV_VIEW, PRIV_VM_CONTROL],
ROLE_MONITOR => [PRIV_VIEW]}
+
+ validates_inclusion_of :user_role,
+ :in => ROLES.keys
+
def self.invert_roles
return_hash = {}
ROLES.each do |role, privs|
diff --git a/src/app/models/pool.rb b/src/app/models/pool.rb
index 614325a..ed767dd 100644
--- a/src/app/models/pool.rb
+++ b/src/app/models/pool.rb
@@ -53,6 +53,9 @@ class Pool < ActiveRecord::Base
validates_presence_of :name
validates_uniqueness_of :name, :scope => :parent_id
+ validates_inclusion_of :type,
+ :in => %w( DirectoryPool HardwarePool VmResourcePool SmartPool )
+
# overloading this method such that we can use permissions.admins to get all
the admins for an object
has_many :permissions, :dependent => :destroy, :order => "id
ASC" do
def super_admins
diff --git a/src/app/models/quota.rb b/src/app/models/quota.rb
index fc52177..2a9e0f0 100644
--- a/src/app/models/quota.rb
+++ b/src/app/models/quota.rb
@@ -22,6 +22,28 @@ require 'util/ovirt'
class Quota < ActiveRecord::Base
belongs_to :pool
+ validates_presence_of :pool_id
+
+
+ validates_numericality_of :total_vcpus,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vcpus.nil? }
+
+ validates_numericality_of :total_vmemory,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vmemory.nil? }
+
+ validates_numericality_of :total_vnics,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vnics.nil? }
+
+ validates_numericality_of :total_storage,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_storage.nil? }
+
+ validates_numericality_of :total_vms,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |quota| quota.total_vms.nil? }
def total_vmemory_in_mb
kb_to_mb(total_vmemory)
diff --git a/src/app/models/smart_pool_tag.rb b/src/app/models/smart_pool_tag.rb
index e135ddf..00bb5a7 100644
--- a/src/app/models/smart_pool_tag.rb
+++ b/src/app/models/smart_pool_tag.rb
@@ -31,6 +31,11 @@ class SmartPoolTag < ActiveRecord::Base
validates_uniqueness_of :smart_pool_id, :scope => [:tagged_id,
:tagged_type]
+ validates_presence_of :tagged_id
+
+ validates_inclusion_of :tagged_type,
+ :in => %w( Pool StoragePool Host Vm )
+
def tagged_type=(sType)
super(sType.to_s.classify.constantize.base_class.to_s)
end
diff --git a/src/app/models/storage_pool.rb b/src/app/models/storage_pool.rb
index bab7031..f9abb55 100644
--- a/src/app/models/storage_pool.rb
+++ b/src/app/models/storage_pool.rb
@@ -39,6 +39,13 @@ class StoragePool < ActiveRecord::Base
validates_presence_of :hardware_pool_id
+ validates_inclusion_of :type,
+ :in => %w( IscsiStoragePool LvmStoragePool NfsStoragePool )
+
+
+ validates_numericality_of :capacity,
+ :greater_than_or_equal_to => 0
+
acts_as_xapian :texts => [ :ip_addr, :target, :export_path, :type ],
:terms => [ [ :search_users, 'U',
"search_users" ] ],
:eager_load => :smart_pools
@@ -54,6 +61,9 @@ class StoragePool < ActiveRecord::Base
STATE_PENDING_DELETION = "pending_deletion"
STATE_AVAILABLE = "available"
+ validates_inclusion_of :state,
+ :in => [ STATE_PENDING_SETUP, STATE_PENDING_DELETION, STATE_AVAILABLE]
+
def self.factory(type, params = {})
params[:state] = STATE_PENDING_SETUP unless params[:state]
case type
@@ -121,4 +131,11 @@ class StoragePool < ActiveRecord::Base
end
return_hash
end
+
+ def movable?
+ storage_volumes.each{ |x|
+ return false unless x.movable?
+ }
+ return true
+ end
end
diff --git a/src/app/models/storage_volume.rb b/src/app/models/storage_volume.rb
index 39b72d5..f8bd4fb 100644
--- a/src/app/models/storage_volume.rb
+++ b/src/app/models/storage_volume.rb
@@ -32,10 +32,23 @@ class StorageVolume < ActiveRecord::Base
end
end
+ validates_presence_of :storage_pool_id
+
+
+ validates_numericality_of :size,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new { |storage_volume| storage_volume.nil? }
+
+ validates_inclusion_of :type,
+ :in => %w( IscsiStorageVolume LvmStorageVolume NfsStorageVolume )
+
STATE_PENDING_SETUP = "pending_setup"
STATE_PENDING_DELETION = "pending_deletion"
STATE_AVAILABLE = "available"
+ validates_inclusion_of :state,
+ :in => [ STATE_PENDING_SETUP, STATE_PENDING_DELETION, STATE_AVAILABLE]
+
def self.factory(type, params = {})
params[:state] = STATE_PENDING_SETUP unless params[:state]
case type
@@ -131,4 +144,12 @@ class StorageVolume < ActiveRecord::Base
return_hash
end
+ def movable?
+ if vms.size > 0 or
+ (not lvm_storage_pool.nil? and not lvm_stoage_pool.movable?)
+ return false
+ end
+ return true
+ end
+
end
diff --git a/src/app/models/task.rb b/src/app/models/task.rb
index dd24e6a..17004df 100644
--- a/src/app/models/task.rb
+++ b/src/app/models/task.rb
@@ -45,6 +45,17 @@ class Task < ActiveRecord::Base
COMPLETED_STATES = [STATE_FINISHED, STATE_FAILED, STATE_CANCELED]
WORKING_STATES = [STATE_QUEUED, STATE_RUNNING, STATE_PAUSED]
+ validates_inclusion_of :type,
+ :in => %w( HostTask StorageTask StorageVolumeTask VmTask )
+
+ validates_inclusion_of :state,
+ :in => COMPLETED_STATES + WORKING_STATES
+
+ # FIXME validate action depending on type / subclass
+ # validate task_target_id, task_type_id, arg, message
+ # depending on subclass, action, state
+
+
TASK_TYPES_OPTIONS = [["VM Tasks", "VmTask"],
["Host Tasks", "HostTask"],
["Storage Tasks", "StorageTask"],
@@ -89,4 +100,9 @@ class Task < ActiveRecord::Base
ret_val += " #{args}" if args
ret_val
end
+
+ def validate
+ errors.add("time_ended", "Tasks ends before its
started") unless time_ended.nil? or time_started.nil? or time_ended >
time_started
+ errors.add("time_started", "Tasks starts before its
created") unless time_started.nil? or created_at.nil? or time_started >
created_at
+ end
end
diff --git a/src/app/models/usage.rb b/src/app/models/usage.rb
index 353e8f4..59b0e48 100644
--- a/src/app/models/usage.rb
+++ b/src/app/models/usage.rb
@@ -18,4 +18,10 @@
class Usage < ActiveRecord::Base
has_and_belongs_to_many :networks, :join_table =>
'networks_usages'
+
+ validates_presence_of :label
+ validates_presence_of :usage
+
+ validates_uniqueness_of :usage
+
end
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index 227f343..bf99e2d 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -34,7 +34,32 @@ class Vm < ActiveRecord::Base
validates_presence_of :uuid, :description, :num_vcpus_allocated,
:boot_device, :memory_allocated_in_mb,
- :memory_allocated, :vnic_mac_addr
+ :memory_allocated, :vnic_mac_addr,
+ :vm_resource_pool_id
+
+ validates_format_of :uuid,
+ :with =>
%r([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})
+
+ validates_numericality_of :needs_restart,
+ :greater_than_or_equal_to => 0,
+ :less_than_or_equal_to => 1,
+ :unless => Proc.new{ |vm| vm.needs_restart.nil? }
+
+ validates_numericality_of :memory_used,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.memory_used.nil? }
+
+ validates_numericality_of :vnc_port,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.vnc_port.nil? }
+
+ validates_numericality_of :num_vcpus_allocated,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.num_vcpus_allocated.nil? }
+
+ validates_numericality_of :memory_allocated,
+ :greater_than_or_equal_to => 0,
+ :unless => Proc.new{ |vm| vm.memory_allocated.nil? }
acts_as_xapian :texts => [ :uuid, :description, :vnic_mac_addr, :state ],
:terms => [ [ :search_users, 'U',
"search_users" ] ],
@@ -117,9 +142,14 @@ class Vm < ActiveRecord::Base
STATE_SAVED => STATE_SAVED,
STATE_RESTORING => STATE_RUNNING,
STATE_MIGRATING => STATE_RUNNING,
- STATE_CREATE_FAILED => STATE_CREATE_FAILED}
+ STATE_CREATE_FAILED => STATE_CREATE_FAILED,
+ STATE_INVALID => STATE_INVALID}
TASK_STATE_TRANSITIONS = []
+ validates_inclusion_of :state,
+ :in => EFFECTIVE_STATE.keys
+
+
def get_hardware_pool
pool = vm_resource_pool
pool = pool.get_hardware_pool if pool
@@ -311,7 +341,7 @@ class Vm < ActiveRecord::Base
# FIXME: what should memory min/max be?
errors.add("memory_allocated_in_mb", "must be at least 256
MB") unless not(memory_allocated_in_mb) or memory_allocated_in_mb >=256
# FIXME: what should cpu min/max
- errors.add("num_vcpus_allocated", "must be between 1 and
16") unless (num_vcpus_allocated >=1 and num_vcpus_allocated <= 16)
+ errors.add("num_vcpus_allocated", "must be between 1 and
16") unless (num_vcpus_allocated.nil? or (num_vcpus_allocated >=1 and
num_vcpus_allocated <= 16))
errors.add("memory_allocated_in_mb", "violates quota")
unless not(memory_allocated) or resources[:memory].nil? or memory_allocated
<= resources[:memory]
errors.add("num_vcpus_allocated", "violates quota")
unless not(num_vcpus_allocated) or resources[:cpus].nil? or num_vcpus_allocated
<= resources[:cpus]
errors.add_to_base("No available nics in quota") unless
resources[:nics].nil? or resources[:nics] >= 1
diff --git a/src/host-browser/host-browser.rb b/src/host-browser/host-browser.rb
index 52d2999..852d6bf 100755
--- a/src/host-browser/host-browser.rb
+++ b/src/host-browser/host-browser.rb
@@ -250,13 +250,13 @@ class HostBrowser
puts "Saving new CPU records"
cpu_info.collect do |cpu|
detail = Cpu.new(
- "cpu_number" => cpu['CPUNUM'],
- "core_number" => cpu['CORENUM]'],
- "number_of_cores" => cpu['NUMCORES'],
+ "cpu_number" => cpu['CPUNUM'].to_i,
+ "core_number" => cpu['CORENUM]'].to_i,
+ "number_of_cores" => cpu['NUMCORES'].to_i,
"vendor" => cpu['VENDOR'],
"model" => cpu['MODEL'],
"family" => cpu['FAMILY'],
- "cpuid_level" => cpu['CPUIDLVL'],
+ "cpuid_level" => cpu['CPUIDLVL'].to_i,
"speed" => cpu['SPEED'],
"cache" => cpu['CACHE'],
"flags" => cpu['FLAGS'])
@@ -290,7 +290,7 @@ class HostBrowser
updated_nic = Nic.find_by_id(nic.id)
- updated_nic.bandwidth = detail['BANDWIDTH']
+ updated_nic.bandwidth = detail['BANDWIDTH'].to_i
updated_nic.save!
found=true
@@ -310,7 +310,7 @@ class HostBrowser
puts "Creating a new nic..."
detail = Nic.new(
'mac' => nic['MAC'].upcase,
- 'bandwidth' => nic['BANDWIDTH'],
+ 'bandwidth' => nic['BANDWIDTH'].to_i,
'usage_type' => 1)
detail.physical_network = network
diff --git a/src/test/fixtures/help_sections.yml
b/src/test/fixtures/help_sections.yml
index 5bf0293..d9f93e4 100644
--- a/src/test/fixtures/help_sections.yml
+++ b/src/test/fixtures/help_sections.yml
@@ -1,7 +1,11 @@
# Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html
-# one:
-# column: value
-#
-# two:
-# column: value
+dashboard_help:
+ controller: dashboard
+ action: index
+ section: 'index.html'
+
+hardware_hosts_help:
+ controller: hardware
+ action: hosts
+ section: 'hardware_hosts.html'
diff --git a/src/test/fixtures/hosts.yml b/src/test/fixtures/hosts.yml
index 31e7580..4b97053 100644
--- a/src/test/fixtures/hosts.yml
+++ b/src/test/fixtures/hosts.yml
@@ -5,7 +5,7 @@ prod_corp_com:
memory: 18384
is_disabled: 0
state: available
- hypervisor_type: xen
+ hypervisor_type: KVM
hardware_pool: corp_com
workstation_corp_com:
uuid: 1f2a8694-961d-11dc-9387-001558c41534
@@ -13,7 +13,8 @@ workstation_corp_com:
arch: i386
memory: 2048
is_disabled: 0
- hypervisor_type: qemu
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com
macworkstation_qa_corp_com:
uuid: 58a85f44-75fd-4934-805f-88e45b40d4b4
@@ -21,7 +22,8 @@ macworkstation_qa_corp_com:
arch: mips
memory: 2048
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
fedoraworkstation_qa_corp_com:
uuid: 520bbb34-6515-490e-9d07-0c8b14f76805
@@ -29,7 +31,8 @@ fedoraworkstation_qa_corp_com:
arch: i386
memory: 2048
is_disabled: 1
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
pipeline_qa_corp_com:
uuid: 2e422f66-324e-48d4-973f-0b91b33070f9
@@ -37,7 +40,8 @@ pipeline_qa_corp_com:
arch: xeon
memory: 1048576
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
app_qa_corp_com:
uuid: bb0ce7c7-f234-49ae-84b5-6f4fd8bddcaa
@@ -45,7 +49,8 @@ app_qa_corp_com:
arch: xeon
memory: 16384
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
issue_qa_corp_com:
uuid: ec0d86de-657b-48f6-b7cc-e733a3f9a834
@@ -53,7 +58,8 @@ issue_qa_corp_com:
arch: x86
memory: 4096
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_qa
db_dev_corp_com:
uuid: 81c15560-dbf4-45f5-9b75-106cdbd63aeb
@@ -61,7 +67,8 @@ db_dev_corp_com:
arch: x86
memory: 4096
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_dev
mystation_dev_corp_com:
uuid: 6ae3d22e-97e0-4d86-9712-5395b20a0f45
@@ -69,7 +76,8 @@ mystation_dev_corp_com:
arch: i686
memory: 2048
is_disabled: 0
- hypervisor_type: xen
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_dev
node3_priv_ovirt_org:
uuid: 75f8d5f3-faf0-4308-9f20-e4b03d013a27
@@ -77,16 +85,17 @@ node3_priv_ovirt_org:
arch: x86_64
memory: 4194304
is_disabled: 0
- hypervisor_type: kvm
- hardware_pool: default
state: available
+ hypervisor_type: KVM
+ hardware_pool: default
mailservers_managed_node:
uuid: 182a8596-961d-11dc-9387-001558c41534
hostname: mail.mynetwork.com
arch: i386
memory: 16384
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
fileserver_managed_node:
uuid: 928ad8172-9723-11dc-9387-001558c41534
@@ -94,7 +103,8 @@ fileserver_managed_node:
arch: x86_64
memory: 32768
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
ldapserver_managed_node:
uuid: 919ae372-9156-11dc-9387-001558c41534
@@ -102,7 +112,8 @@ ldapserver_managed_node:
arch: i386
memory: 16384
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
buildserver_managed_node:
uuid: 6293acd9-2784-11dc-9387-001558c41534
@@ -110,7 +121,8 @@ buildserver_managed_node:
arch: x86_64
memory: 65536
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
mediaserver_managed_node:
uuid: 6293acd9-2784-11dc-9387-001558c41534
@@ -118,5 +130,6 @@ mediaserver_managed_node:
arch: x86_64
memory: 65536
is_disabled: 0
- hypervisor_type: kvm
+ state: available
+ hypervisor_type: KVM
hardware_pool: corp_com_prod
diff --git a/src/test/fixtures/storage_pools.yml
b/src/test/fixtures/storage_pools.yml
index ac72f99..35bfc88 100644
--- a/src/test/fixtures/storage_pools.yml
+++ b/src/test/fixtures/storage_pools.yml
@@ -5,14 +5,25 @@ corp_com_ovirtpriv_storage:
port: 3260
target: ovirtpriv:storage
state: available
+ capacity: 1024
corp_com_nfs_ovirtnfs:
hardware_pool: corp_com
type: NfsStoragePool
ip_addr: 192.168.50.2
export_path: /ovirtnfs
+ capacity: 1024
+ state: available
corp_com_dev_nfs_ovirtnfs:
hardware_pool: corp_com_dev
type: NfsStoragePool
ip_addr: 192.168.50.3
export_path: /devnfs
state: available
+ capacity: 1024
+corp_com_dev_lvm_ovirtlvm:
+ hardware_pool: corp_com_dev
+ type: LvmStoragePool
+ ip_addr: 192.168.50.3
+ export_path: /devnfs
+ state: available
+ capacity: 1024
diff --git a/src/test/fixtures/usages.yml b/src/test/fixtures/usages.yml
index 5bf0293..3bd72e6 100644
--- a/src/test/fixtures/usages.yml
+++ b/src/test/fixtures/usages.yml
@@ -1,7 +1,9 @@
# Read about fixtures at http://ar.rubyonrails.org/classes/Fixtures.html
-# one:
-# column: value
-#
-# two:
-# column: value
+management_usage:
+ label: management
+ usage: management
+
+guest_usage:
+ label: guest
+ usage: guest
diff --git a/src/test/functional/resources_controller_test.rb
b/src/test/functional/resources_controller_test.rb
index 8e3989b..0312c0c 100644
--- a/src/test/functional/resources_controller_test.rb
+++ b/src/test/functional/resources_controller_test.rb
@@ -45,7 +45,7 @@ class ResourcesControllerTest < ActionController::TestCase
def test_create
assert_difference('VmResourcePool.count') do
- post :create, :vm_resource_pool => { :name =>
'foo_resource_pool' }, :parent_id => pools(:default).id
+ post :create, :pool => { :name => 'foo_resource_pool'},
:parent_id => pools(:default).id
end
assert_response :success
diff --git a/src/test/functional/storage_controller_test.rb
b/src/test/functional/storage_controller_test.rb
index a448fdf..f08fe1b 100644
--- a/src/test/functional/storage_controller_test.rb
+++ b/src/test/functional/storage_controller_test.rb
@@ -69,11 +69,11 @@ class StorageControllerTest < Test::Unit::TestCase
assert_not_nil assigns(:storage_pools)
end
- def test_create
+ def test_create_storage_controller
hw_pool = HardwarePool.get_default_pool
num_storage_volumes = StoragePool.count
- post :create, :storage_type => 'NFS', :storage_pool => {
:hardware_pool => hw_pool, :ip_addr => '111.121.131.141',
:export_path => '/tmp/path' }
+ post :create, :storage_type => 'NFS', :storage_pool => {
:hardware_pool => hw_pool, :ip_addr => '111.121.131.141',
:export_path => '/tmp/path', :state => 'available',
:capacity => 1024 }
assert_response :success
diff --git a/src/test/unit/bonding_test.rb b/src/test/unit/bonding_test.rb
index 8b90cd5..33bfd56 100644
--- a/src/test/unit/bonding_test.rb
+++ b/src/test/unit/bonding_test.rb
@@ -35,6 +35,7 @@ class BondingTest < ActiveSupport::TestCase
:bonding_type_id => bonding_types(:failover_bonding_type),
:host_id => hosts(:mailservers_managed_node))
@bonding.vlan = networks(:dhcp_vlan_one)
+ @bonding.bonding_type = bonding_types(:load_balancing_bonding_type)
end
# Ensures that the name is required.
@@ -69,6 +70,40 @@ class BondingTest < ActiveSupport::TestCase
flunk 'Bondings have to have a host.' if @bonding.valid?
end
+ def test_valid_fails_without_bonding_type
+ @bonding.bonding_type = nil
+ flunk 'Bonding must specify bonding type' if @bonding.valid?
+ end
+
+ def test_valid_fails_without_vlan
+ @bonding.vlan = nil
+ flunk 'Bonding must specify vlan' if @bonding.valid?
+ end
+
+ # Ensures that an arp ping address must have the correct format
+ #
+ def test_valid_fails_with_bad_arp_ping_address
+ @bonding.arp_ping_address = 'foobar'
+
+ flunk "An arp ping address must be in the format ##.##.##.##." if
@bonding.valid?
+ end
+
+ # Ensures that an arp interval is a numerical value >= 0
+ #
+ def test_valid_fails_with_bad_arp_interval
+ @bonding.arp_interval = -1
+
+ flunk "An arp interval must be >= 0" if @bonding.valid?
+ end
+
+ def test_static_network_bonding_must_have_ip
+ @bonding.vlan = networks(:static_vlan_one)
+ @bonding.ip_addresses.delete_if { true }
+
+ flunk 'Bonding assigned to static networks must have at least one
ip' if @bonding.valid?
+ end
+
+
# Ensure that retrieving a bonding returns its associated objects.
#
def test_find_all_for_host
diff --git a/src/test/unit/boot_type_test.rb b/src/test/unit/boot_type_test.rb
index a30e258..778f75a 100644
--- a/src/test/unit/boot_type_test.rb
+++ b/src/test/unit/boot_type_test.rb
@@ -19,8 +19,36 @@
require File.dirname(__FILE__) + '/../test_helper'
class BootTypeTest < ActiveSupport::TestCase
- # Replace this with your real tests.
- def test_truth
- assert true
- end
+ fixtures :boot_types
+
+ def setup
+ @boot_type = BootType.new(
+ :label => 'TestBootType',
+ :proto => 'static' )
+ end
+
+ def test_valid_fails_without_label
+ @boot_type.label = ''
+
+ flunk 'Boot type must have label' if @boot_type.valid?
+ end
+
+ def test_valid_fails_without_unique_label
+ @boot_type.label = 'Static IP'
+
+ flunk 'Boot type must have unique label' if @boot_type.valid?
+ end
+
+ def test_valid_fails_with_bad_proto
+ @boot_type.proto = 'foobar'
+
+ flunk 'Boot type must have valid proto' if @boot_type.valid?
+ end
+
+ def test_find_all_for_boot_type
+ result = BootType.find(:all)
+
+ assert_equal 3, result.size, "Did not find right number of boot
types"
+ end
+
end
diff --git a/src/test/unit/cpu_test.rb b/src/test/unit/cpu_test.rb
index 9558525..56e2d5d 100644
--- a/src/test/unit/cpu_test.rb
+++ b/src/test/unit/cpu_test.rb
@@ -1,10 +1,99 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi <mmorsi 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 CpuTest < ActiveSupport::TestCase
fixtures :cpus
+ fixtures :hosts
+
+ def setup
+ @cpu = Cpu.new(
+ :cpu_number => 3,
+ :core_number => 1,
+ :number_of_cores => 1,
+ :cpuid_level => 0,
+ :speed => 2048,
+ :vendor => 'foo',
+ :model => 'bar',
+ :family => 'alpha')
+ @cpu.host = hosts(:prod_corp_com)
+ end
+
+ def test_find_cpus
+ assert_equal Cpu.find(:all).size, 8, "Failure retrieving all
cpus"
+
+ result = Cpu.find(cpus(:prod_corp_com_1).id)
+ assert_equal result.host.id, hosts(:prod_corp_com).id
+ end
+
+ def test_valid_without_host
+ @cpu.host = nil
+
+ flunk "CPUs must be associated with hosts" if @cpu.valid?
+ end
+
+ def test_valid_with_bad_cpu_numer
+ @cpu.cpu_number = -1
+
+ flunk "cpu number must be >= 0" if @cpu.valid?
+ end
+
+ def test_valid_with_bad_core_number
+ @cpu.core_number = -1
+
+ flunk "cpu core number must be >= 0" if @cpu.valid?
+ end
+
+ def test_valid_with_bad_number_of_cores
+ @cpu.number_of_cores = 0
- # Replace this with your real tests.
- def test_truth
- assert true
+ flunk "cpu number of cores must be > 0" if @cpu.valid?
end
+
+ def test_valid_with_bad_cpuid_level
+ @cpu.cpuid_level = -1
+
+ flunk "cpu id level must be >= 0" if @cpu.valid?
+ end
+
+ def test_valid_with_bad_speed
+ @cpu.speed = 0
+
+ flunk "cpu speed must be > 0" if @cpu.valid?
+ end
+
+ def test_valid_without_vendor
+ @cpu.vendor = ''
+
+ flunk "cpu vendor must be specified" if @cpu.valid?
+ end
+
+ def test_valid_without_model
+ @cpu.model = ''
+
+ flunk "cpu model must be specified" if @cpu.valid?
+ end
+
+ def test_valid_without_family
+ @cpu.family = ''
+
+ flunk "cpu family must be specified" if @cpu.valid?
+ end
+
end
diff --git a/src/test/unit/help_section_test.rb
b/src/test/unit/help_section_test.rb
index a0e6a08..1596262 100644
--- a/src/test/unit/help_section_test.rb
+++ b/src/test/unit/help_section_test.rb
@@ -1,8 +1,51 @@
+# Copyright (C) 2008 Red Hat, Inc.
+# Written by Mohammed Morsi <mmorsi 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 'test_helper'
class HelpSectionTest < ActiveSupport::TestCase
- # Replace this with your real tests.
- def test_truth
- assert true
- end
+ fixtures :help_sections
+
+ def setup
+ @help_section = HelpSection.new(
+ :controller => 'foo',
+ :action => 'bar',
+ :section => 'foobar')
+ end
+
+ def test_unique_controller_action
+ @help_section.controller = 'dashboard'
+ @help_section.action = 'index'
+
+ flunk 'help section must have unique controller / action' if
@help_section.valid?
+ end
+
+ def test_valid_fails_without_controller
+ @help_section.controller = ''
+ flunk 'help section controller must be specified' if
@help_section.valid?
+ end
+
+ def test_valid_fails_without_action
+ @help_section.action = ''
+ flunk 'help section action must be specified' if
@help_section.valid?
+ end
+
+ def test_valid_fails_without_section
+ @help_section.section = ''
+ flunk 'help section section must be specified' if
@help_section.valid?
+ end
end
diff --git a/src/test/unit/host_test.rb b/src/test/unit/host_test.rb
index a4ead5d..338fbaf 100644
--- a/src/test/unit/host_test.rb
+++ b/src/test/unit/host_test.rb
@@ -21,9 +21,63 @@ require File.dirname(__FILE__) + '/../test_helper'
class HostTest < Test::Unit::TestCase
fixtures :hosts
+ fixtures :pools
+ fixtures :vms
- # Replace this with your real tests.
- def test_truth
- assert true
+ def setup
+ @host = Host.new(
+ :uuid => 'foobar',
+ :hostname => 'foobar',
+ :arch => 'x86_64',
+ :hypervisor_type => 'KVM',
+ :state => 'available')
+
+ @host.hardware_pool = pools(:corp_com)
+ end
+
+ def test_valid_fails_without_hardware_pool
+ @host.hardware_pool = nil
+
+ flunk "Hosts must be associated w/ a hardware pool" if
@host.valid?
+ end
+
+ def test_valid_without_uuid
+ @host.uuid = nil
+
+ flunk "Hosts don't need to be associated w/ a uuid" unless
@host.valid?
end
+
+
+ def test_valid_fails_without_hostname
+ @host.hostname = ''
+
+ flunk "Hosts must be associated w/ a hostname" if @host.valid?
+ end
+
+
+ def test_valid_fails_without_arch
+ @host.arch = ''
+
+ flunk "Hosts must be associated w/ an arch" if @host.valid?
+ end
+
+ def test_valid_fails_with_bad_hypervisor_type
+ @host.hypervisor_type = 'foobar'
+
+ flunk "Hosts must be associated w/ a valid hypervisor type" if
@host.valid?
+ end
+
+ def test_valid_fails_with_bad_state
+ @host.state = 'foobar'
+
+ flunk "Hosts must be associated w/ a valid state" if
@host.valid?
+ end
+
+ def test_host_movable
+ assert_equal @host.movable?, true, "Hosts are movable unless
associated w/ vms"
+
+ @host.vms << vms(:production_httpd_vm)
+ assert_equal @host.movable?, false, "Hosts with associated vms are
not movable"
+ end
+
end
diff --git a/src/test/unit/ip_address_test.rb b/src/test/unit/ip_address_test.rb
index 5ecff77..4cfc694 100644
--- a/src/test/unit/ip_address_test.rb
+++ b/src/test/unit/ip_address_test.rb
@@ -10,4 +10,9 @@ class IpAddressTest < ActiveSupport::TestCase
def test_can_get_ipaddress_object
assert_equal ip_addresses(:ip_v4_mailserver_nic_one).address,
'172.31.0.15'
end
+
+ def test_valid_fails_without_foreign_entity
+ @ip_address = IpAddress.new
+ flunk "Ip Address must be associated with network, nic, or
bonding" if @ip_address.valid?
+ end
end
diff --git a/src/test/unit/ip_v4_address_test.rb
b/src/test/unit/ip_v4_address_test.rb
index 65a08d3..9cb7362 100644
--- a/src/test/unit/ip_v4_address_test.rb
+++ b/src/test/unit/ip_v4_address_test.rb
@@ -24,7 +24,8 @@ class IpV4AddressTest < ActiveSupport::TestCase
@address = IpV4Address.new(:address => '192.168.50.2',
:netmask => '255.255.255.0',
:gateway => '192.168.50.1',
- :broadcast => '192.168.50.255')
+ :broadcast => '192.168.50.255',
+ :nic_id => 1)
end
# Ensures that an address must be supplied.
diff --git a/src/test/unit/ip_v6_address_test.rb
b/src/test/unit/ip_v6_address_test.rb
index 070f407..96f212a 100644
--- a/src/test/unit/ip_v6_address_test.rb
+++ b/src/test/unit/ip_v6_address_test.rb
@@ -23,7 +23,8 @@ 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')
+ :prefix =>
'0000:0000:0000:0000:1234:1234:1234:1234',
+ :nic_id => 1)
end
# Ensures that the address must be provided.
diff --git a/src/test/unit/nic_test.rb b/src/test/unit/nic_test.rb
index 1de1e00..2644148 100644
--- a/src/test/unit/nic_test.rb
+++ b/src/test/unit/nic_test.rb
@@ -22,9 +22,61 @@ require File.dirname(__FILE__) + '/../test_helper'
class NicTest < Test::Unit::TestCase
fixtures :ip_addresses
fixtures :nics
+ fixtures :hosts
+ fixtures :networks
- # Replace this with your real tests.
- def test_truth
- assert true
+ def setup
+ @nic = Nic.new(
+ :mac => '00:11:22:33:44:55',
+ :usage_type => 1,
+ :bandwidth => 100 )
+ @nic.host = hosts(:prod_corp_com)
+ @nic.physical_network = networks(:static_physical_network_one)
+
+ @ip_address = IpV4Address.new(
+ :address => '1.2.3.4',
+ :netmask => '2.3.4.5',
+ :gateway => '3.4.5.6',
+ :broadcast => '4.5.6.7' )
+
+ @nic.ip_addresses << @ip_address
+ end
+
+ def test_valid_fails_without_mac
+ @nic.mac = ''
+
+ flunk 'Nic must have a mac' if @nic.valid?
+ end
+
+ def test_valid_fails_with_invalid_mac
+ @nic.mac = 'foobar'
+
+ flunk 'Nic must have a valid mac' if @nic.valid?
+ end
+
+ def test_valid_fails_without_host
+ @nic.host = nil
+
+ flunk 'Nic must have a host' if @nic.valid?
end
+
+ def test_valid_fails_without_physical_network
+ @nic.physical_network = nil
+
+ flunk 'Nic must have a physical network' if @nic.valid?
+ end
+
+ def test_valid_fails_with_invalid_bandwidth
+ @nic.bandwidth = -1
+
+ flunk 'Nic bandwidth must be >= 0' if @nic.valid?
+ end
+
+ def test_static_network_nic_must_have_ip
+ @nic.physical_network = networks(:static_physical_network_one)
+ @nic.ip_addresses.delete_if { true }
+
+ flunk 'Nics assigned to static networks must have at least one ip'
if @nic.valid?
+ end
+
end
diff --git a/src/test/unit/permission_test.rb b/src/test/unit/permission_test.rb
index 5e2c7ba..0c0f4c7 100644
--- a/src/test/unit/permission_test.rb
+++ b/src/test/unit/permission_test.rb
@@ -21,6 +21,14 @@ require File.dirname(__FILE__) + '/../test_helper'
class PermissionTest < Test::Unit::TestCase
fixtures :permissions
+ fixtures :pools
+
+ def setup
+ @permission = Permission.new(
+ :uid => 'foobar',
+ :user_role => 'Super Admin' )
+ @permission.pool = pools(:root_dir_pool)
+ end
# Replace this with your real tests.
def test_simple_permission
@@ -32,4 +40,24 @@ class PermissionTest < Test::Unit::TestCase
assert_equal permissions(:ovirtadmin_default).inherited_from_id,
permissions(:ovirtadmin_root).id
assert_equal permissions(:ovirtadmin_default).parent_permission,
permissions(:ovirtadmin_root)
end
+
+ def test_valid_fails_without_pool
+ @permission.pool = nil
+ flunk 'Permission must specify pool' if @permission.valid?
+ end
+
+ def test_valid_fails_without_uid
+ @permission.uid = ''
+ flunk 'Permission must specify uid' if @permission.valid?
+ end
+
+ def test_valid_fails_without_user_role
+ @permission.user_role = ''
+ flunk 'Permission must specify user role' if @permission.valid?
+ end
+
+ def test_valid_fails_with_invalid_user_role
+ @permission.user_role = 'foobar'
+ flunk 'Permission must specify valid user role' if
@permission.valid?
+ end
end
diff --git a/src/test/unit/pool_test.rb b/src/test/unit/pool_test.rb
index 6f20f9f..c9a5554 100644
--- a/src/test/unit/pool_test.rb
+++ b/src/test/unit/pool_test.rb
@@ -22,6 +22,12 @@ require File.dirname(__FILE__) + '/../test_helper'
class PoolTest < Test::Unit::TestCase
fixtures :pools
+ def setup
+ @pool = Pool.new(
+ :name => 'foobar',
+ :type => 'DirectoryPool' )
+ end
+
# Replace this with your real tests.
def test_get_name
assert_equal(pools(:corp_com_prod).name, 'Production Operations')
@@ -30,4 +36,19 @@ class PoolTest < Test::Unit::TestCase
def test_get_parent
assert_equal(pools(:corp_com_prod).parent.name, 'corp.com')
end
+
+ def test_valid_fails_without_name
+ @pool.name = ''
+ flunk "Pool must specify name" if @pool.valid?
+ end
+
+ def test_valid_fails_without_unique_name
+ @pool.name = 'root'
+ flunk "Pool must specify unique name" if @pool.valid?
+ end
+
+ def test_valid_fails_with_invalid_type
+ @pool.name = 'foobar'
+ flunk "Pool must specify valid type" if @pool.valid?
+ end
end
diff --git a/src/test/unit/quota_test.rb b/src/test/unit/quota_test.rb
index ac0fae0..5903cc8 100644
--- a/src/test/unit/quota_test.rb
+++ b/src/test/unit/quota_test.rb
@@ -21,9 +21,40 @@ require File.dirname(__FILE__) + '/../test_helper'
class QuotaTest < Test::Unit::TestCase
fixtures :quotas
+ fixtures :pools
- # Replace this with your real tests.
- def test_truth
- assert true
+ def setup
+ @quota = Quota.new
+ @quota.pool = pools(:root_dir_pool)
+ end
+
+ def test_valid_fails_without_pool
+ @quota.pool = nil
+ flunk "Quota's must specify pool" if @quota.valid?
+ end
+
+ def test_valid_fails_with_bad_total_vcpus
+ @quota.total_vcpus = -1
+ flunk "Quota must specify valid total vcpus" if @quota.valid?
+ end
+
+ def test_valid_fails_with_bad_total_vmemory
+ @quota.total_vmemory = -1
+ flunk "Quota must specify valid total vmemory" if @quota.valid?
+ end
+
+ def test_valid_fails_with_bad_total_vnics
+ @quota.total_vnics = -1
+ flunk "Quota must specify valid total vnics" if @quota.valid?
+ end
+
+ def test_valid_fails_with_bad_total_storage
+ @quota.total_storage = -1
+ flunk "Quota must specify valid total storage" if @quota.valid?
+ end
+
+ def test_valid_fails_with_bad_total_vms
+ @quota.total_vms = -1
+ flunk "Quota must specify valid total vms" if @quota.valid?
end
end
diff --git a/src/test/unit/storage_pool_test.rb
b/src/test/unit/storage_pool_test.rb
index 8cc8d1b..4e18a8c 100644
--- a/src/test/unit/storage_pool_test.rb
+++ b/src/test/unit/storage_pool_test.rb
@@ -21,8 +21,53 @@ require File.dirname(__FILE__) + '/../test_helper'
class StoragePoolTest < Test::Unit::TestCase
fixtures :storage_pools
+ fixtures :pools
+ fixtures :vms
+
+ def setup
+ @storage_pool = StoragePool.new(
+ :type => 'IscsiStoragePool',
+ :capacity => 100,
+ :state => 'available' )
+ @storage_pool.hardware_pool = pools(:default)
+ end
+
+ def test_valid_fails_without_hardware_pool
+ @storage_pool.hardware_pool = nil
+ flunk "Storage pool must specify hardware pool" if
@storage_pool.valid?
+ end
+
+ def test_valid_fails_with_bad_type
+ @storage_pool.type = 'foobar'
+ flunk 'Storage pool must specify valid type' if
@storage_pool.valid?
+ end
+
+ def test_valid_fails_with_bad_capacity
+ @storage_pool.capacity = -1
+ flunk 'Storage pool must specify valid capacity >= 0' if
@storage_pool.valid?
+ end
+
+ def test_valid_fails_with_bad_state
+ @storage_pool.state = 'foobar'
+ flunk 'Storage pool must specify valid state' if
@storage_pool.valid?
+ end
def test_hardware_pool_relationship
assert_equal 'corp.com',
storage_pools(:corp_com_ovirtpriv_storage).hardware_pool.name
end
+
+ def test_movable
+ assert_equal @storage_pool.movable?, true, "Storage pool without
volumes should be movable"
+
+ storage_volume = StorageVolume.new(
+ :size => 100,
+ :type => 'IscsiStorageVolume',
+ :state => 'available' )
+ @storage_pool.storage_volumes << storage_volume
+
+ assert_equal @storage_pool.movable?, true, "Storage pool w/ movable
storage volumes should be movable"
+
+ storage_volume.vms << vms(:production_httpd_vm)
+ assert_equal @storage_pool.movable?, false, "Storage pool w/ unmovable
storage volumes should not be movable"
+ end
end
diff --git a/src/test/unit/storage_volume_test.rb
b/src/test/unit/storage_volume_test.rb
index d0a89f4..3978685 100644
--- a/src/test/unit/storage_volume_test.rb
+++ b/src/test/unit/storage_volume_test.rb
@@ -21,9 +21,95 @@ require File.dirname(__FILE__) + '/../test_helper'
class StorageVolumeTest < Test::Unit::TestCase
fixtures :storage_volumes
+ fixtures :storage_pools
+ fixtures :vms
+
+ def setup
+ @storage_volume = StorageVolume.new(
+ :size => 100,
+ :type => 'IscsiStorageVolume',
+ :state => 'available' )
+
+ @iscsi_storage_volume = IscsiStorageVolume.new(
+ :lun => 'foobar',
+ :size => 100,
+ :state => 'available' )
+
+ @lvm_storage_volume = LvmStorageVolume.new(
+ :lv_name => 'foobar',
+ :lv_owner_perms => '0700',
+ :lv_group_perms => '0777',
+ :lv_mode_perms => '0000' )
+
+ @storage_volume.storage_pool = storage_pools(:corp_com_ovirtpriv_storage)
+ @iscsi_storage_volume.storage_pool =
storage_pools(:corp_com_ovirtpriv_storage)
+ @lvm_storage_volume.storage_pool =
storage_pools(:corp_com_dev_lvm_ovirtlvm)
+ end
# Replace this with your real tests.
def test_relationship_to_storage_pool
assert_equal 'corp.com',
storage_volumes(:ovirtpriv_storage_lun_1).storage_pool.hardware_pool.name
end
+
+
+ def test_valid_fails_with_bad_size
+ @storage_volume.size = -1
+
+ flunk "Storage volume size must be >= 0" if
@storage_volume.valid?
+ end
+
+ def test_valid_fails_with_bad_type
+ @storage_volume.type = 'foobar'
+
+ flunk "Storage volume type must be valid" if
@storage_volume.valid?
+ end
+
+ def test_valid_fails_with_bad_state
+ @storage_volume.state = 'foobar'
+ flunk "Storage volume state must be valid" if
@storage_volume.valid?
+ end
+
+ def test_valid_fails_without_storage_pool
+ @storage_volume.storage_pool = nil
+
+ flunk "Storage volume must be associated with a storage pool"
if @storage_volume.valid?
+ end
+
+ def test_valid_fails_without_lun
+ @iscsi_storage_volume.lun = ''
+
+ flunk "iscsi storage volume lun must be valid" if
@iscsi_storage_volume.valid?
+ end
+
+ def test_valid_fails_without_lv_name
+ @lvm_storage_volume.lv_name = ''
+
+ flunk "lvm storage volume lv_name must be valid" if
@lvm_storage_volume.valid?
+ end
+
+ def test_valid_fails_without_lv_owner_perms
+ @lvm_storage_volume.lv_owner_perms = ''
+
+ flunk "lvm storage volume lv_owner_perms must be valid" if
@lvm_storage_volume.valid?
+ end
+
+ def test_valid_fails_without_lv_group_perms
+ @lvm_storage_volume.lv_group_perms = ''
+
+ flunk "lvm storage volume lv_group_perms must be valid" if
@lvm_storage_volume.valid?
+ end
+
+ def test_valid_fails_without_lv_model_perms
+ @lvm_storage_volume.lv_mode_perms = ''
+
+ flunk "lvm storage volume lv_model_perms must be valid" if
@lvm_storage_volume.valid?
+ end
+
+ def test_movable
+ assert_equal @storage_volume.movable?, true, "Storage volume without
vms should be movable"
+ @storage_volume.vms << vms(:production_httpd_vm)
+
+ assert_equal @storage_volume.movable?, false, "Storage volume w/ vms
should not be movable"
+ end
+
end
diff --git a/src/test/unit/task_test.rb b/src/test/unit/task_test.rb
index 33a3bd8..ba780e7 100644
--- a/src/test/unit/task_test.rb
+++ b/src/test/unit/task_test.rb
@@ -22,6 +22,20 @@ require File.dirname(__FILE__) + '/../test_helper'
class TaskTest < Test::Unit::TestCase
fixtures :pools, :hosts, :vms, :permissions, :tasks
+ def setup
+ @task = Task.new( :type => 'HostTask', :state =>
'finished' )
+ end
+
+ def test_valid_fails_with_bad_type
+ @task.type = 'foobar'
+ flunk 'Task must specify valid type' if @task.valid?
+ end
+
+ def test_valid_fails_with_bad_state
+ @task.state = 'foobar'
+ flunk 'Task must specify valid state' if @task.valid?
+ end
+
def test_exercise_task_relationships
assert_equal
tasks(:shutdown_production_httpd_appliance_task).task_target.vm_resource_pool.name,
'corp.com production vmpool'
assert_equal
tasks(:shutdown_production_httpd_appliance_task).task_target.host.hostname,
'prod.corp.com'
diff --git a/src/test/unit/usage_test.rb b/src/test/unit/usage_test.rb
index 9a8ec9b..d6c989e 100644
--- a/src/test/unit/usage_test.rb
+++ b/src/test/unit/usage_test.rb
@@ -1,8 +1,47 @@
+# Copyright (C) 2008 Red Hat, Inc.
+#
+# 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 'test_helper'
class UsageTest < ActiveSupport::TestCase
- # Replace this with your real tests.
- def test_truth
- assert true
- end
+ fixtures :usages
+
+ def setup
+ @usage = Usage.new(
+ :label => 'TestUsage',
+ :usage => 'foobar' )
+ end
+
+ def test_valid_fails_without_label
+ @usage.label = ''
+
+ flunk 'Usage must have label' if @usage.valid?
+ end
+
+ def test_valid_fails_without_usage
+ @usage.usage = ''
+
+ flunk 'Usage must have usage' if @usage.valid?
+ end
+
+ def test_valid_fails_without_unique_usage
+ @usage.usage = 'management'
+
+ flunk 'Usage must have unique usage' if @usage.valid?
+ end
+
end
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index a196130..cba3188 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -21,6 +21,7 @@ require File.dirname(__FILE__) + '/../test_helper'
class VmTest < Test::Unit::TestCase
fixtures :vms
+ fixtures :pools
def setup
@vm_name = "Test"
@@ -29,6 +30,69 @@ class VmTest < Test::Unit::TestCase
"#{Vm::IMAGE_PREFIX}@#{Vm::COBBLER_PREFIX}#{Vm::PROVISIONING_DELIMITER}#{@vm_name}"
@cobbler_profile_provisioning
"#{Vm::PROFILE_PREFIX}@#{Vm::COBBLER_PREFIX}#{Vm::PROVISIONING_DELIMITER}#{@vm_name}"
+
+ @vm = Vm.new(
+ :uuid => '00000000-1111-2222-3333-444444444444',
+ :description => 'foobar',
+ :num_vcpus_allocated => 1,
+ :boot_device => 'hd',
+ :memory_allocated_in_mb => 1,
+ :memory_allocated => 1024,
+ :vnic_mac_addr => '11:22:33:44:55:66')
+
+ @vm.vm_resource_pool = pools(:corp_com_production_vmpool)
+ end
+
+ def test_valid_fails_with_bad_uuid
+ @vm.uuid = 'foobar'
+ flunk "Vm must specify valid uuid" if @vm.valid?
+ end
+
+ def test_valid_fails_without_description
+ @vm.description = ''
+ flunk 'Vm must specify description' if @vm.valid?
+ end
+
+ def test_valid_fails_without_num_vcpus_allocated
+ @vm.num_vcpus_allocated = nil
+ flunk 'Vm must specify num_vcpus_allocated' if @vm.valid?
+ end
+
+ def test_valid_fails_without_boot_device
+ @vm.boot_device = ''
+ flunk 'Vm must specify boot_device' if @vm.valid?
+ end
+
+
+ def test_valid_fails_without_memory_allocated
+ @vm.memory_allocated = ''
+ flunk 'Vm must specify memory_allocated' if @vm.valid?
+ end
+
+
+ def test_valid_fails_without_memory_allocated_in_mb
+ @vm.memory_allocated_in_mb = ''
+ flunk 'Vm must specify memory_allocated_in_mb' if @vm.valid?
+ end
+
+ def test_valid_fails_without_vnic_mac_addr
+ @vm.vnic_mac_addr = ''
+ flunk 'Vm must specify vnic_mac_addr' if @vm.valid?
+ end
+
+ def test_valid_fails_without_vm_resources_pool_id
+ @vm.vm_resource_pool_id = ''
+ flunk 'Vm must specify vm_resources_pool_id' if @vm.valid?
+ end
+
+ def test_valid_fails_with_bad_needs_restart
+ @vm.needs_restart = 5
+ flunk 'Vm must specify valid needs_restart' if @vm.valid?
+ end
+
+ def test_valid_fails_with_bad_state
+ @vm.state = 'foobar'
+ flunk 'Vm must specify valid state' if @vm.valid?
end
# Ensures that, if the VM does not contain the Cobbler prefix, that it
--
1.6.0.6
Steve Linabery
2009-Jan-19 22:24 UTC
[Ovirt-devel] Re: [PATCH server] comprehensive / up to date validations patch (revised again)
On Thu, Jan 15, 2009 at 03:21:06PM -0500, Mohammed Morsi wrote:> (revisions relate to small validation bugs in host, nic, task models > and host-browser module) >snip> > # Ensures that, if the VM does not contain the Cobbler prefix, that it > -- > 1.6.0.6 >Line 379, maybe say "MAC address" just for clarity's sake in output if it fails: + :message => 'A mac must be specified.' Typo at line 569: + (not lvm_storage_pool.nil? and not lvm_stoage_pool.movable?) and 606-607 should read "Task ends before it's started" (grammar police): + errors.add("time_ended", "Tasks ends before its started") unless time_ended.nil? or time_started.nil? or time_ended > time_started + errors.add("time_started", "Tasks starts before its created") unless time_started.nil? or created_at.nil? or time_started > created_at Inconsistent indentation in boot_type_test.rb...and elsewhere...(I told you it was minor nitpicky stuff :) So, conditional ACK, but as we discussed, a few times I found myself thinking "Well, that validation *seems* to make sense, but I don't know the model well enough to be 100% certain." But it works and all unit tests passed. --Steve