From 4bb914d10bf1f2039da2cf168524404d561074bb Mon Sep 17 00:00:00 2001 From: Johannes Kulik Date: Mon, 27 Sep 2021 09:09:50 +0200 Subject: [PATCH] api: Send server-group update to relevant hosts When a server-group gets updated, we send an update to all hosts that got an update - which are the hosts the instances we removed/added belong to. Change-Id: Ica1e516358bf6301ab7a2c59631203721dbe658d --- nova/api/openstack/compute/server_groups.py | 13 ++++++- .../openstack/compute/test_server_groups.py | 34 +++++++++++++++---- 2 files changed, 39 insertions(+), 8 deletions(-) diff --git a/nova/api/openstack/compute/server_groups.py b/nova/api/openstack/compute/server_groups.py index 955c75936bf..ed77c831886 100644 --- a/nova/api/openstack/compute/server_groups.py +++ b/nova/api/openstack/compute/server_groups.py @@ -27,6 +27,7 @@ from nova.api.openstack.compute.schemas import server_groups as schema from nova.api.openstack import wsgi from nova.api import validation +from nova.compute import api as compute import nova.conf from nova import context as nova_context import nova.exception @@ -95,6 +96,10 @@ def _should_enable_custom_max_server_rules(context, rules): class ServerGroupController(wsgi.Controller): """The Server group API controller for the OpenStack API.""" + def __init__(self, **kwargs): + super(ServerGroupController, self).__init__(**kwargs) + self.compute_api = compute.API() + def _format_server_group(self, context, group, req, not_deleted_inst=None): """Format ServerGroup according to API version. @@ -311,7 +316,8 @@ def update(self, req, id, body): # retrieve all the instances to add, failing if one doesn't exist, # because we need to check the hosts against the policy and adding # non-existent instances doesn't make sense - found_instances_hosts = _get_not_deleted(context, members_to_add) + members_to_search = members_to_add | members_to_remove + found_instances_hosts = _get_not_deleted(context, members_to_search) missing_uuids = members_to_add - set(found_instances_hosts) if missing_uuids: msg = ("One or more members in add_members cannot be found: {}" @@ -386,4 +392,9 @@ def update(self, req, id, body): request_spec.instance_group = None request_spec.save() + # tell the compute hosts about the update, so they can sync if + # necessary + hosts_to_update = set(h for h in found_instances_hosts.values() if h) + self.compute_api.sync_server_group(context, hosts_to_update, id) + return {'server_group': self._format_server_group(context, sg, req)} diff --git a/nova/tests/unit/api/openstack/compute/test_server_groups.py b/nova/tests/unit/api/openstack/compute/test_server_groups.py index 74110daf21e..04ae92d6b54 100644 --- a/nova/tests/unit/api/openstack/compute/test_server_groups.py +++ b/nova/tests/unit/api/openstack/compute/test_server_groups.py @@ -784,7 +784,8 @@ def test_update_server_group_not_found(self): self.assertRaises(webob.exc.HTTPNotFound, self.controller.update, req, uuidsentinel.group1, body={}) - def test_update_server_group_empty(self): + @mock.patch('nova.compute.api.API.sync_server_group') + def test_update_server_group_empty(self, mock_sync): """We do not fail if the user doesn't request any changes""" req = fakes.HTTPRequest.blank('', version=self.wsgi_api_version) ctx = context.RequestContext('fake_user', 'fake') @@ -794,6 +795,7 @@ def test_update_server_group_empty(self): self.assertEqual(3, len(result_members)) for member in members: self.assertIn(member, result_members) + mock_sync.assert_not_called() def test_update_server_group_add_remove_overlap(self): """We do not accept changes, if there's a server to be both added and @@ -813,7 +815,8 @@ def test_update_server_group_add_remove_overlap(self): 'overlapping in {}'.format(uuidsentinel.uuid2), str(result)) - def test_update_server_group_remove_nonexisting(self): + @mock.patch('nova.compute.api.API.sync_server_group') + def test_update_server_group_remove_nonexisting(self, mock_sync): """Don't fail if the user tries to remove a server not being member of the server group. """ @@ -828,8 +831,10 @@ def test_update_server_group_remove_nonexisting(self): self.assertEqual(3, len(result_members)) for member in members: self.assertIn(member, result_members) + mock_sync.assert_not_called() - def test_update_server_group_add_already_added(self): + @mock.patch('nova.compute.api.API.sync_server_group') + def test_update_server_group_add_already_added(self, mock_sync): """Don't fail if the user adds a server that's already a member of the server group. """ @@ -844,6 +849,7 @@ def test_update_server_group_add_already_added(self): self.assertEqual(3, len(result_members)) for member in members: self.assertIn(member, result_members) + mock_sync.assert_not_called() def test_update_server_group_add_against_policy_affinity(self): """Fail if adding the server would break the policy.""" @@ -891,13 +897,14 @@ def test_update_server_group_add_against_policy_anti_affinity(self): "'anti-affinity'.".format(new_instance.uuid), str(result)) + @mock.patch('nova.compute.api.API.sync_server_group') @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') def test_update_server_group_add_with_remove_fixes_policy(self, - mock_req_spec): + mock_req_spec, + mock_sync): """Don't fail if adding a server would break the policy, but the remove in the same request fixes that. """ - """Fail if adding the server would break the policy.""" req = fakes.HTTPRequest.blank('', version=self.wsgi_api_version) ctx = context.RequestContext('fake_user', 'fake') @@ -920,6 +927,9 @@ def test_update_server_group_add_with_remove_fixes_policy(self, self.assertEqual(2, len(result_members)) for member in [instances[0].uuid, new_instance.uuid]: self.assertIn(member, result_members) + req_context = req.environ['nova.context'] + mock_sync.assert_called_with(req_context, set(['host2']), + ig_uuid) def test_update_server_group_add_nonexisting_instance(self): """Fail if the instances the user tries to add does not exist.""" @@ -935,9 +945,11 @@ def test_update_server_group_add_nonexisting_instance(self): '{}'.format(uuidsentinel.uuid1), str(result)) + @mock.patch('nova.compute.api.API.sync_server_group') @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') def test_update_server_group_add_instance_multiple_cells(self, - mock_req_spec): + mock_req_spec, + mock_sync): """Don't fail if the instance the user tries to add is in another cell. """ req = fakes.HTTPRequest.blank('', version=self.wsgi_api_version) @@ -957,9 +969,15 @@ def test_update_server_group_add_instance_multiple_cells(self, self.assertIn(member, result_members) for instance in new_instances: self.assertIn(instance.uuid, result_members) + expected_hosts = \ + set([i.host for i in instances] + [i.host for i in new_instances]) + req_context = req.environ['nova.context'] + mock_sync.assert_called_with(req_context, expected_hosts, ig_uuid) + @mock.patch('nova.compute.api.API.sync_server_group') @mock.patch.object(objects.RequestSpec, 'get_by_instance_uuid') - def test_update_server_group_add_against_soft_policy(self, mock_req_spec): + def test_update_server_group_add_against_soft_policy(self, mock_req_spec, + mock_sync): """Don't fail if the policy would fail, but it's a soft-* policy - they are best-effort by design. """ @@ -983,6 +1001,8 @@ def test_update_server_group_add_against_soft_policy(self, mock_req_spec): self.assertEqual(2, len(result_members)) for member in [instances[0].uuid, new_instance.uuid]: self.assertIn(member, result_members) + req_context = req.environ['nova.context'] + mock_sync.assert_called_with(req_context, set(['host1']), ig_uuid) @mock.patch('nova.objects.InstanceGroupList.get_by_instance_uuids') def test_update_server_group_add_already_in_other(self, mock_gbiu):