From e5e929f019c05fa57f6b216855b9246b7a5f8d48 Mon Sep 17 00:00:00 2001 From: Hang Yang Date: Thu, 17 Jan 2019 16:57:56 -0800 Subject: [PATCH] Fix port dns_name reset When external DNS service is enabled, use user's context to request dns_name reset instead of using admin context. The dns record need be found in user's zone and recordset. Also add show_network to NeutronFixture as needed in functional test given I621f5111d1cccf3dc2c2ea6767ce879bc286e42b has not been backported yet in stable/queens. Change-Id: I35335b501f8961b9ac8e5f92e0686e402b78617b Closes-Bug: #1812110 (cherry picked from commit 1b797f6f7e99fdef380340d6fe29e4004be48781) (cherry picked from commit d2662df48526a2a4ddcd4a7bdbb6108102fe8321) --- nova/network/neutronv2/api.py | 48 +++++++++++++++++++---- nova/tests/fixtures.py | 7 ++++ nova/tests/unit/network/test_neutronv2.py | 44 +++++++++++++++++++-- 3 files changed, 88 insertions(+), 11 deletions(-) diff --git a/nova/network/neutronv2/api.py b/nova/network/neutronv2/api.py index 4692293cae40..57c62abe8765 100644 --- a/nova/network/neutronv2/api.py +++ b/nova/network/neutronv2/api.py @@ -513,8 +513,8 @@ class API(base_api.NetworkAPI): def _unbind_ports(self, context, ports, neutron, port_client=None): - """Unbind the given ports by clearing their device_id and - device_owner. + """Unbind the given ports by clearing their device_id, + device_owner and dns_name. :param context: The request context. :param ports: list of port IDs. @@ -525,6 +525,7 @@ class API(base_api.NetworkAPI): if port_client is None: # Requires admin creds to set port bindings port_client = get_client(context, admin=True) + networks = {} for port_id in ports: # A port_id is optional in the NetworkRequest object so check here # in case the caller forgot to filter the list. @@ -535,19 +536,28 @@ class API(base_api.NetworkAPI): try: port = self._show_port(context, port_id, neutron_client=neutron, - fields=BINDING_PROFILE) + fields=[BINDING_PROFILE, 'network_id']) except exception.PortNotFound: LOG.debug('Unable to show port %s as it no longer ' 'exists.', port_id) return except Exception: - # NOTE: In case we can't retrieve the binding:profile assume - # that they are empty + # NOTE: In case we can't retrieve the binding:profile or + # network info assume that they are empty LOG.exception("Unable to get binding:profile for port '%s'", port_id) port_profile = {} + network = {} else: port_profile = port.get(BINDING_PROFILE, {}) + net_id = port.get('network_id') + if net_id in networks: + network = networks.get(net_id) + else: + network = neutron.show_network(net_id, + fields=['dns_domain'] + ).get('network') + networks[net_id] = network # NOTE: We're doing this to remove the binding information # for the physical device but don't want to overwrite the other @@ -556,9 +566,12 @@ class API(base_api.NetworkAPI): if profile_key in port_profile: del port_profile[profile_key] port_req_body['port'][BINDING_PROFILE] = port_profile - if self._has_dns_extension(): - port_req_body['port']['dns_name'] = '' + # NOTE: For internal DNS integration (network does not have a + # dns_domain), or if we cannot retrieve network info, we use the + # admin client to reset dns_name. + if self._has_dns_extension() and not network.get('dns_domain'): + port_req_body['port']['dns_name'] = '' try: port_client.update_port(port_id, port_req_body) except neutron_client_exc.PortNotFoundClient: @@ -567,6 +580,10 @@ class API(base_api.NetworkAPI): except Exception: LOG.exception("Unable to clear device ID for port '%s'", port_id) + # NOTE: For external DNS integration, we use the neutron client + # with user's context to reset the dns_name since the recordset is + # under user's zone. + self._reset_port_dns_name(network, port_id, neutron) def _validate_requested_port_ids(self, context, instance, neutron, requested_networks): @@ -1229,6 +1246,23 @@ class API(base_api.NetworkAPI): 'name') % {'hostname': instance.hostname}) raise exception.InvalidInput(reason=msg) + def _reset_port_dns_name(self, network, port_id, neutron_client): + """Reset an instance port dns_name attribute to empty when using + external DNS service. + + _unbind_ports uses a client with admin context to reset the dns_name if + the DNS extension is enabled and network does not have dns_domain set. + When external DNS service is enabled, we use this method to make the + request with a Neutron client using user's context, so that the DNS + record can be found under user's zone and domain. + """ + if self._has_dns_extension() and network.get('dns_domain'): + try: + port_req_body = {'port': {'dns_name': ''}} + neutron_client.update_port(port_id, port_req_body) + except neutron_client_exc.NeutronClientException: + LOG.exception("Failed to reset dns_name for port %s", port_id) + def _delete_ports(self, neutron, instance, ports, raise_if_fail=False): exceptions = [] for port in ports: diff --git a/nova/tests/fixtures.py b/nova/tests/fixtures.py index ddd0ab4e8143..570160b0a8a6 100644 --- a/nova/tests/fixtures.py +++ b/nova/tests/fixtures.py @@ -27,6 +27,7 @@ import warnings import fixtures from keystoneauth1 import session as ks import mock +from neutronclient.common import exceptions as neutron_client_exc from oslo_concurrency import lockutils from oslo_config import cfg import oslo_messaging as messaging @@ -1250,6 +1251,12 @@ class NeutronFixture(fixtures.Fixture): if current_port['id'] == port: self._ports.remove(current_port) + def show_network(self, network, **_params): + network = self._get_first_id_match(network, self._networks) + if network is None: + raise neutron_client_exc.NetworkNotFoundClient() + return {'network': network} + def list_networks(self, retrieve_all=True, **_params): return copy.deepcopy({'networks': self._networks}) diff --git a/nova/tests/unit/network/test_neutronv2.py b/nova/tests/unit/network/test_neutronv2.py index 30bc659749d9..70c6ffd8981d 100644 --- a/nova/tests/unit/network/test_neutronv2.py +++ b/nova/tests/unit/network/test_neutronv2.py @@ -1574,8 +1574,13 @@ class TestNeutronv2(TestNeutronv2Base): self.moxed_client) if requested_networks: for net, fip, port, request_id in requested_networks: - self.moxed_client.show_port(port, fields='binding:profile' - ).AndReturn({'port': ret_data[0]}) + self.moxed_client.show_port(port, fields=[ + 'binding:profile', 'network_id'] + ).AndReturn({'port': ret_data[0]}) + self.moxed_client.show_network( + ret_data[0]['network_id'], + fields=['dns_domain']).AndReturn( + {'network': {'id': ret_data[0]['network_id']}}) self.moxed_client.update_port(port) for port in ports: self.moxed_client.delete_port(port).InAnyOrder("delete_port_group") @@ -4685,8 +4690,14 @@ class TestNeutronv2WithMock(test.TestCase): self.context) @mock.patch('nova.network.neutronv2.api.API._show_port') - def test_unbind_ports_reset_dns_name(self, mock_show): + def test_unbind_ports_reset_dns_name_by_admin(self, mock_show): neutron = mock.Mock() + neutron.show_network.return_value = { + 'network': { + 'id': 'net1', + 'dns_domain': None + } + } port_client = mock.Mock() self.api.extensions = [constants.DNS_INTEGRATION] ports = [uuids.port_id] @@ -4699,6 +4710,31 @@ class TestNeutronv2WithMock(test.TestCase): 'dns_name': ''}} port_client.update_port.assert_called_once_with( uuids.port_id, port_req_body) + neutron.update_port.assert_not_called() + + @mock.patch('nova.network.neutronv2.api.API._show_port') + def test_unbind_ports_reset_dns_name_by_non_admin(self, mock_show): + neutron = mock.Mock() + neutron.show_network.return_value = { + 'network': { + 'id': 'net1', + 'dns_domain': 'test.domain' + } + } + port_client = mock.Mock() + self.api.extensions = [constants.DNS_INTEGRATION] + ports = [uuids.port_id] + mock_show.return_value = {'id': uuids.port} + self.api._unbind_ports(self.context, ports, neutron, port_client) + admin_port_req_body = {'port': {'binding:host_id': None, + 'binding:profile': {}, + 'device_id': '', + 'device_owner': ''}} + non_admin_port_req_body = {'port': {'dns_name': ''}} + port_client.update_port.assert_called_once_with( + uuids.port_id, admin_port_req_body) + neutron.update_port.assert_called_once_with( + uuids.port_id, non_admin_port_req_body) @mock.patch('nova.network.neutronv2.api.API._show_port') def test_unbind_ports_reset_binding_profile(self, mock_show): @@ -4864,7 +4900,7 @@ class TestNeutronv2WithMock(test.TestCase): neutron_client, neutron_client) mock_show.assert_called_once_with( mock.ANY, uuids.port_id, - fields='binding:profile', + fields=['binding:profile', 'network_id'], neutron_client=mock.ANY) mock_log.assert_not_called()