openstack/neutron
Code Issues Proposed changes

Merge "Make ip rule comparison more robust"

Browse Source
This commit is contained in:
Jenkins 2015-09-09 20:48:19 +00:00 committed by Gerrit Code Review
commit d91f6ee911
2 changed files with 118 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

91
neutron/agent/linux/ip_lib.py
View File

@ -20,6 +20,7 @@ from oslo_config import cfg
from oslo_log import log as logging
from oslo_utils import excutils
import re
import six
from neutron.agent.common import utils
from neutron.common import constants
@ -287,6 +288,67 @@ class IPRule(SubProcessBase):
class IpRuleCommand(IpCommandBase):
COMMAND = 'rule'
@staticmethod
def _make_canonical(ip_version, settings):
"""Converts settings to a canonical represention to compare easily"""
def canonicalize_fwmark_string(fwmark_mask):
"""Reformats fwmark/mask in to a canonical form
Examples, these are all equivalent:
"0x1"
0x1
"0x1/0xfffffffff"
(0x1, 0xfffffffff)
:param fwmark_mask: The firewall and mask (default 0xffffffff)
:type fwmark_mask: A string with / as delimiter, an iterable, or a
single value.
"""
# Turn the value we were passed in to an iterable: fwmark[, mask]
if isinstance(fwmark_mask, six.string_types):
# A / separates the optional mask in a string
iterable = fwmark_mask.split('/')
else:
try:
iterable = iter(fwmark_mask)
except TypeError:
# At this point, it must be a single integer
iterable = [fwmark_mask]
def to_i(s):
if isinstance(s, six.string_types):
# Passing 0 as "base" arg to "int" causes it to determine
# the base automatically.
return int(s, 0)
# s isn't a string, can't specify base argument
return int(s)
integers = [to_i(x) for x in iterable]
# The default mask is all ones, the mask is 32 bits.
if len(integers) == 1:
integers.append(0xffffffff)
# We now have two integers in a list. Convert to canonical string.
return '/'.join(map(hex, integers))
def canonicalize(item):
k, v = item
# ip rule shows these as 'any'
if k == 'from' and v == 'all':
return k, constants.IP_ANY[ip_version]
# lookup and table are interchangeable. Use table every time.
if k == 'lookup':
return 'table', v
if k == 'fwmark':
return k, canonicalize_fwmark_string(v)
return k, v
if 'type' not in settings:
settings['type'] = 'unicast'
return {k: str(v) for k, v in map(canonicalize, settings.items())}
def _parse_line(self, ip_version, line):
# Typical rules from 'ip rule show':
# 4030201: from 1.2.3.4/24 lookup 10203040
@ -296,23 +358,21 @@ class IpRuleCommand(IpCommandBase):
if not parts:
return {}
# Format of line is: "priority: <key> <value> ..."
# Format of line is: "priority: <key> <value> ... [<type>]"
settings = {k: v for k, v in zip(parts[1::2], parts[2::2])}
settings['priority'] = parts[0][:-1]
if len(parts) % 2 == 0:
# When line has an even number of columns, last one is the type.
settings['type'] = parts[-1]
# Canonicalize some arguments
if settings.get('from') == "all":
settings['from'] = constants.IP_ANY[ip_version]
if 'lookup' in settings:
settings['table'] = settings.pop('lookup')
return self._make_canonical(ip_version, settings)
return settings
def list_rules(self, ip_version):
lines = self._as_root([ip_version], ['show']).splitlines()
return [self._parse_line(ip_version, line) for line in lines]
def _exists(self, ip_version, **kwargs):
kwargs_strings = {k: str(v) for k, v in kwargs.items()}
lines = self._as_root([ip_version], ['show']).splitlines()
return kwargs_strings in (self._parse_line(ip_version, line)
for line in lines)
return kwargs in self.list_rules(ip_version)
def _make__flat_args_tuple(self, *args, **kwargs):
for kwargs_item in sorted(kwargs.items(), key=lambda i: i[0]):
@ -323,9 +383,10 @@ class IpRuleCommand(IpCommandBase):
ip_version = get_ip_version(ip)
kwargs.update({'from': ip})
canonical_kwargs = self._make_canonical(ip_version, kwargs)
if not self._exists(ip_version, **kwargs):
args_tuple = self._make__flat_args_tuple('add', **kwargs)
if not self._exists(ip_version, **canonical_kwargs):
args_tuple = self._make__flat_args_tuple('add', **canonical_kwargs)
self._as_root([ip_version], args_tuple)
def delete(self, ip, **kwargs):
@ -333,7 +394,9 @@ class IpRuleCommand(IpCommandBase):
# TODO(Carl) ip ignored in delete, okay in general?
args_tuple = self._make__flat_args_tuple('del', **kwargs)
canonical_kwargs = self._make_canonical(ip_version, kwargs)
args_tuple = self._make__flat_args_tuple('del', **canonical_kwargs)
self._as_root([ip_version], args_tuple)

44
neutron/tests/unit/agent/linux/test_ip_lib.py
View File

@ -562,7 +562,9 @@ class TestIpRuleCommand(TestIPCmdBase):
self.rule_cmd.add(ip, table=table, priority=priority)
self._assert_sudo([ip_version], (['show']))
self._assert_sudo([ip_version], ('add', 'from', ip,
'priority', priority, 'table', table))
'priority', str(priority),
'table', str(table),
'type', 'unicast'))
def _test_add_rule_exists(self, ip, table, priority, output):
self.parent._as_root.return_value = output
@ -574,8 +576,8 @@ class TestIpRuleCommand(TestIPCmdBase):
ip_version = netaddr.IPNetwork(ip).version
self.rule_cmd.delete(ip, table=table, priority=priority)
self._assert_sudo([ip_version],
('del', 'priority', priority,
'table', table))
('del', 'priority', str(priority),
'table', str(table), 'type', 'unicast'))
def test__parse_line(self):
def test(ip_version, line, expected):
@ -585,13 +587,49 @@ class TestIpRuleCommand(TestIPCmdBase):
test(4, "4030201:\tfrom 1.2.3.4/24 lookup 10203040",
{'from': '1.2.3.4/24',
'table': '10203040',
'type': 'unicast',
'priority': '4030201'})
test(6, "1024: from all iif qg-c43b1928-48 lookup noscope",
{'priority': '1024',
'from': '::/0',
'type': 'unicast',
'iif': 'qg-c43b1928-48',
'table': 'noscope'})
def test__make_canonical_all_v4(self):
actual = self.rule_cmd._make_canonical(4, {'from': 'all'})
self.assertEqual({'from': '0.0.0.0/0', 'type': 'unicast'}, actual)
def test__make_canonical_all_v6(self):
actual = self.rule_cmd._make_canonical(6, {'from': 'all'})
self.assertEqual({'from': '::/0', 'type': 'unicast'}, actual)
def test__make_canonical_lookup(self):
actual = self.rule_cmd._make_canonical(6, {'lookup': 'table'})
self.assertEqual({'table': 'table', 'type': 'unicast'}, actual)
def test__make_canonical_iif(self):
actual = self.rule_cmd._make_canonical(6, {'iif': 'iface_name'})
self.assertEqual({'iif': 'iface_name', 'type': 'unicast'}, actual)
def test__make_canonical_fwmark(self):
actual = self.rule_cmd._make_canonical(6, {'fwmark': '0x400'})
self.assertEqual({'fwmark': '0x400/0xffffffff',
'type': 'unicast'}, actual)
def test__make_canonical_fwmark_with_mask(self):
actual = self.rule_cmd._make_canonical(6, {'fwmark': '0x400/0x00ff'})
self.assertEqual({'fwmark': '0x400/0xff', 'type': 'unicast'}, actual)
def test__make_canonical_fwmark_integer(self):
actual = self.rule_cmd._make_canonical(6, {'fwmark': 0x400})
self.assertEqual({'fwmark': '0x400/0xffffffff',
'type': 'unicast'}, actual)
def test__make_canonical_fwmark_iterable(self):
actual = self.rule_cmd._make_canonical(6, {'fwmark': (0x400, 0xffff)})
self.assertEqual({'fwmark': '0x400/0xffff', 'type': 'unicast'}, actual)
def test_add_rule_v4(self):
self._test_add_rule('192.168.45.100', 2, 100)