openstack
/
octavia
Code Issues Proposed changes

Merge "Fix incorrect removal of IP rules in the amphora"

This commit is contained in:
Zuul 2023-08-16 22:46:06 +00:00 committed by Gerrit Code Review
parent 8ccc0d0176 b3bf177992
commit 36ed6ec35f
9 changed files with 117 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

1
octavia/amphorae/backends/agent/api_server/osutils.py
View File

@ -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

16
octavia/amphorae/backends/utils/interface.py
View File

@ -197,7 +197,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)
@ -378,11 +380,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])

9
octavia/amphorae/backends/utils/interface_file.py
View File

@ -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,
@ -105,7 +108,7 @@ class InterfaceFile(object):
class VIPInterfaceFile(InterfaceFile):
def __init__(self, name, mtu, vips, vrrp_info, fixed_ips, topology):
super().__init__(name, mtu=mtu)
super().__init__(name, if_type=consts.VIP, mtu=mtu)
has_ipv4 = any(vip['ip_version'] == 4 for vip in vips)
has_ipv6 = any(vip['ip_version'] == 6 for vip in vips)
@ -236,7 +239,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()

3
octavia/common/constants.py
View File

@ -911,6 +911,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'

2
octavia/tests/unit/amphorae/backends/agent/api_server/test_osutils.py
View File

@ -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()

81
octavia/tests/unit/amphorae/backends/utils/test_interface.py
View File

@ -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,

2
octavia/tests/unit/amphorae/backends/utils/test_interface_file.py
View File

@ -691,6 +691,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'
@ -717,6 +718,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",

4
octavia/tests/unit/cmd/test_interface.py
View File

@ -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()

11
releasenotes/notes/fix-ip-rules-in-amphora-b74b7b616752c13b.yaml Normal file
View File

@ -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.