Assaf Muller has uploaded a new change for review. Change subject: Made ipwrapper.py:Route class more robust ......................................................................
Made ipwrapper.py:Route class more robust Change-Id: Id2d62ad31bd9d5c7bea28605a0b195c1886f0dab Signed-off-by: Assaf Muller <[email protected]> --- M lib/vdsm/ipwrapper.py M lib/vdsm/netinfo.py M tests/functional/networkTests.py M tests/ipwrapperTests.py M vdsm/sourceRoute.py 5 files changed, 46 insertions(+), 33 deletions(-) git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/88/21788/1 diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py index 5e5ec29..4a3472b 100644 --- a/lib/vdsm/ipwrapper.py +++ b/lib/vdsm/ipwrapper.py @@ -238,15 +238,19 @@ @equals class Route(object): - def __init__(self, network, ipaddr=None, device=None, table=None): - if not _isValid(network, IPNetwork): + def __init__(self, network, via=None, src=None, device=None, table=None): + if network != 'local' and not _isValid(network, IPNetwork): raise ValueError('network %s is not properly defined' % network) - if ipaddr and not _isValid(ipaddr, IPAddress): - raise ValueError('ipaddr %s is not properly defined' % ipaddr) + if via and not _isValid(via, IPAddress): + raise ValueError('via %s is not a proper IP address' % via) + + if src and not _isValid(src, IPAddress): + raise ValueError('src %s is not a proper IP address' % src) self.network = network - self.ipaddr = ipaddr + self.via = via + self.src = src self.device = device self.table = table @@ -257,15 +261,13 @@ textual representation. """ route = text.split() - """ - The network / first column is required, followed by key+value pairs. - Thus, the length of a route must be odd. - """ - if len(route) % 2 == 0: - raise ValueError('Route %s: The length of the textual ' - 'representation of a route must be odd.' % text) - network, params = route[0], route[1:] + network = route[0] + if network == 'local': + params = route[2:] + else: + params = route[1:] + data = dict(params[i:i + 2] for i in range(0, len(params), 2)) data['network'] = '0.0.0.0/0' if network == 'default' else network return data @@ -273,35 +275,46 @@ @classmethod def fromText(cls, text): """ - Creates a Route object from a textual representation. For the vdsm - use case we require the network IP address and interface to reach - the network to be provided in the text. + Creates a Route object from a textual representation. Examples: 'default via 192.168.99.254 dev eth0': '0.0.0.0/0 via 192.168.99.254 dev eth0 table foo': '200.100.50.0/16 via 11.11.11.11 dev eth2 table foo': + 'local 127.0.0.1 dev lo scope host src 127.0.0.1': """ - data = cls.parse(text) try: - ipaddr = data['via'] - except KeyError: - raise ValueError('Route %s: Routes require an IP address.' % text) + data = cls.parse(text) + except Exception: + raise ValueError('Route %s: Failed to parse route.' % text) + + via = data.get('via') + src = data.get('src') try: device = data['dev'] except KeyError: raise ValueError('Route %s: Routes require a device.' % text) table = data.get('table') - return cls(data['network'], ipaddr=ipaddr, device=device, table=table) + return cls(data['network'], via=via, src=src, device=device, + table=table) def __str__(self): - str = '%s via %s dev %s' % (self.network, self.ipaddr, self.device) + output = str(self.network) + if self.network == 'local': + output += ' %s' % self.src + if self.via: + output += ' via %s' % self.via + + output += ' dev %s' % self.device + + if self.src: + output += ' src %s' % self.src if self.table: - str += ' table %s' % self.table + output += ' table %s' % self.table - return str + return output def __iter__(self): for word in str(self).split(): diff --git a/lib/vdsm/netinfo.py b/lib/vdsm/netinfo.py index 00640e2..2d11276 100644 --- a/lib/vdsm/netinfo.py +++ b/lib/vdsm/netinfo.py @@ -367,7 +367,7 @@ """Return the default gateway for each interface that has one.""" default_routes = (Route.fromText(text) for text in routeShowAllDefaultGateways()) - return dict((route.device, route.ipaddr) for route in default_routes) + return dict((route.device, route.via) for route in default_routes) def ipv6StrToAddress(ipv6_str): diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 650d6b2..8599c36 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -1339,10 +1339,10 @@ try: dummy.setLinkUp(nic) - routes = [Route(network='0.0.0.0/0', ipaddr=IP_GATEWAY, + routes = [Route(network='0.0.0.0/0', via=IP_GATEWAY, device=nic, table=IP_TABLE), Route(network=IP_NETWORK_AND_CIDR, - ipaddr=IP_ADDRESS, device=nic, table=IP_TABLE)] + via=IP_ADDRESS, device=nic, table=IP_TABLE)] for route in routes: self.assertFalse(routeExists(route)) routeAdd(route) @@ -1370,10 +1370,10 @@ deviceName = NETWORK_NAME if bridged else nics[0] # Assert that routes and rules exist - routes = [Route(network='0.0.0.0/0', ipaddr=IP_GATEWAY, + routes = [Route(network='0.0.0.0/0', via=IP_GATEWAY, device=deviceName, table=IP_TABLE), Route(network=IP_NETWORK_AND_CIDR, - ipaddr=IP_ADDRESS, device=deviceName, + via=IP_ADDRESS, device=deviceName, table=IP_TABLE)] rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE), Rule(destination=IP_NETWORK_AND_CIDR, table=IP_TABLE, diff --git a/tests/ipwrapperTests.py b/tests/ipwrapperTests.py index d85deb3..8ad9db0 100644 --- a/tests/ipwrapperTests.py +++ b/tests/ipwrapperTests.py @@ -145,7 +145,7 @@ self.assertEqual(devices[-2].name, 'p1p3.13') def testRouteFromText(self): - _getRouteAttrs = lambda x: (x.network, x.ipaddr, x.device, x.table) + _getRouteAttrs = lambda x: (x.network, x.via, x.device, x.table) good_routes = { 'default via 192.168.99.254 dev eth0': ('0.0.0.0/0', '192.168.99.254', 'eth0', None), @@ -158,7 +158,7 @@ self.assertEqual(_getRouteAttrs(route), attributes) bad_routes = ['default via 192.168.99.257 dev eth0 table foo', - '200.100.50.0/16 dev eth2 table foo', + '200.100.50.0/16 dev eth2 table foo superfluousKey', '288.100.23.9/43 via 192.168.99.254 dev eth0 table foo', '200.100.50.0/16 via 192.168.99.254 table foo'] for text in bad_routes: diff --git a/vdsm/sourceRoute.py b/vdsm/sourceRoute.py index 73fe075..0b847b6 100644 --- a/vdsm/sourceRoute.py +++ b/vdsm/sourceRoute.py @@ -53,9 +53,9 @@ return netaddr.IPAddress(self.ipaddr).value def _buildRoutes(self): - return [Route(network='0.0.0.0/0', ipaddr=self.gateway, + return [Route(network='0.0.0.0/0', via=self.gateway, device=self.device, table=self.table), - Route(network=self.network, ipaddr=self.ipaddr, + Route(network=self.network, via=self.ipaddr, device=self.device, table=self.table)] def _buildRules(self): -- To view, visit http://gerrit.ovirt.org/21788 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: newchange Gerrit-Change-Id: Id2d62ad31bd9d5c7bea28605a0b195c1886f0dab Gerrit-PatchSet: 1 Gerrit-Project: vdsm Gerrit-Branch: master Gerrit-Owner: Assaf Muller <[email protected]> _______________________________________________ vdsm-patches mailing list [email protected] https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
