openstack/watcher
Code Issues Proposed changes

Merge "Skip volume_migrate actions in pre_condition phase"

Browse Source
This commit is contained in:
Zuul
2026-02-26 13:06:01 +00:00
committed by Gerrit Code Review
parent b69a174ff5 b7e8d459fc
commit da05dc7f65
3 changed files with 178 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
doc/source/actions/volume_migration.rst
View File

@@ -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)

49
watcher/applier/actions/volume_migration.py
View File

@@ -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

113
watcher/tests/unit/applier/actions/test_volume_migration.py
View File

@@ -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)