diff --git a/doc/source/actions/volume_migration.rst b/doc/source/actions/volume_migration.rst index 8c0e42aaf..cdc2c2bdb 100644 --- a/doc/source/actions/volume_migration.rst +++ b/doc/source/actions/volume_migration.rst @@ -40,3 +40,20 @@ parameter type required description Mandatory for changing a volume to a different volume type ======================== ====== ======== =================================== + +Skipping conditions +-------------------- + +Volume migration actions will be automatically skipped in the pre_condition +phase in the following cases: + +- The volume does not exist +- The migration_type is 'retype' and the destination_type is the same as the + current volume type +- The migration_type is 'migrate' and the destination_node is the same as the + current volume host + +On other conditions the action will be FAILED in the pre_condition check: + +- The destination_type does not exist (if specified) +- The destination_node (pool) does not exist (if specified) diff --git a/watcher/applier/actions/volume_migration.py b/watcher/applier/actions/volume_migration.py index 04ae2effe..9732c104b 100644 --- a/watcher/applier/actions/volume_migration.py +++ b/watcher/applier/actions/volume_migration.py @@ -15,6 +15,7 @@ import jsonschema +from cinderclient import exceptions as cinder_exception from oslo_log import log from watcher._i18n import _ @@ -190,7 +191,53 @@ class VolumeMigrate(base.BaseAction): pass def pre_condition(self): - pass + """Check volumemigration preconditions + + Skipping conditions: + - Volume does not exist + - Volume type is already the destination type in retype migration + - Volume is already on the destination node in migrate migration + Failing conditions: + - Destination node (if specified) does not exist + - Destination type (if specified) does not exist + """ + try: + volume = self.cinder_util.get_volume(self.volume_id) + except cinder_exception.NotFound: + raise exception.ActionSkipped( + _("Volume %s not found") % self.volume_id) + + # Check if destination_type exists (if specified) + if self.destination_type: + volume_types = self.cinder_util.get_volume_type_list() + type_names = [vt.name for vt in volume_types] + if self.destination_type not in type_names: + raise exception.ActionExecutionFailure( + _("Volume type %s not found") % self.destination_type) + + # Check if destination_node (pool) exists (if specified) + if self.destination_node: + try: + self.cinder_util.get_storage_pool_by_name( + self.destination_node) + except exception.PoolNotFound: + raise exception.ActionExecutionFailure( + _("Pool %s not found") % self.destination_node) + + # Check if retype to same type + if (self.migration_type == self.RETYPE and + self.destination_type and + volume.volume_type == self.destination_type): + raise exception.ActionSkipped( + _("Volume type is already %s") % self.destination_type) + + # Check if migrate to same node + if (self.migration_type in (self.SWAP, self.MIGRATE) and + self.destination_node): + current_host = getattr(volume, 'os-vol-host-attr:host') + if current_host == self.destination_node: + raise exception.ActionSkipped( + _("Volume is already on node %s") % self.destination_node) def post_condition(self): pass diff --git a/watcher/tests/unit/applier/actions/test_volume_migration.py b/watcher/tests/unit/applier/actions/test_volume_migration.py index 2706b49b8..c83e88ffa 100644 --- a/watcher/tests/unit/applier/actions/test_volume_migration.py +++ b/watcher/tests/unit/applier/actions/test_volume_migration.py @@ -14,12 +14,14 @@ from unittest import mock +from cinderclient import exceptions as cinder_exception import jsonschema from watcher.applier.actions import base as baction from watcher.applier.actions import volume_migration from watcher.common import cinder_helper from watcher.common import clients +from watcher.common import exception from watcher.common import keystone_helper from watcher.common import nova_helper from watcher.tests.unit import base @@ -110,6 +112,9 @@ class TestMigration(base.TestCase): volume.snapshot_id = kwargs.get('snapshot_id', None) volume.availability_zone = kwargs.get('availability_zone', 'nova') volume.attachments = kwargs.get('attachments', []) + volume.volume_type = kwargs.get('volume_type', 'default-type') + setattr(volume, 'os-vol-host-attr:host', + kwargs.get('host', 'current-host')) return volume @staticmethod @@ -223,3 +228,111 @@ class TestMigration(base.TestCase): volume, "storage1-poolname" ) + + def test_pre_condition_volume_not_found(self): + self.m_c_helper.get_volume.side_effect = ( + cinder_exception.NotFound('404')) + + # ActionSkipped is expected because the volume is not found + self.assertRaisesRegex( + exception.ActionSkipped, + f"Volume {self.VOLUME_UUID} not found", + self.action_migrate.pre_condition) + + def test_pre_condition_destination_type_not_found(self): + volume = self.fake_volume() + self.m_c_helper.get_volume.return_value = volume + + # Mock volume type list that doesn't contain the destination type + fake_type_1 = mock.MagicMock() + fake_type_1.name = "type-1" + fake_type_2 = mock.MagicMock() + fake_type_2.name = "type-2" + self.m_c_helper.get_volume_type_list.return_value = [ + fake_type_1, fake_type_2] + + # ActionExecutionFailure is expected because the destination type + # is not found + self.assertRaisesRegex( + exception.ActionExecutionFailure, + "Volume type storage1-typename not found", + self.action_retype.pre_condition) + + def test_pre_condition_destination_pool_not_found(self): + volume = self.fake_volume() + self.m_c_helper.get_volume.return_value = volume + + # Mock get_storage_pool_by_name to raise PoolNotFound + self.m_c_helper.get_storage_pool_by_name.side_effect = ( + exception.PoolNotFound(name="storage1-poolname")) + + # ActionExecutionFailure is expected because the destination pool + # is not found + self.assertRaisesRegex( + exception.ActionExecutionFailure, + "Pool storage1-poolname not found", + self.action_migrate.pre_condition) + + def test_pre_condition_success_with_type(self): + volume = self.fake_volume() + self.m_c_helper.get_volume.return_value = volume + + # Mock volume type list that contains the destination type + fake_type_1 = mock.MagicMock() + fake_type_1.name = "storage1-typename" + fake_type_2 = mock.MagicMock() + fake_type_2.name = "type-2" + self.m_c_helper.get_volume_type_list.return_value = [ + fake_type_1, fake_type_2] + + # Should not raise any exception + self.action_retype.pre_condition() + self.m_c_helper.get_volume.assert_called_once_with(self.VOLUME_UUID) + self.m_c_helper.get_volume_type_list.assert_called_once_with() + + def test_pre_condition_success_with_pool(self): + volume = self.fake_volume() + self.m_c_helper.get_volume.return_value = volume + + # Mock pool + fake_pool = mock.MagicMock() + fake_pool.name = "storage1-poolname" + self.m_c_helper.get_storage_pool_by_name.return_value = fake_pool + + # Should not raise any exception + self.action_migrate.pre_condition() + self.m_c_helper.get_volume.assert_called_once_with(self.VOLUME_UUID) + self.m_c_helper.get_storage_pool_by_name.assert_called_once_with( + "storage1-poolname") + + def test_pre_condition_retype_same_type(self): + # Create volume with the same type as destination + volume = self.fake_volume(volume_type="storage1-typename") + self.m_c_helper.get_volume.return_value = volume + + # Mock volume type list that contains the destination type + fake_type = mock.MagicMock() + fake_type.name = "storage1-typename" + self.m_c_helper.get_volume_type_list.return_value = [fake_type] + + # ActionSkipped is expected because volume already has the target type + self.assertRaisesRegex( + exception.ActionSkipped, + "Volume type is already storage1-typename", + self.action_retype.pre_condition) + + def test_pre_condition_migrate_same_node(self): + # Create volume on the same node as destination + volume = self.fake_volume(host="storage1-poolname") + self.m_c_helper.get_volume.return_value = volume + + # Mock pool + fake_pool = mock.MagicMock() + fake_pool.name = "storage1-poolname" + self.m_c_helper.get_storage_pool_by_name.return_value = fake_pool + + # ActionSkipped is expected because volume is already on target node + self.assertRaisesRegex( + exception.ActionSkipped, + "Volume is already on node storage1-poolname", + self.action_migrate.pre_condition)