openstack
/
nova
Code Issues Proposed changes

Merge "api: Align availability zone info with forced host" into stable/wallaby

This commit is contained in:
Zuul 2021-08-20 22:03:44 +00:00 committed by Gerrit Code Review
parent 732caaf296 58782403cd
commit 5453a3e757
4 changed files with 217 additions and 25 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

80
nova/api/openstack/compute/servers.py
View File

@ -545,6 +545,84 @@ class ServersController(wsgi.Controller):
create_kwargs['requested_networks'] = requested_networks
@staticmethod
def _validate_host_availability_zone(context, availability_zone, host):
"""Ensure the host belongs in the availability zone.
This is slightly tricky and it's probably worth recapping how host
aggregates and availability zones are related before reading. Hosts can
belong to zero or more host aggregates, but they will always belong to
exactly one availability zone. If the user has set the availability
zone key on one of the host aggregates that the host is a member of
then the host will belong to this availability zone. If the user has
not set the availability zone key on any of the host aggregates that
the host is a member of or the host is not a member of any host
aggregates, then the host will belong to the default availability zone.
Setting the availability zone key on more than one of host aggregates
that the host is a member of is an error and will be rejected by the
API.
Given the above, our host-availability zone check needs to vary
behavior based on whether we're requesting the default availability
zone or not. If we are not, then we simply ask "does this host belong
to a host aggregate and, if so, do any of the host aggregates have the
requested availability zone metadata set". By comparison, if we *are*
requesting the default availability zone then we want to ask the
inverse, or "does this host not belong to a host aggregate or, if it
does, is the availability zone information unset (or, naughty naughty,
set to the default) for each of the host aggregates". If both cases, if
the answer is no then we warn about the mismatch and then use the
actual availability zone of the host to avoid mismatches.
:param context: The nova auth request context
:param availability_zone: The name of the requested availability zone
:param host: The name of the requested host
:returns: The availability zone that should actually be used for the
request
"""
aggregates = objects.AggregateList.get_by_host(context, host=host)
if not aggregates:
# a host is assigned to the default availability zone if it is not
# a member of any host aggregates
if availability_zone == CONF.default_availability_zone:
return availability_zone
LOG.warning(
"Requested availability zone '%s' but forced host '%s' "
"does not belong to any availability zones; ignoring "
"requested availability zone to avoid bug #1934770",
availability_zone, host,
)
return None
# only one host aggregate will have the availability_zone field set so
# use the first non-null value
host_availability_zone = next(
(a.availability_zone for a in aggregates if a.availability_zone),
None,
)
if availability_zone == host_availability_zone:
# if there's an exact match, use what the user requested
return availability_zone
if (
availability_zone == CONF.default_availability_zone and
host_availability_zone is None
):
# special case the default availability zone since this won't (or
# rather shouldn't) be explicitly stored on any host aggregate
return availability_zone
# no match, so use the host's availability zone information, if any
LOG.warning(
"Requested availability zone '%s' but forced host '%s' "
"does not belong to this availability zone; overwriting "
"requested availability zone to avoid bug #1934770",
availability_zone, host,
)
return None
@staticmethod
def _process_hosts_for_create(
context, target, server_dict, create_kwargs, host, node):
@ -665,6 +743,8 @@ class ServersController(wsgi.Controller):
if host or node:
context.can(server_policies.SERVERS % 'create:forced_host',
target=target)
availability_zone = self._validate_host_availability_zone(
context, availability_zone, host)
if api_version_request.is_supported(req, min_version='2.74'):
self._process_hosts_for_create(context, target, server_dict,

12
nova/compute/api.py
View File

@ -2036,17 +2036,15 @@ class API(base.Base):
self._check_multiple_instances_with_neutron_ports(
requested_networks)
if availability_zone:
available_zones = availability_zones.\
get_availability_zones(context.elevated(), self.host_api,
get_only_available=True)
if forced_host is None and availability_zone not in \
available_zones:
if availability_zone and forced_host is None:
azs = availability_zones.get_availability_zones(
context.elevated(), self.host_api, get_only_available=True)
if availability_zone not in azs:
msg = _('The requested availability zone is not available')
raise exception.InvalidRequest(msg)
filter_properties = scheduler_utils.build_filter_properties(
scheduler_hints, forced_host, forced_node, instance_type)
scheduler_hints, forced_host, forced_node, instance_type)
return self._create_instance(
context, instance_type,

147
nova/tests/unit/api/openstack/compute/test_servers.py
View File

@ -4563,21 +4563,144 @@ class ServersControllerCreateTest(test.TestCase):
self.assertRaises(exception.ValidationError,
self.controller.create, req, body=body)
def test_create_az_with_leading_trailing_spaces(self):
def test_create_instance_az_with_leading_trailing_spaces(self):
self.body['server']['availability_zone'] = ' zone1 '
self.req.body = jsonutils.dump_as_bytes(self.body)
self.assertRaises(exception.ValidationError,
self.controller.create, self.req, body=self.body)
def test_create_az_with_leading_trailing_spaces_in_compat_mode(
self):
def test_create_instance_az_with_leading_trailing_spaces_compat_mode(self):
self.body['server']['name'] = ' abc def '
self.body['server']['availability_zones'] = ' zone1 '
self.body['server']['availability_zone'] = ' zone1 '
self.req.body = jsonutils.dump_as_bytes(self.body)
self.req.set_legacy_v2()
with mock.patch.object(availability_zones, 'get_availability_zones',
return_value=[' zone1 ']):
with mock.patch.object(
availability_zones, 'get_availability_zones',
return_value=[' zone1 '],
) as mock_get_azs:
self.controller.create(self.req, body=self.body)
mock_get_azs.assert_called_once()
def test_create_instance_invalid_az(self):
self.body['server']['availability_zone'] = 'zone1'
self.req.body = jsonutils.dump_as_bytes(self.body)
with mock.patch.object(
availability_zones, 'get_availability_zones',
return_value=['zone2'],
) as mock_get_azs:
self.assertRaises(
webob.exc.HTTPBadRequest,
self.controller.create,
self.req, body=self.body)
mock_get_azs.assert_called_once()
@mock.patch.object(objects.AggregateList, 'get_by_host')
@mock.patch.object(servers, 'LOG')
def test_create_instance_az_host(self, mock_log, mock_get_host_aggs):
"""Ensure we handle az:host format for 'availability_zone'."""
mock_get_host_aggs.return_value = objects.AggregateList(
objects=[
objects.Aggregate(metadata={'availability_zone': 'zone1'}),
],
)
self.body['server']['availability_zone'] = 'zone1:host1'
self.req.body = jsonutils.dump_as_bytes(self.body)
self.controller.create(self.req, body=self.body)
mock_get_host_aggs.assert_called_once()
mock_log.warning.assert_not_called()
@mock.patch.object(objects.AggregateList, 'get_by_host')
@mock.patch.object(servers, 'LOG')
def test_create_instance_az_host_mismatch_without_aggs(
self, mock_log, mock_get_host_aggs,
):
"""User requests an AZ but the host doesn't have one"""
mock_get_host_aggs.return_value = objects.AggregateList(objects=[])
self.body['server']['availability_zone'] = 'zone1:host1'
self.req.body = jsonutils.dump_as_bytes(self.body)
self.controller.create(self.req, body=self.body)
mock_get_host_aggs.assert_called_once()
# we should see a log since the host doesn't belong to the requested AZ
self.assertIn('bug #1934770', mock_log.warning.call_args[0][0])
@mock.patch.object(objects.AggregateList, 'get_by_host')
@mock.patch.object(servers, 'LOG')
def test_create_instance_az_host_mismatch_without_aggs_in_default_az(
self, mock_log, mock_get_host_aggs,
):
"""User requests the default AZ and host isn't in any explicit AZ"""
self.flags(default_availability_zone='zone1')
mock_get_host_aggs.return_value = objects.AggregateList(objects=[])
self.body['server']['availability_zone'] = 'zone1:host1'
self.req.body = jsonutils.dump_as_bytes(self.body)
self.controller.create(self.req, body=self.body)
mock_get_host_aggs.assert_called_once()
# we shouldn't see a log since the host is not in any aggregate and
# therefore is in the default AZ
mock_log.warning.assert_not_called()
@mock.patch.object(objects.AggregateList, 'get_by_host')
@mock.patch.object(servers, 'LOG')
def test_create_instance_az_host_mismatch_with_aggs(
self, mock_log, mock_get_host_aggs,
):
"""User requests an AZ but the host has a different one."""
mock_get_host_aggs.return_value = objects.AggregateList(
objects=[
objects.Aggregate(metadata={'availability_zone': 'zone2'}),
],
)
self.body['server']['availability_zone'] = 'zone1:host1'
self.req.body = jsonutils.dump_as_bytes(self.body)
self.controller.create(self.req, body=self.body)
mock_get_host_aggs.assert_called_once()
# we should see a log since the host belongs to a different AZ
self.assertIn('bug #1934770', mock_log.warning.call_args[0][0])
@mock.patch.object(objects.AggregateList, 'get_by_host')
@mock.patch.object(servers, 'LOG')
def test_create_instance_az_host_mismatch_with_aggs_in_default_az(
self, mock_log, mock_get_host_aggs,
):
"""User requests the default AZ and host is in aggregates without AZ"""
self.flags(default_availability_zone='zone1')
mock_get_host_aggs.return_value = objects.AggregateList(
objects=[objects.Aggregate(metadata={})],
)
self.body['server']['availability_zone'] = 'zone1:host1'
self.req.body = jsonutils.dump_as_bytes(self.body)
self.controller.create(self.req, body=self.body)
mock_get_host_aggs.assert_called_once()
# we shouldn't see a log since none of the host aggregates have an
# explicit AZ set and the host is therefore in the default AZ
mock_log.warning.assert_not_called()
def test_create_instance_invalid_az_format(self):
self.body['server']['availability_zone'] = 'invalid::::zone'
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create,
self.req, body=self.body)
def test_create_instance_invalid_az_as_int(self):
self.body['server']['availability_zone'] = 123
self.assertRaises(exception.ValidationError,
self.controller.create,
self.req, body=self.body)
def test_create_instance(self):
self.stub_out('uuid.uuid4', lambda: FAKE_UUID)
@ -6181,18 +6304,6 @@ class ServersControllerCreateTest(test.TestCase):
self.controller.create,
self.req, body=self.body)
def test_create_instance_invalid_availability_zone(self):
self.body['server']['availability_zone'] = 'invalid::::zone'
self.assertRaises(webob.exc.HTTPBadRequest,
self.controller.create,
self.req, body=self.body)
def test_create_instance_invalid_availability_zone_as_int(self):
self.body['server']['availability_zone'] = 123
self.assertRaises(exception.ValidationError,
self.controller.create,
self.req, body=self.body)
@mock.patch.object(compute_api.API, 'create',
side_effect=exception.FixedIpNotFoundForAddress(
address='dummy'))

3
nova/tests/unit/policies/test_servers.py
View File

@ -420,6 +420,9 @@ class ServersPolicyTest(base.BasePolicyTest):
@mock.patch('nova.compute.api.API.create')
@mock.patch('nova.compute.api.API.parse_availability_zone')
@mock.patch.object(
servers.ServersController, '_validate_host_availability_zone',
new=mock.Mock(return_value=None))
def test_create_forced_host_server_policy(self, mock_az, mock_create):
# 'create' policy is checked before 'create:forced_host' so
# we have to allow it for everyone otherwise it will