diff --git a/manila/cmd/manage.py b/manila/cmd/manage.py index a53b60224d..ca684c5b0b 100644 --- a/manila/cmd/manage.py +++ b/manila/cmd/manage.py @@ -375,24 +375,28 @@ class ShareCommands(object): @args('--force', required=False, type=bool, default=False, help="Ignore validations.") def update_host(self, current_host, new_host, force=False): - """Modify the host name associated with a share and share server. + """Modify the host name associated with resources. Particularly to recover from cases where one has moved their Manila Share node, or modified their 'host' opt or their backend section name in the manila configuration file. + Affects shares, share servers and share groups """ if not force: self._validate_hosts(current_host, new_host) ctxt = context.get_admin_context() - updated = db.share_instances_host_update(ctxt, current_host, new_host) - print("Updated host of %(count)s share instances on %(chost)s " - "to %(nhost)s." % {'count': updated, 'chost': current_host, - 'nhost': new_host}) - - servers = db.share_servers_host_update(ctxt, current_host, new_host) - print("Updated host of %(count)s share servers on %(chost)s " - "to %(nhost)s." % {'count': servers, 'chost': current_host, - 'nhost': new_host}) + updated = db.share_resources_host_update(ctxt, current_host, new_host) + msg = ("Updated host of %(si_count)d share instances, " + "%(sg_count)d share groups and %(ss_count)d share servers on " + "%(chost)s to %(nhost)s.") + msg_args = { + 'si_count': updated['instances'], + 'sg_count': updated['groups'], + 'ss_count': updated['servers'], + 'chost': current_host, + 'nhost': new_host, + } + print(msg % msg_args) CATEGORIES = { diff --git a/manila/db/api.py b/manila/db/api.py index 0fc8c9a70e..033bf91bf1 100644 --- a/manila/db/api.py +++ b/manila/db/api.py @@ -339,11 +339,6 @@ def share_instances_status_update(context, share_instance_ids, values): context, share_instance_ids, values) -def share_instances_host_update(context, current_host, new_host): - """Update the host attr of all share instances that are on current_host.""" - return IMPL.share_instances_host_update(context, current_host, new_host) - - def share_instances_get_all(context, filters=None): """Returns all share instances.""" return IMPL.share_instances_get_all(context, filters=filters) @@ -1026,11 +1021,6 @@ def share_server_backend_details_set(context, share_server_id, server_details): server_details) -def share_servers_host_update(context, current_host, new_host): - """Update the host attr of all share servers that are on current_host.""" - return IMPL.share_servers_host_update(context, current_host, new_host) - - ################## @@ -1304,6 +1294,11 @@ def share_group_snapshot_member_update(context, member_id, values): return IMPL.share_group_snapshot_member_update(context, member_id, values) +def share_resources_host_update(context, current_host, new_host): + """Update the host attr of all share resources that are on current_host.""" + return IMPL.share_resources_host_update(context, current_host, new_host) + + #################### def share_replicas_get_all(context, with_share_server=False, diff --git a/manila/db/sqlalchemy/api.py b/manila/db/sqlalchemy/api.py index ab3b89753f..bba238c85a 100644 --- a/manila/db/sqlalchemy/api.py +++ b/manila/db/sqlalchemy/api.py @@ -389,6 +389,33 @@ QUOTA_SYNC_FUNCTIONS = { } +################### + +@require_admin_context +def share_resources_host_update(context, current_host, new_host): + """Updates the 'host' attribute of resources""" + + resources = { + 'instances': models.ShareInstance, + 'servers': models.ShareServer, + 'groups': models.ShareGroup, + } + result = {} + + session = get_session() + with session.begin(): + for res_name, res_model in resources.items(): + host_field = res_model.host + query = model_query( + context, res_model, session=session, read_deleted="no", + ).filter(host_field.like('{}%'.format(current_host))) + count = query.update( + {host_field: func.replace(host_field, current_host, new_host)}, + synchronize_session=False) + result.update({res_name: count}) + return result + + ################### @@ -1395,20 +1422,6 @@ def _share_instance_create(context, share_id, values, session): session=session) -@require_admin_context -def share_instances_host_update(context, current_host, new_host): - session = get_session() - host_field = models.ShareInstance.host - with session.begin(): - query = model_query( - context, models.ShareInstance, session=session, read_deleted="no", - ).filter(host_field.like('{}%'.format(current_host))) - result = query.update( - {host_field: func.replace(host_field, current_host, new_host)}, - synchronize_session=False) - return result - - @require_context def share_instance_update(context, share_instance_id, values, with_share_data=False): @@ -4051,20 +4064,6 @@ def share_server_backend_details_delete(context, share_server_id, item.soft_delete(session) -@require_admin_context -def share_servers_host_update(context, current_host, new_host): - session = get_session() - host_field = models.ShareServer.host - with session.begin(): - query = model_query( - context, models.ShareServer, session=session, read_deleted="no", - ).filter(host_field.like('{}%'.format(current_host))) - result = query.update( - {host_field: func.replace(host_field, current_host, new_host)}, - synchronize_session=False) - return result - - ################### def _driver_private_data_query(session, context, entity_id, key=None, diff --git a/manila/tests/cmd/test_manage.py b/manila/tests/cmd/test_manage.py index a9657f1398..e855d9746c 100644 --- a/manila/tests/cmd/test_manage.py +++ b/manila/tests/cmd/test_manage.py @@ -404,13 +404,13 @@ class ManilaCmdManageTestCase(test.TestCase): def test_share_update_host_fail_validation(self, current_host, new_host): self.mock_object(context, 'get_admin_context', mock.Mock(return_value='admin_ctxt')) - self.mock_object(db, 'share_instances_host_update') + self.mock_object(db, 'share_resources_host_update') self.assertRaises(SystemExit, self.share_cmds.update_host, current_host, new_host) - self.assertFalse(db.share_instances_host_update.called) + self.assertFalse(db.share_resources_host_update.called) @ddt.data({'current_host': 'controller-0@fancystore01#pool100', 'new_host': 'controller-0@fancystore02#pool0'}, @@ -422,23 +422,18 @@ class ManilaCmdManageTestCase(test.TestCase): 'new_host': 'controller-1@fancystore02', 'force': True}) @ddt.unpack def test_share_update_host(self, current_host, new_host, force=False): + db_op = {'instances': 3, 'groups': 4, 'servers': 2} self.mock_object(context, 'get_admin_context', mock.Mock(return_value='admin_ctxt')) - self.mock_object(db, 'share_instances_host_update', - mock.Mock(return_value=20)) - self.mock_object(db, 'share_servers_host_update', - mock.Mock(return_value=5)) + self.mock_object(db, 'share_resources_host_update', + mock.Mock(return_value=db_op)) with mock.patch('sys.stdout', new=six.StringIO()) as intercepted_op: self.share_cmds.update_host(current_host, new_host, force) - expected_op_si = ("Updated host of 20 share instances on " - "%(chost)s to %(nhost)s." % - {'chost': current_host, 'nhost': new_host}) - expected_op_sv = ("Updated host of 5 share servers on " - "%(chost)s to %(nhost)s." % - {'chost': current_host, 'nhost': new_host}) - self.assertEqual(expected_op_si + "\n" + expected_op_sv, - intercepted_op.getvalue().strip()) - db.share_instances_host_update.assert_called_once_with( + expected_op = ("Updated host of 3 share instances, 4 share groups and " + "2 share servers on %(chost)s to %(nhost)s." % + {'chost': current_host, 'nhost': new_host}) + self.assertEqual(expected_op, intercepted_op.getvalue().strip()) + db.share_resources_host_update.assert_called_once_with( 'admin_ctxt', current_host, new_host) diff --git a/manila/tests/db/sqlalchemy/test_api.py b/manila/tests/db/sqlalchemy/test_api.py index 20bae61647..1f6eab6287 100644 --- a/manila/tests/db/sqlalchemy/test_api.py +++ b/manila/tests/db/sqlalchemy/test_api.py @@ -4003,16 +4003,18 @@ class BackendInfoDatabaseAPITestCase(test.TestCase): @ddt.ddt -class ShareInstancesTestCase(test.TestCase): +class ShareResourcesAPITestCase(test.TestCase): def setUp(self): - super(ShareInstancesTestCase, self).setUp() + super(ShareResourcesAPITestCase, self).setUp() self.context = context.get_admin_context() @ddt.data('controller-100', 'controller-0@otherstore03', 'controller-0@otherstore01#pool200') - def test_share_instances_host_update_no_matches(self, current_host): + def test_share_resources_host_update_no_matches(self, current_host): share_id = uuidutils.generate_uuid() + share_network_id = uuidutils.generate_uuid() + share_network_subnet_id = uuidutils.generate_uuid() if '@' in current_host: if '#' in current_host: new_host = 'new-controller-X@backendX#poolX' @@ -4020,7 +4022,8 @@ class ShareInstancesTestCase(test.TestCase): new_host = 'new-controller-X@backendX' else: new_host = 'new-controller-X' - instances = [ + resources = [ # noqa + # share instances db_utils.create_share_instance( share_id=share_id, host='controller-0@fancystore01#pool100', @@ -4033,28 +4036,71 @@ class ShareInstancesTestCase(test.TestCase): share_id=share_id, host='controller-2@beststore07#pool200', status=constants.STATUS_DELETING), - ] - db_utils.create_share(id=share_id, instances=instances) + # share groups + db_utils.create_share_group( + share_network_id=share_network_id, + host='controller-0@fancystore01#pool200', + status=constants.STATUS_AVAILABLE), + db_utils.create_share_group( + share_network_id=share_network_id, + host='controller-0@otherstore02#pool100', + status=constants.STATUS_ERROR), + db_utils.create_share_group( + share_network_id=share_network_id, + host='controller-2@beststore07#pool100', + status=constants.STATUS_DELETING), + # share servers + db_utils.create_share_server( + share_network_subnet_id=share_network_subnet_id, + host='controller-0@fancystore01', + status=constants.STATUS_ACTIVE), + db_utils.create_share_server( + share_network_subnet_id=share_network_subnet_id, + host='controller-0@otherstore02#pool100', + status=constants.STATUS_ERROR), + db_utils.create_share_server( + share_network_subnet_id=share_network_subnet_id, + host='controller-2@beststore07', + status=constants.STATUS_DELETING), - updates = db_api.share_instances_host_update(self.context, + ] + + updates = db_api.share_resources_host_update(self.context, current_host, new_host) + expected_updates = {'instances': 0, 'servers': 0, 'groups': 0} + self.assertDictMatch(expected_updates, updates) + # validate that resources are unmodified: share_instances = db_api.share_instances_get_all( self.context, filters={'share_id': share_id}) - self.assertEqual(0, updates) + share_groups = db_api.share_group_get_all( + self.context, filters={'share_network_id': share_network_id}) + share_servers = db_api._server_get_query(self.context).filter_by( + share_network_subnet_id=share_network_subnet_id).all() + self.assertEqual(3, len(share_instances)) + self.assertEqual(3, len(share_groups)) + self.assertEqual(3, len(share_servers)) for share_instance in share_instances: self.assertTrue(not share_instance['host'].startswith(new_host)) + for share_group in share_groups: + self.assertTrue(not share_group['host'].startswith(new_host)) + for share_server in share_servers: + self.assertTrue(not share_server['host'].startswith(new_host)) - @ddt.data({'current_host': 'controller-2', 'expected_updates': 1}, - {'current_host': 'controller-0@fancystore01', - 'expected_updates': 2}, - {'current_host': 'controller-0@fancystore01#pool100', - 'expected_updates': 1}) + @ddt.data( + {'current_host': 'controller-2', + 'expected_updates': {'instances': 1, 'servers': 2, 'groups': 1}}, + {'current_host': 'controller-0@fancystore01', + 'expected_updates': {'instances': 2, 'servers': 1, 'groups': 2}}, + {'current_host': 'controller-0@fancystore01#pool100', + 'expected_updates': {'instances': 1, 'servers': 1, 'groups': 0}}) @ddt.unpack - def test_share_instance_host_update_partial_matches(self, current_host, - expected_updates): + def test_share_resources_host_update_partial_matches(self, current_host, + expected_updates): share_id = uuidutils.generate_uuid() + share_network_id = uuidutils.generate_uuid() + share_network_subnet_id = uuidutils.generate_uuid() if '@' in current_host: if '#' in current_host: new_host = 'new-controller-X@backendX#poolX' @@ -4062,7 +4108,11 @@ class ShareInstancesTestCase(test.TestCase): new_host = 'new-controller-X@backendX' else: new_host = 'new-controller-X' - instances = [ + total_updates_expected = (expected_updates['instances'] + + expected_updates['groups'] + + expected_updates['servers']) + resources = [ # noqa + # share instances db_utils.create_share_instance( share_id=share_id, host='controller-0@fancystore01#pool100', @@ -4075,19 +4125,50 @@ class ShareInstancesTestCase(test.TestCase): share_id=share_id, host='controller-2@beststore07#pool200', status=constants.STATUS_DELETING), + # share groups + db_utils.create_share_group( + share_network_id=share_network_id, + host='controller-0@fancystore01#pool101', + status=constants.STATUS_ACTIVE), + db_utils.create_share_group( + share_network_id=share_network_id, + host='controller-0@fancystore01#pool101', + status=constants.STATUS_ERROR), + db_utils.create_share_group( + share_network_id=share_network_id, + host='controller-2@beststore07#pool200', + status=constants.STATUS_DELETING), + # share servers + db_utils.create_share_server( + share_network_subnet_id=share_network_subnet_id, + host='controller-0@fancystore01#pool100', + status=constants.STATUS_ACTIVE), + db_utils.create_share_server( + share_network_subnet_id=share_network_subnet_id, + host='controller-2@fancystore01', + status=constants.STATUS_ERROR), + db_utils.create_share_server( + share_network_subnet_id=share_network_subnet_id, + host='controller-2@beststore07#pool200', + status=constants.STATUS_DELETING), ] - db_utils.create_share(id=share_id, instances=instances) - actual_updates = db_api.share_instances_host_update( + actual_updates = db_api.share_resources_host_update( self.context, current_host, new_host) share_instances = db_api.share_instances_get_all( self.context, filters={'share_id': share_id}) + share_groups = db_api.share_group_get_all( + self.context, filters={'share_network_id': share_network_id}) + share_servers = db_api._server_get_query(self.context).filter_by( + share_network_subnet_id=share_network_subnet_id).all() - host_updates = [si for si in share_instances if - si['host'].startswith(new_host)] - self.assertEqual(actual_updates, expected_updates) - self.assertEqual(expected_updates, len(host_updates)) + updated_resources = [ + res for res in share_instances + share_groups + share_servers + if res['host'].startswith(new_host) + ] + self.assertEqual(expected_updates, actual_updates) + self.assertEqual(total_updates_expected, len(updated_resources)) def test_share_instances_status_update(self): for i in range(1, 3): diff --git a/releasenotes/notes/bug-1881098-add-share-server-update-to-manila-manage-share-update-host-bbbc4fe2da48cae9.yaml b/releasenotes/notes/bug-1881098-1895323-manila-manage-update-host-fixes-bbbc4fe2da48cae9.yaml similarity index 55% rename from releasenotes/notes/bug-1881098-add-share-server-update-to-manila-manage-share-update-host-bbbc4fe2da48cae9.yaml rename to releasenotes/notes/bug-1881098-1895323-manila-manage-update-host-fixes-bbbc4fe2da48cae9.yaml index 54082f821d..0699446bf4 100644 --- a/releasenotes/notes/bug-1881098-add-share-server-update-to-manila-manage-share-update-host-bbbc4fe2da48cae9.yaml +++ b/releasenotes/notes/bug-1881098-1895323-manila-manage-update-host-fixes-bbbc4fe2da48cae9.yaml @@ -2,4 +2,4 @@ fixes: - | The ``manila-manage share update_host`` command now updates the host - attribute of share servers in addition to shares. + attribute of share servers and share groups in addition to shares.