From 03a525f9b0d69fe9ae58aabf8fe42bb6d717308b Mon Sep 17 00:00:00 2001 From: Mitchell Jameson Date: Tue, 3 May 2022 16:59:40 -0700 Subject: [PATCH] Don't use router_interface fixed_ip for MLAG peer SVI IPs When selecting IPs for MLAG peers SVIs, we typically choose the top two IPs in the subnet (eg. .254 and .253 in a /24). However, this may collide with the fixed_ip for the router, which will cause EOS to reject the router create on one of the peers as the VARP IP cannot be the same as the SVI IP. The fix prosed here is to use the third highest IP in the subnet if the fixed_ip is one of the two highest IPs in the subnet. Change-Id: I6a320a31b55edc947d288c9ad030bc476ae6ee9f --- .../l3Plugin/arista_l3_driver.py | 37 ++-- .../unit/l3Plugin/test_arista_l3_driver.py | 205 +++++++++++++++++- 2 files changed, 219 insertions(+), 23 deletions(-) diff --git a/networking_arista/l3Plugin/arista_l3_driver.py b/networking_arista/l3Plugin/arista_l3_driver.py index 51ae3ed2..123fb415 100644 --- a/networking_arista/l3Plugin/arista_l3_driver.py +++ b/networking_arista/l3Plugin/arista_l3_driver.py @@ -568,11 +568,15 @@ class AristaL3Driver(object): if self._mlag_configured: # For MLAG, we send a specific IP address as opposed to cidr # For now, we are using x.x.x.253 and x.x.x.254 as virtual IP + # unless either collides with the router interface fixed_ip, + # in which case we use x.x.x.252 mlag_peer_failed = False + router_ips = self._get_router_ips(cidr, len(self._servers), + router_info['ip_version'], + router_info['fixed_ip']) for i, server in enumerate(self._servers): # Get appropriate virtual IP address for this router - router_ip = self._get_router_ip(cidr, i, - router_info['ip_version']) + router_ip = router_ips[i] try: self.add_interface_to_router(router_info['seg_id'], router_name, @@ -708,18 +712,17 @@ class AristaL3Driver(object): lo = bin_addr & 0xFFFFFFFF return socket.inet_ntop(socket.AF_INET6, struct.pack("!QQ", hi, lo)) - def _get_router_ip(self, cidr, ip_count, ip_ver): + def _get_router_ips(self, cidr, ip_count, ip_ver, fixed_ip): """For a given IP subnet and IP version type, generate IP for router. - This method takes the network address (cidr) and selects an - IP address that should be assigned to virtual router running + This method takes the network address (cidr) and selects a set of + IP addresses that should be assigned to virtual router running on multiple switches. It uses upper addresses in a subnet address as IP for the router. Each instace of the router, on each switch, requires uniqe IP address. For example in IPv4 case, on a 255 subnet, it will pick X.X.X.254 as first addess, X.X.X.253 for next, - and so on. + and so on, skipping an address specified as the reserved_ip """ - start_ip = MLAG_SWITCHES + ip_count network_addr, prefix = cidr.split('/') if ip_ver == 4: bits = IPV4_BITS @@ -732,10 +735,18 @@ class AristaL3Driver(object): network_addr = ip & mask - router_ip = pow(2, bits - int(prefix)) - start_ip + router_ips = list() + ip_idx = 0 + while len(router_ips) < ip_count: + start_ip = MLAG_SWITCHES + ip_idx + router_ip = pow(2, bits - int(prefix)) - start_ip - router_ip = network_addr | router_ip - if ip_ver == 4: - return self._get_ipv4_from_binary(router_ip) + '/' + prefix - else: - return self._get_ipv6_from_binary(router_ip) + '/' + prefix + router_ip = network_addr | router_ip + if ip_ver == 4: + router_ip = self._get_ipv4_from_binary(router_ip) + else: + router_ip = self._get_ipv6_from_binary(router_ip) + ip_idx += 1 + if router_ip != fixed_ip.lower(): + router_ips.append(router_ip + '/' + prefix) + return router_ips diff --git a/networking_arista/tests/unit/l3Plugin/test_arista_l3_driver.py b/networking_arista/tests/unit/l3Plugin/test_arista_l3_driver.py index 246c2b5d..50167880 100644 --- a/networking_arista/tests/unit/l3Plugin/test_arista_l3_driver.py +++ b/networking_arista/tests/unit/l3Plugin/test_arista_l3_driver.py @@ -628,38 +628,54 @@ class AristaL3DriverTestCases_v4(base.BaseTestCase): def test_add_v4_interface_to_router(self): gateway_ip = '10.10.10.1' cidrs = ['10.10.10.0/24', '10.11.11.0/24'] - fixed_id = '10.10.10.15' + fixed_ip = '10.10.10.15' + segment_id = 123 # Add couple of IPv4 subnets to router for cidr in cidrs: router = {'id': 'r1', 'name': 'test-router-1', 'tenant_id': 'ten-a', - 'seg_id': '123', + 'seg_id': '%s' % segment_id, 'cidr': "%s" % cidr, 'gip': "%s" % gateway_ip, 'ip_version': 4, - 'fixed_ip': fixed_id} + 'fixed_ip': fixed_ip} self.assertFalse(self.drv.add_router_interface(None, router)) + for s in self.drv._servers: + cmds = ['enable', 'configure', 'ip routing', + 'vlan %s' % segment_id, 'exit', + 'interface vlan %s' % segment_id, + 'ip address %s/%s' % (fixed_ip, cidr.split('/')[1]), + 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() def test_delete_v4_interface_from_router(self): gateway_ip = '10.10.10.1' cidrs = ['10.10.10.0/24', '10.11.11.0/24'] fixed_ip = '10.10.10.15' + segment_id = 123 # remove couple of IPv4 subnets from router for cidr in cidrs: router = {'id': 'r1', 'name': 'test-router-1', 'tenant_id': 'ten-a', - 'seg_id': '123', + 'seg_id': '%s' % segment_id, 'cidr': "%s" % cidr, 'gip': "%s" % gateway_ip, 'ip_version': 4, 'fixed_id': fixed_ip} self.assertFalse(self.drv.remove_router_interface(None, router)) + for s in self.drv._servers: + cmds = ['enable', 'configure', + 'no interface vlan %s' % segment_id, + 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() class AristaL3DriverTestCases_v6(base.BaseTestCase): @@ -683,37 +699,156 @@ class AristaL3DriverTestCases_v6(base.BaseTestCase): gateway_ip = '3FFE::1' cidrs = ['3FFE::/16', '2001::/16'] fixed_ip = '3FFE::5' + segment_id = 123 # Add couple of IPv6 subnets to router for cidr in cidrs: router = {'id': 'r1', 'name': 'test-router-1', 'tenant_id': 'ten-a', - 'seg_id': '123', + 'seg_id': '%s' % segment_id, 'cidr': "%s" % cidr, 'gip': "%s" % gateway_ip, 'ip_version': 6, 'fixed_ip': fixed_ip} self.assertFalse(self.drv.add_router_interface(None, router)) + for s in self.drv._servers: + cmds = ['enable', 'configure', 'ipv6 unicast-routing', + 'vlan %s' % segment_id, 'exit', + 'interface vlan %s' % segment_id, 'ipv6 enable', + 'ipv6 address %s/%s' % (fixed_ip, cidr.split('/')[1]), + 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() def test_delete_v6_interface_from_router(self): gateway_ip = '3FFE::1' cidrs = ['3FFE::/16', '2001::/16'] fixed_ip = '3FFE::5' + segment_id = 123 # remove couple of IPv6 subnets from router for cidr in cidrs: router = {'id': 'r1', 'name': 'test-router-1', 'tenant_id': 'ten-a', - 'seg_id': '123', + 'seg_id': '%s' % segment_id, 'cidr': "%s" % cidr, 'gip': "%s" % gateway_ip, 'ip_version': 6, 'fixed_ip': fixed_ip} self.assertFalse(self.drv.remove_router_interface(None, router)) + for s in self.drv._servers: + cmds = ['enable', 'configure', + 'no interface vlan %s' % segment_id, + 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() + + +class AristaL3DriverTestCases_MLAG_v4(base.BaseTestCase): + """Test cases to test the RPC between Arista Driver and EOS. + + Tests all methods used to send commands between Arista L3 Driver and EOS + to program routing functions in Default VRF on MLAG'ed switches using IPv4. + """ + + def setUp(self): + super(AristaL3DriverTestCases_MLAG_v4, self).setUp() + setup_arista_config('value', mlag=True) + self.drv = arista.AristaL3Driver() + self.drv._servers = [] + self.drv._servers.append(mock.MagicMock()) + self.drv._servers.append(mock.MagicMock()) + + def test_no_exception_on_correct_configuration(self): + self.assertIsNotNone(self.drv) + + def test_add_v4_interface_to_router(self): + gateway_ip = '10.10.10.1' + cidrs = ['10.10.10.0/24', '10.11.11.0/24'] + fixed_ip = '10.10.10.15' + segment_id = 123 + svi_ips = [['10.10.10.254', '10.10.10.253'], + ['10.11.11.254', '10.11.11.253']] + + # Add couple of IPv6 subnets to router + for i, cidr in enumerate(cidrs): + router = {'id': 'r1', + 'name': 'test-router-1', + 'tenant_id': 'ten-a', + 'seg_id': '%s' % segment_id, + 'cidr': "%s" % cidr, + 'gip': "%s" % gateway_ip, + 'ip_version': 4, + 'fixed_ip': fixed_ip} + + self.assertFalse(self.drv.add_router_interface(None, router)) + for j, s in enumerate(self.drv._servers): + svi_ip = svi_ips[i][j] + cmds = ['enable', 'configure', 'ip routing', + 'vlan %s' % segment_id, 'exit', + 'interface vlan %s' % segment_id, + 'ip address %s/%s' % (svi_ip, cidr.split('/')[1]), + 'ip virtual-router address %s' % fixed_ip, 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() + + def test_add_v4_interface_to_router_high_fixed_ip(self): + gateway_ip = '10.10.10.1' + cidrs = ['10.10.10.0/24', '10.11.11.0/24'] + fixed_ips = ['10.10.10.254', '10.11.11.253'] + segment_id = 123 + svi_ips = [['10.10.10.253', '10.10.10.252'], + ['10.11.11.254', '10.11.11.252']] + + # Add couple of IPv6 subnets to router + for i, cidr in enumerate(cidrs): + fixed_ip = fixed_ips[i] + router = {'id': 'r1', + 'name': 'test-router-1', + 'tenant_id': 'ten-a', + 'seg_id': '%s' % segment_id, + 'cidr': "%s" % cidr, + 'gip': "%s" % gateway_ip, + 'ip_version': 4, + 'fixed_ip': fixed_ip} + + self.assertFalse(self.drv.add_router_interface(None, router)) + for j, s in enumerate(self.drv._servers): + svi_ip = svi_ips[i][j] + cmds = ['enable', 'configure', 'ip routing', + 'vlan %s' % segment_id, 'exit', + 'interface vlan %s' % segment_id, + 'ip address %s/%s' % (svi_ip, cidr.split('/')[1]), + 'ip virtual-router address %s' % fixed_ip, 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() + + def test_delete_v4_interface_from_router(self): + gateway_ip = '10.10.10.1' + cidrs = ['10.10.10.0/24', '10.11.11.0/24'] + segment_id = 123 + + # remove couple of IPv6 subnets from router + for cidr in cidrs: + router = {'id': 'r1', + 'name': 'test-router-1', + 'tenant_id': 'ten-a', + 'seg_id': '%s' % segment_id, + 'cidr': "%s" % cidr, + 'gip': "%s" % gateway_ip, + 'ip_version': 4} + + self.assertFalse(self.drv.remove_router_interface(None, router)) + for s in self.drv._servers: + cmds = ['enable', 'configure', + 'no interface vlan %s' % segment_id, + 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() class AristaL3DriverTestCases_MLAG_v6(base.BaseTestCase): @@ -736,37 +871,87 @@ class AristaL3DriverTestCases_MLAG_v6(base.BaseTestCase): def test_add_v6_interface_to_router(self): gateway_ip = '3FFE::1' - cidrs = ['3FFE::/16', '2001::/16'] + cidrs = ['3FFE::/112', '2001::/112'] fixed_ip = '3FFE::5' + segment_id = 123 + svi_ips = [['3ffe::fffe', '3ffe::fffd'], + ['2001::fffe', '2001::fffd']] # Add couple of IPv6 subnets to router - for cidr in cidrs: + for i, cidr in enumerate(cidrs): router = {'id': 'r1', 'name': 'test-router-1', 'tenant_id': 'ten-a', - 'seg_id': '123', + 'seg_id': '%s' % segment_id, 'cidr': "%s" % cidr, 'gip': "%s" % gateway_ip, 'ip_version': 6, 'fixed_ip': fixed_ip} self.assertFalse(self.drv.add_router_interface(None, router)) + for j, s in enumerate(self.drv._servers): + svi_ip = svi_ips[i][j] + cmds = ['enable', 'configure', 'ipv6 unicast-routing', + 'vlan %s' % segment_id, 'exit', + 'interface vlan %s' % segment_id, 'ipv6 enable', + 'ipv6 address %s/%s' % (svi_ip, cidr.split('/')[1]), + 'ipv6 virtual-router address %s' % fixed_ip, 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() + + def test_add_v6_interface_to_router_high_fixed_ip(self): + gateway_ip = '3FFE::1' + cidrs = ['3FFE::/112', '2001::/112'] + fixed_ips = ['3FFE::FFFE', '2001::FFFD'] + segment_id = 123 + svi_ips = [['3ffe::fffd', '3ffe::fffc'], + ['2001::fffe', '2001::fffc']] + + # Add couple of IPv6 subnets to router + for i, cidr in enumerate(cidrs): + fixed_ip = fixed_ips[i] + router = {'id': 'r1', + 'name': 'test-router-1', + 'tenant_id': 'ten-a', + 'seg_id': '%s' % segment_id, + 'cidr': "%s" % cidr, + 'gip': "%s" % gateway_ip, + 'ip_version': 6, + 'fixed_ip': fixed_ip} + + self.assertFalse(self.drv.add_router_interface(None, router)) + for j, s in enumerate(self.drv._servers): + svi_ip = svi_ips[i][j] + cmds = ['enable', 'configure', 'ipv6 unicast-routing', + 'vlan %s' % segment_id, 'exit', + 'interface vlan %s' % segment_id, 'ipv6 enable', + 'ipv6 address %s/%s' % (svi_ip, cidr.split('/')[1]), + 'ipv6 virtual-router address %s' % fixed_ip, 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() def test_delete_v6_interface_from_router(self): gateway_ip = '3FFE::1' cidrs = ['3FFE::/16', '2001::/16'] + segment_id = 123 # remove couple of IPv6 subnets from router for cidr in cidrs: router = {'id': 'r1', 'name': 'test-router-1', 'tenant_id': 'ten-a', - 'seg_id': '123', + 'seg_id': '%s' % segment_id, 'cidr': "%s" % cidr, 'gip': "%s" % gateway_ip, 'ip_version': 6} self.assertFalse(self.drv.remove_router_interface(None, router)) + for s in self.drv._servers: + cmds = ['enable', 'configure', + 'no interface vlan %s' % segment_id, + 'exit'] + s.execute.assert_called_once_with(cmds, keep_alive=True) + s.reset_mock() class AristaL3DriverTestCasesMlag_one_switch_failed(base.BaseTestCase):