[Ovirt-devel] [PATCH server] Add svc_vm_actions method to VmService.
Jason Guiditta
jguiditt at redhat.com
Mon Jun 29 17:14:18 UTC 2009
Also includes a couple minor model changes to support better
error reporting.
This method was added so that a client could submit multiple
vms to the service layer in one call and get back appropriate
messages showing any failures without those failures also causing
the entire call to fail, or forcing the client to implement handling
(begin/rescue blocks) of these errors to continue through the
submitted vm list.
Signed-off-by: Jason Guiditta <jguiditt at redhat.com>
---
src/app/models/vm.rb | 7 ++
src/app/models/vm_task.rb | 4 +-
src/app/services/service_module_helper.rb | 34 +++++++
src/app/services/vm_service.rb | 33 ++++++-
src/db-omatic/db_omatic.rb | 90 +++++++++++++-----
src/test/fixtures/permissions.yml | 6 +-
src/test/fixtures/vms.yml | 13 +++
src/test/unit/vm_service_test.rb | 149 +++++++++++++++++++++++++++++
src/test/unit/vm_task_test.rb | 38 ++++++++
src/test/unit/vm_test.rb | 23 ++++-
10 files changed, 367 insertions(+), 30 deletions(-)
create mode 100644 src/app/services/service_module_helper.rb
create mode 100644 src/test/unit/vm_service_test.rb
create mode 100644 src/test/unit/vm_task_test.rb
diff --git a/src/app/models/vm.rb b/src/app/models/vm.rb
index f445219..df18c7e 100644
--- a/src/app/models/vm.rb
+++ b/src/app/models/vm.rb
@@ -280,6 +280,13 @@ class Vm < ActiveRecord::Base
actions
end
+ # Provide method to check if requested action exists, so caller can decide
+ # if they want to throw an error of some sort before continuing
+ # (ie in service api)
+ def valid_action?(action)
+ return VmTask::ACTIONS.include?(action) ? true : false
+ end
+
# these resource checks are made at VM start/restore time
# use pending here by default since this is used for queueing VM
# creation/start operations
diff --git a/src/app/models/vm_task.rb b/src/app/models/vm_task.rb
index 762438f..649001d 100644
--- a/src/app/models/vm_task.rb
+++ b/src/app/models/vm_task.rb
@@ -146,10 +146,10 @@ class VmTask < Task
end
def self.action_privilege(action)
- return ACTIONS[action][:privilege][0]
+ return ACTIONS[action].nil? ? nil : ACTIONS[action][:privilege][0]
end
def self.action_privilege_object(action, obj)
- return obj.send(ACTIONS[action][:privilege][1])
+ return ACTIONS[action].nil? ? nil : obj.send(ACTIONS[action][:privilege][1])
end
def self.action_label(action)
return ACTIONS[action][:label]
diff --git a/src/app/services/service_module_helper.rb b/src/app/services/service_module_helper.rb
new file mode 100644
index 0000000..dd04f99
--- /dev/null
+++ b/src/app/services/service_module_helper.rb
@@ -0,0 +1,34 @@
+#
+# Copyright (C) 2009 Red Hat, Inc.
+# Written by Jason Guiditta <jguiditt 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.
+
+
+# This module is used to bootstrap testing for the service api,
+# which depends on certain methods existing in the class which includes it.
+# Clearly this is not the case with unit tests on the modules themselves,
+# so any dependencies are set up here.
+module ServiceModuleHelper
+
+ def get_login_user
+ return @user
+ end
+
+ def set_login_user(user=nil)
+ @user = user
+ end
+end
\ No newline at end of file
diff --git a/src/app/services/vm_service.rb b/src/app/services/vm_service.rb
index 3d9777c..a78e17d 100644
--- a/src/app/services/vm_service.rb
+++ b/src/app/services/vm_service.rb
@@ -211,13 +211,42 @@ module VmService
# <tt>VmTask.action_privilege(@action)</tt>
def svc_vm_action(id, vm_action, action_args)
@vm = Vm.find(id)
+ unless @vm.valid_action?(vm_action)
+ raise ActionError.new("#{vm_action} is an invalid action.")
+ end
authorized!(VmTask.action_privilege(vm_action),
VmTask.action_privilege_object(vm_action, at vm))
@task = @vm.queue_action(@user, vm_action, action_args)
unless @task
- raise ActionError.new("#{vm_action} is an invalid action.")
+ raise ActionError.new("#{vm_action} cannot be performed on this vm.")
+ end
+ return "#{@vm.description}: #{vm_action} was successfully queued."
+ end
+
+ # Perform action +vm_action+ on vms identified by +vm_id+
+ #
+ # === Instance variables
+ # * <tt>@vms</tt> VMs identified by +vm_ids+
+ # === Required permissions
+ # permission is action-specific as determined by
+ # <tt>VmTask.action_privilege(@action)</tt>
+ # This method can be called to initiate an action on one or more vms
+ def svc_vm_actions(vm_ids, vm_action, action_args)
+ vm_ids = [vm_ids] unless vm_ids.is_a?(Array)
+ successful_vms = []
+ failed_vms = {}
+ vm_ids.each do |vm_id|
+ begin
+ successful_vms << svc_vm_action(vm_id, vm_action, action_args)
+ rescue Exception => ex
+ failed_vms[@vm.description] = ex.message
+ end
+ end
+ unless failed_vms.empty?
+ raise PartialSuccessError.new("Your request to #{vm_action} encountered the following errors: ",
+ failed_vms, successful_vms)
end
- return "#{vm_action} was successfully queued."
+ return "Request to #{vm_action} submitted."
end
# Cancels queued tasks for for Vm with +id+
diff --git a/src/db-omatic/db_omatic.rb b/src/db-omatic/db_omatic.rb
index b3d5e73..155ff5e 100755
--- a/src/db-omatic/db_omatic.rb
+++ b/src/db-omatic/db_omatic.rb
@@ -113,6 +113,15 @@ class DbOmatic < Qpid::Qmf::Console
end
end
+ def set_vm_stopped(db_vm)
+ db_vm.host_id = nil
+ db_vm.memory_used = nil
+ db_vm.num_vcpus_used = nil
+ db_vm.needs_restart = nil
+ db_vm.vnc_port = nil
+ db_vm.state = Vm::STATE_STOPPED
+ end
+
def update_domain_state(domain, state_override = nil)
vm = Vm.find(:first, :conditions => [ "uuid = ?", domain['uuid'] ])
if vm == nil
@@ -190,12 +199,7 @@ class DbOmatic < Qpid::Qmf::Console
result = qmf_vm.undefine
if result.status == 0
@logger.info "Delete of VM #{vm.description} successful, syncing DB."
- vm.host_id = nil
- vm.memory_used = nil
- vm.num_vcpus_used = nil
- vm.state = Vm::STATE_STOPPED
- vm.needs_restart = nil
- vm.vnc_port = nil
+ set_vm_stopped(vm)
end
end
end
@@ -223,20 +227,37 @@ class DbOmatic < Qpid::Qmf::Console
host_info[:synced] = true
if state == Host::STATE_AVAILABLE
- # At this point we want to set all domains that are
- # unreachable to stopped. If a domain is indeed running
- # then dbomatic will see that and set it either before
- # or after. If the node was rebooted, the VMs will all
- # be gone and dbomatic won't see them so we need to set
- # them to stopped.
- db_vm = Vm.find(:all, :conditions => ["host_id = ? AND state = ?", db_host.id, Vm::STATE_UNREACHABLE])
- db_vm.each do |vm|
- @logger.info "Moving vm #{vm.description} in state #{vm.state} to state stopped."
- vm.state = Vm::STATE_STOPPED
- vm.save!
+ Thread.new do
+ @logger.info "#{host_info['hostname']} has moved to available, sleeping for updates to vms."
+ sleep(20)
+
+ # At this point we want to set all domains that are
+ # unreachable to stopped. We're using a thread here to
+ # sleep for 10 seconds outside of the main dbomatic loop.
+ # If after 10 seconds with this host up there are still
+ # domains set to 'unreachable', then we're going to guess
+ # the node rebooted and so the domains should be set to
+ # stopped.
+ @logger.info "Checking for dead VMs on newly available host #{host_info['hostname']}."
+
+ # Double check to make sure this host is still up.
+ begin
+ qmf_host = @session.object(:class => 'node', 'hostname' => host_info['hostname'])
+ if !qmf_host
+ @logger.info "Host #{host_info['hostname']} is not up after waiting 20 seconds, skipping dead VM check."
+ else
+ db_vm = Vm.find(:all, :conditions => ["host_id = ? AND state = ?", db_host.id, Vm::STATE_UNREACHABLE])
+ db_vm.each do |vm|
+ @logger.info "Moving vm #{vm.description} in state #{vm.state} to state stopped."
+ set_vm_stopped(vm)
+ vm.save!
+ end
+ end
+ rescue Exception => e # just log any errors here
+ @logger.info "Exception checking for dead VMs (could be normal): #{e.message}"
+ end
end
end
-
else
# FIXME: This would be a newly registered host. We could put it in the database.
@logger.info "Unknown host #{host_info['hostname']}, probably not registered yet??"
@@ -344,6 +365,7 @@ class DbOmatic < Qpid::Qmf::Console
end
def heartbeat(agent, timestamp)
+ puts "heartbeat from agent #{agent}"
return if agent == nil
synchronize do
bank_key = "#{agent.agent_bank}.#{agent.broker.broker_bank}"
@@ -376,6 +398,8 @@ class DbOmatic < Qpid::Qmf::Console
values[:timed_out] = true
end
end
+ bank_key = "#{agent.agent_bank}.#{agent.broker.broker_bank}"
+ @heartbeats.delete(bank_key)
end
# The opposite of above, this is called when an agent is alive and well and makes sure
@@ -415,11 +439,30 @@ class DbOmatic < Qpid::Qmf::Console
@logger.error "Error with closing all VM VNCs operation: #{e.message}"
end
+ # On startup, since we don't know the previous states of anything, we basically
+ # do a big sync up with teh states of all VMs. We don't worry about hosts since
+ # they are very simple and are either up or down, but it's possible we left
+ # VMs in various states that are no longer applicable to this moment.
db_vm = Vm.find(:all)
db_vm.each do |vm|
- @logger.info "Marking vm #{vm.description} as stopped."
- vm.state = Vm::STATE_STOPPED
- vm.save!
+ set_stopped = false
+ # Basically here we are looking for VMs which are not up in some form or another and setting
+ # them to stopped. VMs that exist as QMF objects will get set appropriately when the objects
+ # appear on the bus.
+ begin
+ qmf_vm = @session.object(:class => 'domain', 'uuid' => db_vm.uuid)
+ if qmf_vm == nil
+ set_stopped = true
+ end
+ rescue Exception => ex
+ set_stopped = true
+ end
+
+ if set_stopped
+ @logger.info "On startup, VM #{vm.description} is not found, setting to stopped."
+ set_vm_stopped(vm)
+ vm.save!
+ end
end
end
@@ -444,6 +487,7 @@ class DbOmatic < Qpid::Qmf::Console
# Get seconds from the epoch
t = Time.new.to_i
+ puts "going through heartbeats.."
@heartbeats.keys.each do | key |
agent, timestamp = @heartbeats[key]
@@ -451,11 +495,11 @@ class DbOmatic < Qpid::Qmf::Console
s = timestamp / 1000000000
delta = t - s
+ puts "Checking time delta for agent #{agent} - #{delta}"
+
if delta > 30
# No heartbeat for 30 seconds.. deal with dead/disconnected agent.
agent_disconnected(agent)
-
- @heartbeats.delete(key)
else
agent_connected(agent)
end
diff --git a/src/test/fixtures/permissions.yml b/src/test/fixtures/permissions.yml
index 4895f3e..eed95fd 100644
--- a/src/test/fixtures/permissions.yml
+++ b/src/test/fixtures/permissions.yml
@@ -35,5 +35,9 @@ ovirtadmin_prodops_pool:
ovirtadmin_corp_com_qa_pool:
role: monitor
uid: ovirtadmin
- pool: corp_com_qa
+ pool: corp_qa_vmpool
parent_permission: ovirtadmin_root
+testuser_corp_com_qa_pool:
+ role: user
+ uid: testuser
+ pool: corp_qa_vmpool
diff --git a/src/test/fixtures/vms.yml b/src/test/fixtures/vms.yml
index 69b1c2b..b2711b2 100644
--- a/src/test/fixtures/vms.yml
+++ b/src/test/fixtures/vms.yml
@@ -65,6 +65,19 @@ foobar_prod1_vm:
boot_device: cdrom
host: fedoraworkstation_foobar_com
vm_resource_pool: corp_com_production_vmpool
+corp_com_qa_postgres_vm:
+ uuid: f6059569-a81f-4728-bb61-6f16b1c594e7
+ description: corp.com qa postgres vm
+ num_vcpus_allocated: 2
+ num_vcpus_used: 2
+ memory_allocated: 262144
+ memory_used: 131072
+ vnic_mac_addr: 00:16:3e:04:de:c8
+ state: running
+ needs_restart: 0
+ boot_device: hd
+ host: macworkstation_qa_corp_com
+ vm_resource_pool: corp_qa_vmpool
foobar_prod2_vm:
uuid: 24b9a994-d415-481d-ace8-1d810b601eb6
description: foobar prod2
diff --git a/src/test/unit/vm_service_test.rb b/src/test/unit/vm_service_test.rb
new file mode 100644
index 0000000..7bd00ad
--- /dev/null
+++ b/src/test/unit/vm_service_test.rb
@@ -0,0 +1,149 @@
+#
+# Copyright (C) 2009 Red Hat, Inc.
+# Written by Jason Guiditta <jguiditt 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 VmServiceTest < ActiveSupport::TestCase
+
+ include ServiceModuleHelper
+ include VmService
+ fixtures :vms
+
+ def setup
+ set_login_user('ovirtadmin')
+ end
+
+ # Ensure correct message is returned for a valid action requested
+ # by a user with the proper permissions
+ def test_svc_vm_action_valid_user
+ assert_equal("#{vms(:production_mysqld_vm).description}: shutdown_vm was successfully queued.",
+ svc_vm_action(vms(:production_mysqld_vm).id, 'shutdown_vm', nil))
+ end
+
+ # Ensure that if a non-existant action is passed in, ActionError
+ # is thrown
+ def test_svc_vm_action_invalid_action
+ assert_raise ActionError do
+ svc_vm_action(vms(:production_httpd_vm).id, 'stop_vm', nil)
+ end
+ end
+
+ # Ensure that if a user with the wrong permissions is passed in,
+ # PermissionError is thrown
+ def test_svc_vm_action_invalid_user
+ set_login_user('fred')
+ assert_raise PermissionError do
+ svc_vm_action(vms(:production_mysqld_vm).id, 'shutdown_vm', nil)
+ end
+ end
+
+ # Ensure that if an invalid state change for a vm is requested,
+ # ActionError is thrown
+ def test_svc_vm_action_invalid_state_change
+ assert_raise ActionError do
+ svc_vm_action(vms(:production_httpd_vm).id, 'shutdown_vm', nil)
+ end
+ end
+
+ # If only one vm was passed into svc_vm_actions, and that action cannot
+ # be performed, ActionError should be returned
+ def test_failed_single_vm_returns_partial_success_error
+ assert_raise PartialSuccessError do
+ svc_vm_actions(vms(:production_httpd_vm).id, 'shutdown_vm', nil)
+ end
+ end
+
+ # If multiple vms were passed into svc_vm_actions, and one or more (but
+ # not all) actions cannot be performed, PartialSuccessError should be returned
+ def test_failed_multiple_vms_return_partial_success
+ assert_raise PartialSuccessError do
+ svc_vm_actions([vms(:production_httpd_vm).id,
+ vms(:production_mysqld_vm).id,
+ vms(:production_ftpd_vm).id], 'shutdown_vm', nil)
+ end
+ end
+
+ # Ensure we receive the expected success message if all actions succeed
+ # (should be the same message if one or more, so we have one test for
+ # each of those cases)
+ def test_success_message_from_single_vm
+ assert_equal("shutdown_vm successful.",
+ svc_vm_actions(vms(:production_mysqld_vm).id, 'shutdown_vm', nil))
+ end
+
+ # Ensure we receive the expected success message if all actions succeed
+ # (should be the same message if one or more, so we have one test for
+ # each of those cases)
+ def test_success_message_for_multiple_vms
+ assert_equal("shutdown_vm successful.",
+ svc_vm_actions([vms(:production_postgresql_vm).id,
+ vms(:production_mysqld_vm).id,
+ vms(:foobar_prod1_vm).id], 'shutdown_vm', nil))
+ end
+
+ # Ensure that if a non-existant action is passed in, PartialSuccessError
+ # is thrown
+ def test_svc_vm_actions_invalid_action
+ assert_raise PartialSuccessError do
+ svc_vm_actions(vms(:production_httpd_vm).id, 'stop_vm', nil)
+ end
+ end
+
+ # Ensure we receive the expected success message if all actions succeed
+ # (should be the same message if one or more, so we have one test for
+ # each of those cases)
+ def test_success_message_from_single_vm_with_less_privileged_user
+ set_login_user('testuser')
+ assert_equal("shutdown_vm successful.",
+ svc_vm_actions(vms(:corp_com_qa_postgres_vm).id, 'shutdown_vm', nil))
+ end
+
+ # Ensure that if a user with the wrong permissions is passed in,
+ # PartialSuccessError is thrown. This allows some vms to still pass
+ # while others with wrong perms fail.
+ def test_svc_vm_actions_invalid_user
+ set_login_user('testuser')
+ assert_raise PartialSuccessError do
+ svc_vm_actions([vms(:corp_com_qa_postgres_vm).id,
+ vms(:production_mysqld_vm).id], 'shutdown_vm', nil)
+ end
+ end
+
+ # Ensure that if a user with the wrong permissions is passed in,
+ # PartialSuccessError contains the message of success and permission error.
+ def test_error_for_svc_vm_actions_invalid_user
+ set_login_user('testuser')
+ actual_error = nil
+ expected_error = PartialSuccessError.new(
+ "Your request to shutdown_vm encountered the following errors: ",
+ {vms(:production_mysqld_vm).description => "You have insufficient privileges to perform action."},
+ ["#{vms(:corp_com_qa_postgres_vm).description}: shutdown_vm was successfully queued."])
+
+ begin
+ svc_vm_actions([vms(:corp_com_qa_postgres_vm).id,
+ vms(:production_mysqld_vm).id], 'shutdown_vm', nil)
+ rescue Exception => ex
+ actual_error = ex
+ end
+ assert_equal expected_error.message, actual_error.message
+ assert_equal expected_error.failures, actual_error.failures
+ assert_equal expected_error.successes, actual_error.successes
+ end
+end
diff --git a/src/test/unit/vm_task_test.rb b/src/test/unit/vm_task_test.rb
new file mode 100644
index 0000000..39be915
--- /dev/null
+++ b/src/test/unit/vm_task_test.rb
@@ -0,0 +1,38 @@
+#
+# Copyright (C) 2009 Red Hat, Inc.
+# Written by Jason Guiditta <jguiditt 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 VmTaskTest < ActiveSupport::TestCase
+
+ fixtures :vms
+
+ # Ensure action_privilege method returns nil if it is passed an action
+ # that does not exist.
+ def test_action_privilege_with_bad_action
+ assert_equal(nil, VmTask.action_privilege('stop_vm'))
+ end
+
+ # Ensure action_privilege_object method returns nil if it is passed an action
+ # that does not exist.
+ def test_action_privilege_object_with_bad_action
+ assert_equal(nil, VmTask.action_privilege_object('stop_vm', vms(:production_httpd_vm).id))
+ end
+end
diff --git a/src/test/unit/vm_test.rb b/src/test/unit/vm_test.rb
index 5e03715..a5d6b3d 100644
--- a/src/test/unit/vm_test.rb
+++ b/src/test/unit/vm_test.rb
@@ -1,6 +1,7 @@
#
# Copyright (C) 2008 Red Hat, Inc.
-# Written by Scott Seago <sseago at redhat.com>
+# Written by Scott Seago <sseago at redhat.com>,
+# Jason Guiditta <jguiditt 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
@@ -19,7 +20,7 @@
require File.dirname(__FILE__) + '/../test_helper'
-class VmTest < Test::Unit::TestCase
+class VmTest < ActiveSupport::TestCase
fixtures :vms
fixtures :pools
@@ -173,6 +174,24 @@ class VmTest < Test::Unit::TestCase
assert_equal 'stopped', vms(:production_httpd_vm).get_pending_state
end
+ def test_get_action_list_with_no_user
+ empty_array = []
+ assert_equal(empty_array, vms(:production_httpd_vm).get_action_list)
+ end
+
+ def test_queue_action_returns_false_with_invalid_action
+ assert_equal(false, vms(:production_httpd_vm).queue_action('ovirtadmin', 'stop_vm'))
+ end
+
+ def test_valid_action_with_invalid_param
+ assert_equal(false, vms(:production_httpd_vm).valid_action?('stop_vm'))
+ end
+
+ # Ensure valid_action? returns true
+ def test_valid_action_with_valid_param
+ assert_equal(true, vms(:production_httpd_vm).valid_action?('shutdown_vm'))
+ end
+
def test_paginated_results
assert_equal 5, Vm.paged_with_perms('ovirtadmin', Privilege::VIEW, 1, 'vms.id').size
end
--
1.6.0.6
More information about the ovirt-devel
mailing list