diff --git a/doc/source/strategies/zone_migration.rst b/doc/source/strategies/zone_migration.rst index c29b3381f..0d062a776 100644 --- a/doc/source/strategies/zone_migration.rst +++ b/doc/source/strategies/zone_migration.rst @@ -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 ============= ======= ======== ========= ======================== diff --git a/releasenotes/notes/zone_migrate_src_type-642186730a4f354e.yaml b/releasenotes/notes/zone_migrate_src_type-642186730a4f354e.yaml new file mode 100644 index 000000000..72d2d915d --- /dev/null +++ b/releasenotes/notes/zone_migrate_src_type-642186730a4f354e.yaml @@ -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 diff --git a/watcher/decision_engine/strategy/strategies/zone_migration.py b/watcher/decision_engine/strategy/strategies/zone_migration.py index fbd6e4aef..80d7d58e0 100644 --- a/watcher/decision_engine/strategy/strategies/zone_migration.py +++ b/watcher/decision_engine/strategy/strategies/zone_migration.py @@ -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 diff --git a/watcher/tests/api/v1/test_audits.py b/watcher/tests/api/v1/test_audits.py index 7627f3e4a..d4e52820b 100644 --- a/watcher/tests/api/v1/test_audits.py +++ b/watcher/tests/api/v1/test_audits.py @@ -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) diff --git a/watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py b/watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py index fee9d8d22..284d8a031 100644 --- a/watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py +++ b/watcher/tests/decision_engine/strategy/strategies/test_zone_migration.py @@ -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",