[Ovirt-devel] [PATCH server] Add svc_vm_actions method to VmService.

Jason Guiditta jguiditt at redhat.com
Mon Jun 29 18:24:04 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/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 ++++-
 9 files changed, 300 insertions(+), 7 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..073b819 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 "#{vm_action} submitted."
   end
 
   #  Cancels queued tasks for for Vm with +id+
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