diff --git a/octavia/amphorae/backends/agent/api_server/osutils.py b/octavia/amphorae/backends/agent/api_server/osutils.py index ac779db5cc..98648e9745 100644 --- a/octavia/amphorae/backends/agent/api_server/osutils.py +++ b/octavia/amphorae/backends/agent/api_server/osutils.py @@ -56,6 +56,7 @@ class BaseOS(object): def write_interface_file(self, interface, ip_address, prefixlen): interface = interface_file.InterfaceFile( name=interface, + if_type=consts.LO, addresses=[{ "address": ip_address, "prefixlen": prefixlen diff --git a/octavia/amphorae/backends/utils/interface.py b/octavia/amphorae/backends/utils/interface.py index 87828f1bad..eefabd5142 100644 --- a/octavia/amphorae/backends/utils/interface.py +++ b/octavia/amphorae/backends/utils/interface.py @@ -195,7 +195,9 @@ class InterfaceController(object): self._addresses_up(interface, ipr, idx) self._routes_up(interface, ipr, idx) - self._rules_up(interface, ipr, idx) + # only the vip port updates the rules + if interface.if_type == consts.VIP: + self._rules_up(interface, ipr, idx) self._scripts_up(interface, current_state) @@ -374,11 +376,13 @@ class InterfaceController(object): current_state = link.get(consts.STATE) if current_state == consts.IFACE_UP: - for rule in interface.rules: - rule[consts.FAMILY] = self._family(rule[consts.SRC]) - LOG.debug("%s: Deleting rule %s", interface.name, rule) - self._ipr_command(ipr.rule, self.DELETE, - raise_on_error=False, **rule) + # only the vip port updates the rules + if interface.if_type == consts.VIP: + for rule in interface.rules: + rule[consts.FAMILY] = self._family(rule[consts.SRC]) + LOG.debug("%s: Deleting rule %s", interface.name, rule) + self._ipr_command(ipr.rule, self.DELETE, + raise_on_error=False, **rule) for route in interface.routes: route[consts.FAMILY] = self._family(route[consts.DST]) diff --git a/octavia/amphorae/backends/utils/interface_file.py b/octavia/amphorae/backends/utils/interface_file.py index 0db9383112..3811de27f0 100644 --- a/octavia/amphorae/backends/utils/interface_file.py +++ b/octavia/amphorae/backends/utils/interface_file.py @@ -25,9 +25,11 @@ CONF = cfg.CONF class InterfaceFile(object): - def __init__(self, name, mtu=None, addresses=None, + def __init__(self, name, if_type, + mtu=None, addresses=None, routes=None, rules=None, scripts=None): self.name = name + self.if_type = if_type self.mtu = mtu self.addresses = addresses or [] self.routes = routes or [] @@ -92,6 +94,7 @@ class InterfaceFile(object): flags, mode), 'w') as fp: interface = { consts.NAME: self.name, + consts.IF_TYPE: self.if_type, consts.ADDRESSES: self.addresses, consts.ROUTES: self.routes, consts.RULES: self.rules, @@ -108,7 +111,7 @@ class VIPInterfaceFile(InterfaceFile): gateway, vrrp_ip, host_routes, topology, fixed_ips=None): - super().__init__(name, mtu=mtu) + super().__init__(name, if_type=consts.VIP, mtu=mtu) if vrrp_ip: self.addresses.append({ @@ -224,7 +227,7 @@ class VIPInterfaceFile(InterfaceFile): class PortInterfaceFile(InterfaceFile): def __init__(self, name, mtu, fixed_ips): - super().__init__(name, mtu=mtu) + super().__init__(name, if_type=consts.BACKEND, mtu=mtu) if fixed_ips: ip_versions = set() diff --git a/octavia/common/constants.py b/octavia/common/constants.py index 6a5283f2c9..574e565848 100644 --- a/octavia/common/constants.py +++ b/octavia/common/constants.py @@ -909,6 +909,9 @@ AMPHORA_SUPPORTED_ALPN_PROTOCOLS = [lib_consts.ALPN_PROTOCOL_HTTP_2, lib_consts.ALPN_PROTOCOL_HTTP_1_0] # Amphora interface fields +IF_TYPE = 'if_type' +BACKEND = 'backend' +LO = 'lo' MTU = 'mtu' ADDRESSES = 'addresses' ROUTES = 'routes' diff --git a/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py b/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py index 31205e34dd..1cf46da337 100644 --- a/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py +++ b/octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py @@ -106,7 +106,7 @@ class TestOSUtils(base.TestCase): '192.0.2.2', 16) mock_interface_file.assert_called_once_with( - name='eth1', + name='eth1', if_type="lo", addresses=[{"address": "192.0.2.2", "prefixlen": 16}]) mock_interface.write.assert_called_once() diff --git a/octavia/tests/unit/amphorae/backends/utils/test_interface.py b/octavia/tests/unit/amphorae/backends/utils/test_interface.py index b990976000..5ac344284a 100644 --- a/octavia/tests/unit/amphorae/backends/utils/test_interface.py +++ b/octavia/tests/unit/amphorae/backends/utils/test_interface.py @@ -74,6 +74,7 @@ class TestInterface(base.TestCase): '],\n' '"mtu": 1450,\n' '"name": "eth1",\n' + '"if_type": "mytype",\n' '"routes": [\n' '{"dst": "0.0.0.0/0",\n' '"gateway": "10.0.0.1"},\n' @@ -107,6 +108,7 @@ class TestInterface(base.TestCase): expected_dict = { consts.NAME: "eth1", + consts.IF_TYPE: "mytype", consts.MTU: 1450, consts.ADDRESSES: [{ consts.ADDRESS: "10.0.0.2", @@ -331,6 +333,7 @@ class TestInterface(base.TestCase): mock_link, mock_addr, mock_route, mock_rule): iface = interface_file.InterfaceFile( name="eth1", + if_type="vip", mtu=1450, addresses=[{ consts.ADDRESS: '1.2.3.4', @@ -445,6 +448,78 @@ class TestInterface(base.TestCase): mock.call(["post-up", "eth1"]) ]) + @mock.patch('pyroute2.IPRoute.rule') + @mock.patch('pyroute2.IPRoute.route') + @mock.patch('pyroute2.IPRoute.addr') + @mock.patch('pyroute2.IPRoute.link') + @mock.patch('pyroute2.IPRoute.get_links') + @mock.patch('pyroute2.IPRoute.link_lookup') + @mock.patch('pyroute2.IPRoute.get_rules') + @mock.patch('subprocess.check_output') + def test_up_backend(self, mock_check_output, mock_get_rules, + mock_link_lookup, mock_get_links, mock_link, mock_addr, + mock_route, mock_rule): + iface = interface_file.InterfaceFile( + name="eth1", + if_type="backend", + mtu=1450, + addresses=[{ + consts.ADDRESS: '1.2.3.4', + consts.PREFIXLEN: 24 + }], + routes=[], + rules=[], + scripts={ + consts.IFACE_UP: [{ + consts.COMMAND: "post-up eth1" + }], + consts.IFACE_DOWN: [{ + consts.COMMAND: "post-down eth1" + }], + }) + + idx = mock.MagicMock() + mock_link_lookup.return_value = [idx] + + mock_get_links.return_value = [{ + consts.STATE: consts.IFACE_DOWN + }] + mock_get_rules.return_value = [{ + 'src_len': 32, + 'attrs': { + 'FRA_SRC': '1.1.1.1', + 'FRA_TABLE': 20, + 'FRA_PROTOCOL': 0 + } + }] + + controller = interface.InterfaceController() + controller.up(iface) + + mock_link.assert_called_once_with( + controller.SET, + index=idx, + state=consts.IFACE_UP, + mtu=1450) + + mock_addr.assert_has_calls([ + mock.call(controller.ADD, + index=idx, + address='1.2.3.4', + prefixlen=24, + family=socket.AF_INET), + ]) + + mock_route.assert_called_once_with( + 'dump', family=mock.ANY, match=mock.ANY) + + # for 'backend' iface, we don't update the rules + mock_rule.assert_not_called() + + mock_check_output.assert_has_calls([ + mock.call(["post-up", "eth1"]) + ]) + @mock.patch('pyroute2.IPRoute.rule') @mock.patch('pyroute2.IPRoute.route') @mock.patch('pyroute2.IPRoute.addr') @@ -463,6 +538,7 @@ class TestInterface(base.TestCase): mock_route, mock_rule): iface = interface_file.InterfaceFile( name="eth1", + if_type="vip", mtu=1450, addresses=[{ consts.ADDRESS: '1.2.3.4', @@ -658,6 +734,7 @@ class TestInterface(base.TestCase): mock_route, mock_rule): iface = interface_file.InterfaceFile( name="eth1", + if_type="vip", mtu=1450, addresses=[{ consts.DHCP: True, @@ -722,6 +799,7 @@ class TestInterface(base.TestCase): mock_link, mock_addr, mock_route, mock_rule): iface = interface_file.InterfaceFile( name="eth1", + if_type="vip", mtu=1450, addresses=[{ consts.ADDRESS: '1.2.3.4', @@ -848,6 +926,7 @@ class TestInterface(base.TestCase): mock_addr, mock_route, mock_rule): iface = interface_file.InterfaceFile( name="eth1", + if_type="vip", mtu=1450, addresses=[{ consts.ADDRESS: '1.2.3.4', @@ -997,6 +1076,7 @@ class TestInterface(base.TestCase): mock_route, mock_rule): iface = interface_file.InterfaceFile( name="eth1", + if_type="vip", mtu=1450, addresses=[{ consts.ADDRESS: '1.2.3.4', @@ -1069,6 +1149,7 @@ class TestInterface(base.TestCase): mock_addr, mock_route, mock_rule): iface = interface_file.InterfaceFile( name="eth1", + if_type="vip", mtu=1450, addresses=[{ consts.DHCP: True, diff --git a/octavia/tests/unit/amphorae/backends/utils/test_interface_file.py b/octavia/tests/unit/amphorae/backends/utils/test_interface_file.py index 467bd9dd8d..661e602c6e 100644 --- a/octavia/tests/unit/amphorae/backends/utils/test_interface_file.py +++ b/octavia/tests/unit/amphorae/backends/utils/test_interface_file.py @@ -662,6 +662,7 @@ class TestInterfaceFile(base.TestCase): '"prefixlen": 26}\n' '],\n' '"mtu": 1450,\n' + '"if_type": "mytype",\n' '"name": "eth1",\n' '"routes": [\n' '{"dst": "0.0.0.0/0",\n' @@ -688,6 +689,7 @@ class TestInterfaceFile(base.TestCase): expected_dict = { consts.NAME: "eth1", + consts.IF_TYPE: "mytype", consts.MTU: 1450, consts.ADDRESSES: [{ consts.ADDRESS: "10.0.0.181", diff --git a/octavia/tests/unit/cmd/test_interface.py b/octavia/tests/unit/cmd/test_interface.py index fde2498a06..da35c20dac 100644 --- a/octavia/tests/unit/cmd/test_interface.py +++ b/octavia/tests/unit/cmd/test_interface.py @@ -23,8 +23,8 @@ class TestInterfaceCMD(base.TestCase): def setUp(self): super().setUp() - self.interface1 = interface_file.InterfaceFile("eth1") - self.interface2 = interface_file.InterfaceFile("eth2") + self.interface1 = interface_file.InterfaceFile("eth1", if_type="type1") + self.interface2 = interface_file.InterfaceFile("eth2", if_type="type2") def test_interfaces_find(self): controller = mock.Mock() diff --git a/releasenotes/notes/fix-ip-rules-in-amphora-b74b7b616752c13b.yaml b/releasenotes/notes/fix-ip-rules-in-amphora-b74b7b616752c13b.yaml new file mode 100644 index 0000000000..02c0960d0e --- /dev/null +++ b/releasenotes/notes/fix-ip-rules-in-amphora-b74b7b616752c13b.yaml @@ -0,0 +1,11 @@ +--- +fixes: + - | + Fixed a bug that could have made the VIP port unreachable because of the + removal of some IP rules in the Amphora. It could have been triggered only + when sending a request from a subnet that is not the VIP subnet but that is + plugged as a member subnet. +upgrade: + - | + A patch that fixes an issue making the VIP port unreachable because of + missing IP rules requires an update of the Amphora image.