Skip to content

Commit

Permalink
api: Send server-group update to relevant hosts
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@joker-at-work @grandchild
joker-at-work authored and grandchild committed Apr 27, 2023
1 parent e34e05f commit 4bb914d
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 8 deletions.
13 changes: 12 additions & 1 deletion nova/api/openstack/compute/server_groups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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: {}"
Expand Down Expand Up @@ -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)}
34 changes: 27 additions & 7 deletions nova/tests/unit/api/openstack/compute/test_server_groups.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand All @@ -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
Expand All @@ -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.
"""
Expand All @@ -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.
"""
Expand All @@ -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."""
Expand Down Expand Up @@ -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')

Expand All @@ -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."""
Expand All @@ -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)
Expand All @@ -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.
"""
Expand All @@ -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):
Expand Down

0 comments on commit 4bb914d

Please sign in to comment.