openstack/watcher
Code Issues Proposed changes

Use src_type to filter volumes in zone migration

Browse Source 
Despite having the src_type paremeter for the storage_pool dictionary as
a mandatory parameter, the value is not being used to filter the volumes
to migrate, using only 'src_pool'.

This change makes 'src_type' optional, since it was ignored until this
point, making it optional keeps the same behaviour by default. If
'src_type' is in the audit parameters, the strategy uses both 'src_pool' and
'src_type' to filter the volumes to migrate.

Closes-Bug: 2111507
Change-Id: Id83a96de85ada1ae6c0e25f8b7fcf54034604911
Signed-off-by: jgilaber <jgilaber@redhat.com>
This commit is contained in:
jgilaber
2025-06-04 17:23:04 +02:00
parent cee72d2bda
commit cb6fb16097
5 changed files with 471 additions and 19 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
doc/source/strategies/zone_migration.rst
View File

@@ -126,7 +126,7 @@ parameter type default required description
volumes migrate.
``dst_pool`` string None Optional Storage pool to which
volumes migrate.
``src_type`` string None Required Source volume type.
``src_type`` string None Optional Source volume type.
``dst_type`` string None Required Destination volume type
============= ======= ======== ========= ========================

10
releasenotes/notes/zone_migrate_src_type-642186730a4f354e.yaml Normal file
View File

@@ -0,0 +1,10 @@
---
fixes:
- |
Currently, the zone migration strategy has a ``src_type`` parameter in the ``storage_pools``
input parameter which is ignored, even though it's required when storage_pools is defined.
This patch makes the src_type parameter optional in the zone migration strategy, and when
passed by the user, will use its values to filter the volumes which can be migrated.
For more details: https://launchpad.net/bugs/2111507

30
watcher/decision_engine/strategy/strategies/zone_migration.py
View File

@@ -132,7 +132,7 @@ class ZoneMigration(base.ZoneMigrationBaseStrategy):
"type": "string"
}
},
"required": ["src_pool", "src_type", "dst_type"],
"required": ["src_pool", "dst_type"],
"additionalProperties": False
}
},
@@ -382,11 +382,10 @@ class ZoneMigration(base.ZoneMigrationBaseStrategy):
if node.get("src_node") == src_node:
return node.get("dst_node")
def get_dst_pool_and_type(self, src_pool, src_type):
def get_dst_pool_and_type(self, src_pool):
"""Get destination pool and type from self.migration_storage_pools
:param src_pool: storage pool name
:param src_type: storage volume type
:returns: set of storage pool name and volume type name
"""
for pool in self.migrate_storage_pools:
@@ -407,7 +406,7 @@ class ZoneMigration(base.ZoneMigrationBaseStrategy):
continue
src_type = volume.volume_type
dst_pool, dst_type = self.get_dst_pool_and_type(pool, src_type)
dst_pool, dst_type = self.get_dst_pool_and_type(pool)
LOG.debug(src_type)
LOG.debug("%s %s", dst_pool, dst_type)
@@ -541,12 +540,27 @@ class ZoneMigration(base.ZoneMigrationBaseStrategy):
:returns: volume list on src pools and storage scope
"""
def _is_src_type(volume, src_type):
return (src_type is None or
(src_type is not None and volume.volume_type == src_type))
src_pool_list = self.get_src_pool_list()
target_volumes = []
for volume in self.cinder.get_volume_list():
if not self.storage_model.has_node(volume.id):
# skip volumes that are not in the storage model to satisfy
# scope constraints
continue
for migrate_input in self.migrate_storage_pools:
src_pool = migrate_input["src_pool"]
src_type = migrate_input.get("src_type")
if (getattr(volume, 'os-vol-host-attr:host') == src_pool and
_is_src_type(volume, src_type)):
target_volumes.append(volume)
# once the volume satisfies one the storage_pools
# inputs, we don't need to check the rest
break
return [i for i in self.cinder.get_volume_list()
if getattr(i, 'os-vol-host-attr:host') in src_pool_list and
self.storage_model.get_volume_by_uuid(i.id)]
return target_volumes
def filtered_targets(self):
"""Filter targets

9
watcher/tests/api/v1/test_audits.py
View File

@@ -1332,11 +1332,6 @@ class TestAuditZoneMigration(TestPostBase):
audit_input_dict = self._prepare_audit_params(zm_params)
response = self.post_json('/audits', audit_input_dict,
expect_errors=True)
response = self.post_json('/audits', audit_input_dict)
self.assertEqual("application/json", response.content_type)
self.assertEqual(HTTPStatus.BAD_REQUEST, response.status_int)
expected_error_msg = ("'src_type' is a required property")
self.assertTrue(response.json['error_message'])
self.assertIn(expected_error_msg, response.json['error_message'])
assert not mock_trigger_audit.called
self.assertEqual(HTTPStatus.CREATED, response.status_int)

439
watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py
View File

@@ -226,12 +226,234 @@ class TestZoneMigration(TestBaseStrategy):
volumes = self.strategy.get_volumes()
# src1,src2 is in instances
# src3 is not in instances
# src1 is in instances
# src2,src3 is not in instances
self.assertIn(volume_on_src1, volumes)
self.assertNotIn(volume_on_src2, volumes)
self.assertNotIn(volume_on_src3, volumes)
def test_get_volumes_no_src_type(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_2"],
name="volume_2")
volume_on_src3 = self.fake_volume(host="src3@back2#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
]
self.m_migrate_storage_pools.return_value = [
{"src_pool": "src1@back1#pool1", "dst_pool": "dst1@back1#pool1",
"dst_type": "type1"},
{"src_pool": "src2@back1#pool1", "dst_pool": "dst2@back2#pool1",
"dst_type": "type3"}
]
volumes = self.strategy.get_volumes()
# src1, src2 are in volumes
# src3 is not in volumes
self.assertIn(volume_on_src1, volumes)
self.assertIn(volume_on_src2, volumes)
self.assertNotIn(volume_on_src3, volumes)
def test_get_volumes_different_types_different_pool(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
volume_on_src4 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_3"],
volume_type="type2",
name="volume_0")
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4,
]
volumes = self.strategy.get_volumes()
# src1,src4 is in volumes
# src2,src3 is not in volumes
self.assertIn(volume_on_src1, volumes)
self.assertIn(volume_on_src4, volumes)
self.assertNotIn(volume_on_src3, volumes)
self.assertNotIn(volume_on_src2, volumes)
def test_get_volumes_different_types_same_pool(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
volume_on_src4 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_0"],
volume_type="type3",
name="volume_0")
self.m_c_helper.get_volume_list.return_value = {
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4,
}
volumes = self.strategy.get_volumes()
# src1,src3 is in volumes
# src2,src4 is not in volumes
self.assertIn(volume_on_src1, volumes)
self.assertIn(volume_on_src3, volumes)
self.assertNotIn(volume_on_src4, volumes)
self.assertNotIn(volume_on_src2, volumes)
def test_get_volumes_all_types_in_pool(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
volume_on_src4 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_0"],
volume_type="type2",
name="volume_4")
self.m_migrate_storage_pools.return_value = [
{"src_pool": "src1@back1#pool1", "dst_pool": "dst1@back1#pool1",
"src_type": "type1", "dst_type": "type1"},
{"src_pool": "src1@back1#pool1", "dst_pool": "dst2@back2#pool1",
"src_type": "type2", "dst_type": "type3"}
]
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4,
]
volumes = self.strategy.get_volumes()
# all volumes are selected
self.assertIn(volume_on_src1, volumes)
self.assertIn(volume_on_src3, volumes)
self.assertIn(volume_on_src4, volumes)
self.assertIn(volume_on_src2, volumes)
def test_get_volumes_type_in_all_pools(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
name="volume_2")
volume_on_src3 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
volume_on_src4 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_0"],
name="volume_0")
self.m_migrate_storage_pools.return_value = [
{"src_pool": "src1@back1#pool1", "dst_pool": "dst1@back1#pool1",
"src_type": "type1", "dst_type": "type1"},
{"src_pool": "src2@back1#pool1", "dst_pool": "dst2@back2#pool1",
"src_type": "type1", "dst_type": "type3"}
]
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4,
]
volumes = self.strategy.get_volumes()
# all volumes are selected
self.assertIn(volume_on_src1, volumes)
self.assertIn(volume_on_src3, volumes)
self.assertIn(volume_on_src4, volumes)
self.assertIn(volume_on_src2, volumes)
def test_get_volumes_select_no_volumes(self):
volume_on_src1 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src3@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3
]
volumes = self.strategy.get_volumes()
# no volumes are selected
self.assertEqual(len(volumes), 0)
def test_get_volumes_duplicated_input(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
name="volume_2")
volume_on_src3 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_3"],
volume_type="type2",
name="volume_3")
volume_on_src4 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_0"],
name="volume_4")
self.m_migrate_storage_pools.return_value = [
{"src_pool": "src2@back1#pool1", "dst_pool": "dst1@back1#pool1",
"src_type": "type1", "dst_type": "type1"},
{"src_pool": "src2@back1#pool1", "dst_pool": "dst2@back2#pool1",
"src_type": "type1", "dst_type": "type3"}
]
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4,
]
volumes = self.strategy.get_volumes()
# src4 is in volumes
# src1, src2, src3 are not in volumes
self.assertIn(volume_on_src4, volumes)
self.assertNotIn(volume_on_src1, volumes)
self.assertNotIn(volume_on_src2, volumes)
self.assertNotIn(volume_on_src3, volumes)
# only src4 is selected
self.assertEqual(len(volumes), 1)
# execute #
def test_execute_live_migrate_instance(self):
@@ -391,9 +613,10 @@ class TestZoneMigration(TestBaseStrategy):
def test_execute_retype_volume(self):
volume_on_src2 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
self.m_c_helper.get_volume_list.return_value = [
volume_on_src2,
volume_on_src2
]
self.m_n_helper.get_instance_list.return_value = []
@@ -430,6 +653,212 @@ class TestZoneMigration(TestBaseStrategy):
global_efficacy_value = solution.global_efficacy[3].get('value', 0)
self.assertEqual(100, global_efficacy_value)
def test_execute_migrate_volumes_no_src_type(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_2"],
name="volume_2")
volume_on_src3 = self.fake_volume(host="src3@back2#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3
]
self.m_migrate_storage_pools.return_value = [
{"src_pool": "src1@back1#pool1", "dst_pool": "dst1@back1#pool1",
"dst_type": "type1"},
{"src_pool": "src2@back1#pool1", "dst_pool": "dst2@back2#pool1",
"dst_type": "type3"}
]
self.m_n_helper.get_instance_list.return_value = []
solution = self.strategy.execute()
self.assertEqual(2, len(solution.actions))
migration_types = collections.Counter(
[action.get('input_parameters')['migration_type']
for action in solution.actions])
self.assertEqual(1,
migration_types.get("migrate", 0))
self.assertEqual(1,
migration_types.get("retype", 0))
global_efficacy_value = solution.global_efficacy[2].get('value', 0)
self.assertEqual(100, global_efficacy_value)
def test_execute_migrate_volumes_different_types_different_pool(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
volume_on_src4 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_3"],
volume_type="type2",
name="volume_0")
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4
]
self.m_n_helper.get_instance_list.return_value = []
solution = self.strategy.execute()
self.assertEqual(2, len(solution.actions))
migration_types = collections.Counter(
[action.get('input_parameters')['migration_type']
for action in solution.actions])
self.assertEqual(1,
migration_types.get("migrate", 0))
self.assertEqual(1,
migration_types.get("retype", 0))
global_efficacy_value = solution.global_efficacy[2].get('value', 0)
self.assertEqual(100, global_efficacy_value)
def test_execute_migrate_volumes_different_types_same_pool(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
volume_on_src4 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_0"],
volume_type="type3",
name="volume_0")
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4
]
self.m_n_helper.get_instance_list.return_value = []
solution = self.strategy.execute()
self.assertEqual(2, len(solution.actions))
migration_types = collections.Counter(
[action.get('input_parameters')['migration_type']
for action in solution.actions])
self.assertEqual(2,
migration_types.get("migrate", 0))
global_efficacy_value = solution.global_efficacy[2].get('value', 0)
self.assertEqual(100, global_efficacy_value)
def test_execute_migrate_volumes_all_types_in_pool(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
volume_on_src4 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_0"],
volume_type="type2",
name="volume_4")
self.m_migrate_storage_pools.return_value = [
{"src_pool": "src1@back1#pool1", "dst_pool": "dst1@back1#pool1",
"src_type": "type1", "dst_type": "type1"},
{"src_pool": "src1@back1#pool1", "dst_pool": "dst2@back2#pool1",
"src_type": "type2", "dst_type": "type3"}
]
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4
]
self.m_n_helper.get_instance_list.return_value = []
solution = self.strategy.execute()
self.assertEqual(2, len(solution.actions))
migration_types = collections.Counter(
[action.get('input_parameters')['migration_type']
for action in solution.actions])
self.assertEqual(1,
migration_types.get("migrate", 0))
self.assertEqual(1,
migration_types.get("retype", 0))
global_efficacy_value = solution.global_efficacy[2].get('value', 0)
self.assertEqual(50, global_efficacy_value)
def test_execute_migrate_volumes_type_in_all_pools(self):
volume_on_src1 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
name="volume_2")
volume_on_src3 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
volume_on_src4 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_0"],
name="volume_0")
self.m_migrate_storage_pools.return_value = [
{"src_pool": "src1@back1#pool1", "dst_pool": "dst1@back1#pool1",
"src_type": "type1", "dst_type": "type1"},
{"src_pool": "src2@back1#pool1", "dst_pool": "dst2@back2#pool1",
"src_type": "type1", "dst_type": "type3"}
]
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
volume_on_src4
]
self.m_n_helper.get_instance_list.return_value = []
solution = self.strategy.execute()
self.assertEqual(4, len(solution.actions))
migration_types = collections.Counter(
[action.get('input_parameters')['migration_type']
for action in solution.actions])
self.assertEqual(2,
migration_types.get("migrate", 0))
self.assertEqual(2,
migration_types.get("retype", 0))
global_efficacy_value = solution.global_efficacy[2].get('value', 0)
self.assertEqual(100, global_efficacy_value)
def test_execute_migrate_volumes_select_no_volumes(self):
volume_on_src1 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_1"],
name="volume_1")
volume_on_src2 = self.fake_volume(host="src1@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src3@back1#pool1",
id=volume_uuid_mapping["volume_3"],
name="volume_3")
self.m_c_helper.get_volume_list.return_value = [
volume_on_src1,
volume_on_src2,
volume_on_src3,
]
self.m_n_helper.get_instance_list.return_value = []
solution = self.strategy.execute()
self.assertEqual(0, len(solution.actions))
def test_execute_live_migrate_instance_parallel(self):
instance_on_src1_1 = self.fake_instance(
host="src1",
@@ -615,6 +1044,7 @@ class TestZoneMigration(TestBaseStrategy):
name="volume_1")
volume_on_src2 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(host="src3@back2#pool1",
id=volume_uuid_mapping["volume_3"],
@@ -659,6 +1089,7 @@ class TestZoneMigration(TestBaseStrategy):
volume_on_src2 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_2"],
name="volume_2",
volume_type="type2",
project_id="pj1")
volume_on_src3 = self.fake_volume(host="src3@back2#pool1",
id=volume_uuid_mapping["volume_3"],
@@ -820,6 +1251,7 @@ class TestZoneMigration(TestBaseStrategy):
host="src2@back1#pool1",
size="2",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2")
volume_on_src3 = self.fake_volume(
host="src3@back2#pool1",
@@ -849,6 +1281,7 @@ class TestZoneMigration(TestBaseStrategy):
created_at="2017-10-30T00:00:00")
volume_on_src2 = self.fake_volume(host="src2@back1#pool1",
id=volume_uuid_mapping["volume_2"],
volume_type="type2",
name="volume_2",
created_at="1977-03-29T03:03:03")
volume_on_src3 = self.fake_volume(host="src3@back2#pool1",