diff --git a/neutron/agent/l3/dvr_fip_ns.py b/neutron/agent/l3/dvr_fip_ns.py index b30e80f22fb..441005c89d2 100644 --- a/neutron/agent/l3/dvr_fip_ns.py +++ b/neutron/agent/l3/dvr_fip_ns.py @@ -112,22 +112,7 @@ class FipNamespace(namespaces.Namespace): self.driver.init_l3(interface_name, ip_cidrs, namespace=ns_name, clean_connections=True) - for fixed_ip in ex_gw_port['fixed_ips']: - ip_lib.send_ip_addr_adv_notif(ns_name, - interface_name, - fixed_ip['ip_address'], - self.agent_conf) - - for subnet in ex_gw_port['subnets']: - gw_ip = subnet.get('gateway_ip') - if gw_ip: - is_gateway_not_in_subnet = not ipam_utils.check_subnet_ip( - subnet.get('cidr'), gw_ip) - ipd = ip_lib.IPDevice(interface_name, - namespace=ns_name) - if is_gateway_not_in_subnet: - ipd.route.add_route(gw_ip, scope='link') - ipd.route.add_gateway(gw_ip) + self.update_gateway_port(ex_gw_port) cmd = ['sysctl', '-w', 'net.ipv4.conf.%s.proxy_arp=1' % interface_name] # TODO(Carl) mlavelle's work has self.ip_wrapper @@ -195,13 +180,53 @@ class FipNamespace(namespaces.Namespace): Request port creation from Plugin then creates Floating IP namespace and adds gateway port. """ - self.agent_gateway_port = agent_gateway_port - self.create() iface_name = self.get_ext_device_name(agent_gateway_port['id']) self._gateway_added(agent_gateway_port, iface_name) + def _check_for_gateway_ip_change(self, new_agent_gateway_port): + + def get_gateway_ips(gateway_port): + gw_ips = {} + if gateway_port: + for subnet in gateway_port.get('subnets', []): + gateway_ip = subnet.get('gateway_ip', None) + if gateway_ip: + ip_version = ip_lib.get_ip_version(gateway_ip) + gw_ips[ip_version] = gateway_ip + return gw_ips + + new_gw_ips = get_gateway_ips(new_agent_gateway_port) + old_gw_ips = get_gateway_ips(self.agent_gateway_port) + + return new_gw_ips != old_gw_ips + + def update_gateway_port(self, agent_gateway_port): + gateway_ip_not_changed = self.agent_gateway_port and ( + not self._check_for_gateway_ip_change(agent_gateway_port)) + self.agent_gateway_port = agent_gateway_port + if gateway_ip_not_changed: + return + + ns_name = self.get_name() + interface_name = self.get_ext_device_name(agent_gateway_port['id']) + for fixed_ip in agent_gateway_port['fixed_ips']: + ip_lib.send_ip_addr_adv_notif(ns_name, + interface_name, + fixed_ip['ip_address'], + self.agent_conf) + + ipd = ip_lib.IPDevice(interface_name, namespace=ns_name) + for subnet in agent_gateway_port['subnets']: + gw_ip = subnet.get('gateway_ip') + if gw_ip: + is_gateway_not_in_subnet = not ipam_utils.check_subnet_ip( + subnet.get('cidr'), gw_ip) + if is_gateway_not_in_subnet: + ipd.route.add_route(gw_ip, scope='link') + ipd.route.add_gateway(gw_ip) + def _internal_ns_interface_added(self, ip_cidr, interface_name, ns_name): ip_wrapper = ip_lib.IPWrapper(namespace=ns_name) diff --git a/neutron/agent/l3/dvr_local_router.py b/neutron/agent/l3/dvr_local_router.py index a540f2a658b..9abbf36d22e 100644 --- a/neutron/agent/l3/dvr_local_router.py +++ b/neutron/agent/l3/dvr_local_router.py @@ -445,11 +445,14 @@ class DvrLocalRouter(dvr_router_base.DvrRouterBase): LOG.error(_LE("No FloatingIP agent gateway port " "returned from server for 'network-id': " "%s"), ex_gw_port['network_id']) - if is_first and fip_agent_port: + if fip_agent_port: if 'subnets' not in fip_agent_port: LOG.error(_LE('Missing subnet/agent_gateway_port')) else: - self.fip_ns.create_gateway_port(fip_agent_port) + if is_first: + self.fip_ns.create_gateway_port(fip_agent_port) + else: + self.fip_ns.update_gateway_port(fip_agent_port) if (self.fip_ns.agent_gateway_port and (self.dist_fip_count == 0 or is_first)): diff --git a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py index 0275605f438..f57b34a34e4 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py +++ b/neutron/tests/unit/agent/l3/test_dvr_fip_ns.py @@ -12,6 +12,7 @@ # License for the specific language governing permissions and limitations # under the License. +import copy import mock from oslo_utils import uuidutils @@ -69,65 +70,94 @@ class TestDvrFipNs(base.BaseTestCase): self.assertNotIn('20.0.0.30', self.fip_ns._rule_priorities.allocations) self.assertIn(pr, self.fip_ns._rule_priorities.pool) - @mock.patch.object(ip_lib, 'IPWrapper') - @mock.patch.object(ip_lib, 'IPDevice') - @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif') - @mock.patch.object(ip_lib, 'device_exists') - def test_gateway_added(self, device_exists, send_adv_notif, - IPDevice, IPWrapper): - subnet_id = _uuid() + def _get_agent_gw_port(self): + v4_subnet_id = _uuid() + v6_subnet_id = _uuid() agent_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30', 'prefixlen': 24, - 'subnet_id': subnet_id}], - 'subnets': [{'id': subnet_id, + 'subnet_id': v4_subnet_id}, + {'ip_address': 'cafe:dead:beef::3', + 'prefixlen': 64, + 'subnet_id': v6_subnet_id}], + 'subnets': [{'id': v4_subnet_id, 'cidr': '20.0.0.0/24', - 'gateway_ip': '20.0.0.1'}], + 'gateway_ip': '20.0.0.1'}, + {'id': v6_subnet_id, + 'cidr': 'cafe:dead:beef::/64', + 'gateway_ip': 'cafe:dead:beef::1'}], 'id': _uuid(), 'network_id': self.net_id, 'mac_address': 'ca:fe:de:ad:be:ef'} - - device_exists.return_value = False - self.fip_ns._gateway_added(agent_gw_port, - mock.sentinel.interface_name) - self.assertEqual(self.driver.plug.call_count, 1) - self.assertEqual(self.driver.init_l3.call_count, 1) - send_adv_notif.assert_called_once_with(self.fip_ns.get_name(), - mock.sentinel.interface_name, - '20.0.0.30', - mock.ANY) + return agent_gw_port @mock.patch.object(ip_lib, 'IPWrapper') - @mock.patch.object(ip_lib, 'IPDevice') - @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif') @mock.patch.object(ip_lib, 'device_exists') - def test_gateway_outside_subnet_added(self, device_exists, send_adv_notif, - IPDevice, IPWrapper): - device = mock.Mock() - IPDevice.return_value = device - - subnet_id = _uuid() - agent_gw_port = {'fixed_ips': [{'ip_address': '20.0.0.30', - 'prefixlen': 24, - 'subnet_id': subnet_id}], - 'subnets': [{'id': subnet_id, - 'cidr': '20.0.0.0/24', - 'gateway_ip': '20.0.1.1'}], - 'id': _uuid(), - 'network_id': self.net_id, - 'mac_address': 'ca:fe:de:ad:be:ef'} + def test_gateway_added(self, device_exists, ip_wrapper): + agent_gw_port = self._get_agent_gw_port() device_exists.return_value = False + self.fip_ns.update_gateway_port = mock.Mock() self.fip_ns._gateway_added(agent_gw_port, mock.sentinel.interface_name) self.assertEqual(1, self.driver.plug.call_count) self.assertEqual(1, self.driver.init_l3.call_count) - send_adv_notif.assert_called_once_with(self.fip_ns.get_name(), - mock.sentinel.interface_name, - '20.0.0.30', - mock.ANY) - device.route.add_route.assert_called_once_with('20.0.1.1', - scope='link') - device.route.add_gateway.assert_called_once_with('20.0.1.1') + self.fip_ns.update_gateway_port.assert_called_once_with(agent_gw_port) + + @mock.patch.object(ip_lib, 'IPDevice') + @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif') + def test_update_gateway_port(self, send_adv_notif, IPDevice): + self.fip_ns._check_for_gateway_ip_change = mock.Mock(return_value=True) + self.fip_ns.agent_gateway_port = None + agent_gw_port = self._get_agent_gw_port() + self.fip_ns.update_gateway_port(agent_gw_port) + expected = [ + mock.call(self.fip_ns.get_name(), + self.fip_ns.get_ext_device_name(agent_gw_port['id']), + agent_gw_port['fixed_ips'][0]['ip_address'], + mock.ANY), + mock.call(self.fip_ns.get_name(), + self.fip_ns.get_ext_device_name(agent_gw_port['id']), + agent_gw_port['fixed_ips'][1]['ip_address'], + mock.ANY)] + send_adv_notif.assert_has_calls(expected) + gw_ipv4 = agent_gw_port['subnets'][0]['gateway_ip'] + gw_ipv6 = agent_gw_port['subnets'][1]['gateway_ip'] + expected = [mock.call(gw_ipv4), mock.call(gw_ipv6)] + IPDevice().route.add_gateway.assert_has_calls(expected) + + @mock.patch.object(ip_lib, 'IPDevice') + @mock.patch.object(ip_lib, 'send_ip_addr_adv_notif') + def test_update_gateway_port_gateway_outside_subnet_added( + self, send_adv_notif, IPDevice): + self.fip_ns.agent_gateway_port = None + agent_gw_port = self._get_agent_gw_port() + agent_gw_port['subnets'][0]['gateway_ip'] = '20.0.1.1' + + self.fip_ns.update_gateway_port(agent_gw_port) + + IPDevice().route.add_route.assert_called_once_with('20.0.1.1', + scope='link') + + def test_check_gateway_ip_changed_no_change(self): + agent_gw_port = self._get_agent_gw_port() + self.fip_ns.agent_gateway_port = copy.deepcopy(agent_gw_port) + agent_gw_port['mac_address'] = 'aa:bb:cc:dd:ee:ff' + self.assertFalse(self.fip_ns._check_for_gateway_ip_change( + agent_gw_port)) + + def test_check_gateway_ip_changed_v4(self): + agent_gw_port = self._get_agent_gw_port() + self.fip_ns.agent_gateway_port = copy.deepcopy(agent_gw_port) + agent_gw_port['subnets'][0]['gateway_ip'] = '20.0.0.2' + self.assertTrue(self.fip_ns._check_for_gateway_ip_change( + agent_gw_port)) + + def test_check_gateway_ip_changed_v6(self): + agent_gw_port = self._get_agent_gw_port() + self.fip_ns.agent_gateway_port = copy.deepcopy(agent_gw_port) + agent_gw_port['subnets'][1]['gateway_ip'] = 'cafe:dead:beef::2' + self.assertTrue(self.fip_ns._check_for_gateway_ip_change( + agent_gw_port)) @mock.patch.object(iptables_manager, 'IptablesManager') @mock.patch.object(utils, 'execute') diff --git a/neutron/tests/unit/agent/l3/test_dvr_local_router.py b/neutron/tests/unit/agent/l3/test_dvr_local_router.py index dd972baae38..bfbc012ea38 100644 --- a/neutron/tests/unit/agent/l3/test_dvr_local_router.py +++ b/neutron/tests/unit/agent/l3/test_dvr_local_router.py @@ -157,6 +157,19 @@ class TestDvrRouterOperations(base.BaseTestCase): mock.Mock(), **kwargs) + def test_create_dvr_fip_interfaces_update(self): + ri = self._create_router() + fip_agent_port = {'subnets': []} + ri.get_floating_agent_gw_interface = mock.Mock( + return_value=fip_agent_port) + ri.get_floating_ips = mock.Mock(return_value=True) + ri.fip_ns = mock.Mock() + ri.fip_ns.subscribe.return_value = False + ex_gw_port = {'network_id': 'fake_net_id'} + ri.create_dvr_fip_interfaces(ex_gw_port) + ri.fip_ns.update_gateway_port.assert_called_once_with( + fip_agent_port) + def test_get_floating_ips_dvr(self): router = mock.MagicMock() router.get.return_value = [{'host': HOSTNAME},