From 55aeb783e3bfbd024cbd1ffa1b4c6e098a6ee933 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Vincent=20Fran=C3=A7oise?= Date: Wed, 10 Feb 2016 14:59:14 +0100 Subject: [PATCH] Added unit tests on actions As we had a low test coverage on actions, I added some more tests with this patchset. This actually revealed a small bug (typo) in "change_nova_service_state" which has been fixed in here. Note that Tempest test also cover these action via the basic_consolidation strategy. Change-Id: I2d7116a6fdefee82ca254512a9cf50fc61e3c80e Closes-Bug: #1523513 --- .../actions/change_nova_service_state.py | 8 +- .../actions/test_change_nova_service_state.py | 126 ++++++++++-- .../tests/applier/actions/test_migration.py | 189 +++++++++++++----- 3 files changed, 256 insertions(+), 67 deletions(-) diff --git a/watcher/applier/actions/change_nova_service_state.py b/watcher/applier/actions/change_nova_service_state.py index 53eb02822..5eabc494a 100644 --- a/watcher/applier/actions/change_nova_service_state.py +++ b/watcher/applier/actions/change_nova_service_state.py @@ -54,9 +54,9 @@ class ChangeNovaServiceState(base.BaseAction): target_state = None if self.state == hstate.HypervisorState.OFFLINE.value: target_state = False - elif self.status == hstate.HypervisorState.ONLINE.value: + elif self.state == hstate.HypervisorState.ONLINE.value: target_state = True - return self.nova_manage_service(target_state) + return self._nova_manage_service(target_state) def revert(self): target_state = None @@ -64,9 +64,9 @@ class ChangeNovaServiceState(base.BaseAction): target_state = True elif self.state == hstate.HypervisorState.ONLINE.value: target_state = False - return self.nova_manage_service(target_state) + return self._nova_manage_service(target_state) - def nova_manage_service(self, state): + def _nova_manage_service(self, state): if state is None: raise exception.IllegalArgumentException( message=_("The target state is not defined")) diff --git a/watcher/tests/applier/actions/test_change_nova_service_state.py b/watcher/tests/applier/actions/test_change_nova_service_state.py index a2fdc8158..e8810f237 100644 --- a/watcher/tests/applier/actions/test_change_nova_service_state.py +++ b/watcher/tests/applier/actions/test_change_nova_service_state.py @@ -1,10 +1,11 @@ # -*- encoding: utf-8 -*- # Copyright (c) 2016 b<>com +# # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at # -# http://www.apache.org/licenses/LICENSE-2.0 +# http://www.apache.org/licenses/LICENSE-2.0 # # Unless required by applicable law or agreed to in writing, software # distributed under the License is distributed on an "AS IS" BASIS, @@ -12,43 +13,134 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. -# + +from __future__ import unicode_literals + +import mock import voluptuous from watcher.applier.actions import base as baction from watcher.applier.actions import change_nova_service_state +from watcher.common import clients +from watcher.common import nova_helper from watcher.decision_engine.model import hypervisor_state as hstate from watcher.tests import base class TestChangeNovaServiceState(base.TestCase): + def setUp(self): super(TestChangeNovaServiceState, self).setUp() - self.a = change_nova_service_state.ChangeNovaServiceState() + + self.m_osc_cls = mock.Mock() + self.m_helper_cls = mock.Mock() + self.m_helper = mock.Mock(spec=nova_helper.NovaHelper) + self.m_helper_cls.return_value = self.m_helper + self.m_osc = mock.Mock(spec=clients.OpenStackClients) + self.m_osc_cls.return_value = self.m_osc + + m_openstack_clients = mock.patch.object( + clients, "OpenStackClients", self.m_osc_cls) + m_nova_helper = mock.patch.object( + nova_helper, "NovaHelper", self.m_helper_cls) + + m_openstack_clients.start() + m_nova_helper.start() + + self.addCleanup(m_openstack_clients.stop) + self.addCleanup(m_nova_helper.stop) + + self.input_parameters = { + baction.BaseAction.RESOURCE_ID: "compute-1", + "state": hstate.HypervisorState.ONLINE.value, + } + self.action = change_nova_service_state.ChangeNovaServiceState() + self.action.input_parameters = self.input_parameters def test_parameters_down(self): - self.a.input_parameters = { + self.action.input_parameters = { baction.BaseAction.RESOURCE_ID: "compute-1", - self.a.STATE: hstate.HypervisorState.OFFLINE.value} - self.assertEqual(True, self.a.validate_parameters()) + self.action.STATE: hstate.HypervisorState.OFFLINE.value} + self.assertEqual(True, self.action.validate_parameters()) def test_parameters_up(self): - self.a.input_parameters = { + self.action.input_parameters = { baction.BaseAction.RESOURCE_ID: "compute-1", - self.a.STATE: hstate.HypervisorState.ONLINE.value} - self.assertEqual(True, self.a.validate_parameters()) + self.action.STATE: hstate.HypervisorState.ONLINE.value} + self.assertEqual(True, self.action.validate_parameters()) def test_parameters_exception_wrong_state(self): - self.a.input_parameters = { + self.action.input_parameters = { baction.BaseAction.RESOURCE_ID: "compute-1", - self.a.STATE: 'error'} - self.assertRaises(voluptuous.Invalid, self.a.validate_parameters) + self.action.STATE: 'error'} + exc = self.assertRaises( + voluptuous.Invalid, self.action.validate_parameters) + self.assertEqual( + [(['state'], voluptuous.ScalarInvalid)], + [([str(p) for p in e.path], type(e)) for e in exc.errors]) def test_parameters_resource_id_empty(self): - self.a.input_parameters = { - self.a.STATE: None} - self.assertRaises(voluptuous.Invalid, self.a.validate_parameters) + self.action.input_parameters = { + self.action.STATE: hstate.HypervisorState.ONLINE.value, + } + exc = self.assertRaises( + voluptuous.Invalid, self.action.validate_parameters) + self.assertEqual( + [(['resource_id'], voluptuous.RequiredFieldInvalid)], + [([str(p) for p in e.path], type(e)) for e in exc.errors]) def test_parameters_applies_add_extra(self): - self.a.input_parameters = {"extra": "failed"} - self.assertRaises(voluptuous.Invalid, self.a.validate_parameters) + self.action.input_parameters = {"extra": "failed"} + exc = self.assertRaises( + voluptuous.Invalid, self.action.validate_parameters) + self.assertEqual( + sorted([(['resource_id'], voluptuous.RequiredFieldInvalid), + (['state'], voluptuous.RequiredFieldInvalid), + (['extra'], voluptuous.Invalid)], + key=lambda x: str(x[0])), + sorted([([str(p) for p in e.path], type(e)) for e in exc.errors], + key=lambda x: str(x[0]))) + + def test_change_service_state_precondition(self): + try: + self.action.precondition() + except Exception as exc: + self.fail(exc) + + def test_change_service_state_postcondition(self): + try: + self.action.postcondition() + except Exception as exc: + self.fail(exc) + + def test_execute_change_service_state_with_enable_target(self): + self.action.execute() + + self.m_helper_cls.assert_called_once_with(osc=self.m_osc) + self.m_helper.enable_service_nova_compute.assert_called_once_with( + "compute-1") + + def test_execute_change_service_state_with_disable_target(self): + self.action.input_parameters["state"] = ( + hstate.HypervisorState.OFFLINE.value) + self.action.execute() + + self.m_helper_cls.assert_called_once_with(osc=self.m_osc) + self.m_helper.disable_service_nova_compute.assert_called_once_with( + "compute-1") + + def test_revert_change_service_state_with_enable_target(self): + self.action.revert() + + self.m_helper_cls.assert_called_once_with(osc=self.m_osc) + self.m_helper.disable_service_nova_compute.assert_called_once_with( + "compute-1") + + def test_revert_change_service_state_with_disable_target(self): + self.action.input_parameters["state"] = ( + hstate.HypervisorState.OFFLINE.value) + self.action.revert() + + self.m_helper_cls.assert_called_once_with(osc=self.m_osc) + self.m_helper.enable_service_nova_compute.assert_called_once_with( + "compute-1") diff --git a/watcher/tests/applier/actions/test_migration.py b/watcher/tests/applier/actions/test_migration.py index aa92a5839..82409ed00 100644 --- a/watcher/tests/applier/actions/test_migration.py +++ b/watcher/tests/applier/actions/test_migration.py @@ -1,10 +1,11 @@ # -*- encoding: utf-8 -*- # Copyright (c) 2016 b<>com +# # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. # You may obtain a copy of the License at # -# http://www.apache.org/licenses/LICENSE-2.0 +# http://www.apache.org/licenses/LICENSE-2.0 # # Unless required by applicable law or agreed to in writing, software # distributed under the License is distributed on an "AS IS" BASIS, @@ -12,67 +13,163 @@ # implied. # See the License for the specific language governing permissions and # limitations under the License. -# + +from __future__ import unicode_literals + +import mock import voluptuous from watcher.applier.actions import base as baction from watcher.applier.actions import migration +from watcher.common import clients +from watcher.common import exception +from watcher.common import nova_helper from watcher.tests import base class TestMigration(base.TestCase): + + INSTANCE_UUID = "45a37aeb-95ab-4ddb-a305-7d9f62c2f5ba" + def setUp(self): super(TestMigration, self).setUp() - self.mig = migration.Migrate() + + self.m_osc_cls = mock.Mock() + self.m_helper_cls = mock.Mock() + self.m_helper = mock.Mock(spec=nova_helper.NovaHelper) + self.m_helper_cls.return_value = self.m_helper + self.m_osc = mock.Mock(spec=clients.OpenStackClients) + self.m_osc_cls.return_value = self.m_osc + + m_openstack_clients = mock.patch.object( + clients, "OpenStackClients", self.m_osc_cls) + m_nova_helper = mock.patch.object( + nova_helper, "NovaHelper", self.m_helper_cls) + + m_openstack_clients.start() + m_nova_helper.start() + + self.addCleanup(m_openstack_clients.stop) + self.addCleanup(m_nova_helper.stop) + + self.input_parameters = { + "migration_type": "live", + "src_hypervisor": "hypervisor1-hostname", + "dst_hypervisor": "hypervisor2-hostname", + baction.BaseAction.RESOURCE_ID: self.INSTANCE_UUID, + } + self.action = migration.Migrate() + self.action.input_parameters = self.input_parameters def test_parameters(self): params = {baction.BaseAction.RESOURCE_ID: - "45a37aeb-95ab-4ddb-a305-7d9f62c2f5ba", - self.mig.MIGRATION_TYPE: 'live', - self.mig.DST_HYPERVISOR: 'compute-2', - self.mig.SRC_HYPERVISOR: 'compute3'} - self.mig.input_parameters = params - self.assertEqual(True, self.mig.validate_parameters()) - - def test_parameters_exception_resource_id(self): - parameters = {baction.BaseAction.RESOURCE_ID: "EFEF", - 'migration_type': 'live', - 'src_hypervisor': 'compute-2', - 'dst_hypervisor': 'compute3'} - self.mig.input_parameters = parameters - self.assertRaises(voluptuous.Invalid, self.mig.validate_parameters) - - def test_parameters_exception_migration_type(self): - parameters = {baction.BaseAction.RESOURCE_ID: - "45a37aeb-95ab-4ddb-a305-7d9f62c2f5ba", - 'migration_type': 'cold', - 'src_hypervisor': 'compute-2', - 'dst_hypervisor': 'compute3'} - self.mig.input_parameters = parameters - self.assertRaises(voluptuous.Invalid, self.mig.validate_parameters) - - def test_parameters_exception_src_hypervisor(self): - parameters = {baction.BaseAction.RESOURCE_ID: - "45a37aeb-95ab-4ddb-a305-7d9f62c2f5ba", - 'migration_type': 'cold', - 'src_hypervisor': None, - 'dst_hypervisor': 'compute3'} - self.mig.input_parameters = parameters - self.assertRaises(voluptuous.Invalid, self.mig.validate_parameters) - - def test_parameters_exception_dst_hypervisor(self): - parameters = {baction.BaseAction.RESOURCE_ID: - "45a37aeb-95ab-4ddb-a305-7d9f62c2f5ba", - 'migration_type': 'cold', - 'src_hypervisor': 'compute-1', - 'dst_hypervisor': None} - self.mig.input_parameters = parameters - self.assertRaises(voluptuous.Invalid, self.mig.validate_parameters) + self.INSTANCE_UUID, + self.action.MIGRATION_TYPE: 'live', + self.action.DST_HYPERVISOR: 'compute-2', + self.action.SRC_HYPERVISOR: 'compute-3'} + self.action.input_parameters = params + self.assertEqual(True, self.action.validate_parameters()) def test_parameters_exception_empty_fields(self): parameters = {baction.BaseAction.RESOURCE_ID: None, 'migration_type': None, 'src_hypervisor': None, 'dst_hypervisor': None} - self.mig.input_parameters = parameters - self.assertRaises(voluptuous.Invalid, self.mig.validate_parameters) + self.action.input_parameters = parameters + exc = self.assertRaises( + voluptuous.MultipleInvalid, self.action.validate_parameters) + self.assertEqual( + sorted([(['migration_type'], voluptuous.ScalarInvalid), + (['src_hypervisor'], voluptuous.TypeInvalid), + (['dst_hypervisor'], voluptuous.TypeInvalid)]), + sorted([(e.path, type(e)) for e in exc.errors])) + + def test_parameters_exception_migration_type(self): + parameters = {baction.BaseAction.RESOURCE_ID: + self.INSTANCE_UUID, + 'migration_type': 'cold', + 'src_hypervisor': 'compute-2', + 'dst_hypervisor': 'compute-3'} + self.action.input_parameters = parameters + exc = self.assertRaises( + voluptuous.Invalid, self.action.validate_parameters) + self.assertEqual( + [(['migration_type'], voluptuous.ScalarInvalid)], + [(e.path, type(e)) for e in exc.errors]) + + def test_parameters_exception_src_hypervisor(self): + parameters = {baction.BaseAction.RESOURCE_ID: + self.INSTANCE_UUID, + 'migration_type': 'live', + 'src_hypervisor': None, + 'dst_hypervisor': 'compute-3'} + self.action.input_parameters = parameters + exc = self.assertRaises( + voluptuous.MultipleInvalid, self.action.validate_parameters) + self.assertEqual( + [(['src_hypervisor'], voluptuous.TypeInvalid)], + [(e.path, type(e)) for e in exc.errors]) + + def test_parameters_exception_dst_hypervisor(self): + parameters = {baction.BaseAction.RESOURCE_ID: + self.INSTANCE_UUID, + 'migration_type': 'live', + 'src_hypervisor': 'compute-1', + 'dst_hypervisor': None} + self.action.input_parameters = parameters + exc = self.assertRaises( + voluptuous.MultipleInvalid, self.action.validate_parameters) + self.assertEqual( + [(['dst_hypervisor'], voluptuous.TypeInvalid)], + [(e.path, type(e)) for e in exc.errors]) + + def test_parameters_exception_resource_id(self): + parameters = {baction.BaseAction.RESOURCE_ID: "EFEF", + 'migration_type': 'live', + 'src_hypervisor': 'compute-2', + 'dst_hypervisor': 'compute-3'} + self.action.input_parameters = parameters + exc = self.assertRaises( + voluptuous.MultipleInvalid, self.action.validate_parameters) + self.assertEqual( + [(['resource_id'], voluptuous.Invalid)], + [(e.path, type(e)) for e in exc.errors]) + + def test_migration_precondition(self): + try: + self.action.precondition() + except Exception as exc: + self.fail(exc) + + def test_migration_postcondition(self): + try: + self.action.postcondition() + except Exception as exc: + self.fail(exc) + + def test_execute_live_migration_invalid_instance(self): + self.m_helper.find_instance.return_value = None + exc = self.assertRaises( + exception.InstanceNotFound, self.action.execute) + self.m_helper.find_instance.assert_called_once_with(self.INSTANCE_UUID) + self.assertEqual(exc.kwargs["name"], self.INSTANCE_UUID) + + def test_execute_live_migration(self): + self.m_helper.find_instance.return_value = self.INSTANCE_UUID + + self.action.execute() + + self.m_helper.live_migrate_instance.assert_called_once_with( + instance_id=self.INSTANCE_UUID, + dest_hostname="hypervisor2-hostname") + + def test_revert_live_migration(self): + self.m_helper.find_instance.return_value = self.INSTANCE_UUID + + self.action.revert() + + self.m_helper_cls.assert_called_once_with(osc=self.m_osc) + self.m_helper.live_migrate_instance.assert_called_once_with( + instance_id=self.INSTANCE_UUID, + dest_hostname="hypervisor1-hostname" + )