Hello Assaf Muller, Dan Kenigsberg,
I'd like you to do a code review. Please visit
http://gerrit.ovirt.org/34339
to review the following change.
Change subject: Functional test for Multiple Gateways source routing
......................................................................
Functional test for Multiple Gateways source routing
adding assertions on the source routing definitions that are configured
when the network is configured with DHCP.
Conflicts:
tests/functional/networkTests.py
Change-Id: Ib510bb56c205dffc98aa2bae3f55c1b4c7f6f56f
Signed-off-by: ibarkan <[email protected]>
Reviewed-on: http://gerrit.ovirt.org/33612
Reviewed-by: Assaf Muller <[email protected]>
Reviewed-by: Dan Kenigsberg <[email protected]>
---
M AUTHORS
M tests/functional/dhcp.py
M tests/functional/networkTests.py
M vdsm/network/sourceroute.py
4 files changed, 77 insertions(+), 37 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/39/34339/1
diff --git a/AUTHORS b/AUTHORS
index 25197a5..190c018 100644
--- a/AUTHORS
+++ b/AUTHORS
@@ -13,6 +13,7 @@
Miguel Angel Ajo <[email protected]>
Timothy Asir <[email protected]>
Haim Ateya <[email protected]>
+ Ido Barkan <[email protected]>
Daniel P. Berrange <[email protected]>
Yaniv Bronhaim <[email protected]>
Humble Chirammal <[email protected]>
diff --git a/tests/functional/dhcp.py b/tests/functional/dhcp.py
index abed8c8..28d1220 100644
--- a/tests/functional/dhcp.py
+++ b/tests/functional/dhcp.py
@@ -42,15 +42,17 @@
def __init__(self):
self.proc = None
- def start(self, interface, dhcpRangeFrom, dhcpRangeTo):
- # --dhcp-option=3 don't send gateway address which would break routing
+ def start(self, interface, dhcpRangeFrom, dhcpRangeTo, router=None):
+ # --dhcp-option=3,<router> advertise specific router (can be None)
# -O 6 don't reply with any DNS servers either
# -d do not daemonize and log to stderr
# -p 0 disable all the dnsmasq dns functionality
self.proc = execCmd([
_DNSMASQ_BINARY.cmd, '--dhcp-authoritative',
'-p', '0', '--dhcp-range=' + dhcpRangeFrom + ',' +
- dhcpRangeTo + ',2m', '--dhcp-option=3', '-O', '6',
+ dhcpRangeTo + ',2m',
+ '--dhcp-option=3' + ',%s' % (router,) if router else '',
+ '-O', '6',
'-i', interface, '-I', 'lo', '-d',
'--bind-interfaces'], sync=False)
sleep(_START_CHECK_TIMEOUT)
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 2f1e035..974918f 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -20,8 +20,10 @@
from functools import wraps
import os.path
import json
+import netaddr
from hookValidation import ValidatesHook
+from network.sourceroute import StaticSourceRoute
from testrunner import (VdsmTestCase as TestCaseBase, namedTemporaryDir,
expandPermutations, permutations)
from testValidation import (brokentest, RequireDummyMod, RequireVethMod,
@@ -36,7 +38,7 @@
from vdsm.ipwrapper import (ruleAdd, ruleDel, routeAdd, routeDel, routeExists,
ruleExists, Route, Rule, addrFlush, LinkType,
- getLinks)
+ getLinks, routeShowTable)
from vdsm.constants import EXT_BRCTL
from vdsm.utils import RollbackContext, execCmd
@@ -58,8 +60,6 @@
IP_CIDR = '24'
IP_NETWORK_AND_CIDR = IP_NETWORK + '/' + IP_CIDR
IP_GATEWAY = '240.0.0.254'
-# Current implementation converts ip to its 32 bit int representation
-IP_TABLE = '4026531841'
DHCP_RANGE_FROM = '240.0.0.10'
DHCP_RANGE_TO = '240.0.0.100'
CUSTOM_PROPS = {'linux': 'rules', 'vdsm': 'as well'}
@@ -94,7 +94,8 @@
"""Manages the life cycle of dnsmasq as a DHCP server."""
dhcpServer = dhcp.Dnsmasq()
try:
- dhcpServer.start(interface, DHCP_RANGE_FROM, DHCP_RANGE_TO)
+ dhcpServer.start(interface, DHCP_RANGE_FROM, DHCP_RANGE_TO,
+ router=IP_GATEWAY)
except dhcp.DhcpError as e:
raise SkipTest(e)
@@ -263,6 +264,40 @@
self.assertFalse(
self.vdsm_net._vlanInRunningConfig(devName, vlanId),
'%s found unexpectedly in running config' % vlanName)
+
+ def assertRouteExists(self, route, routing_table='all'):
+ if not routeExists(route):
+ raise self.failureException(
+ "routing rule [%s] wasn't found. existing rules: \n%s" % (
+ route, routeShowTable(routing_table)))
+
+ def assertRouteDoesNotExist(self, route, routing_table='all'):
+ if routeExists(route):
+ raise self.failureException(
+ "routing rule [%s] found. existing rules: \n%s" % (
+ route, routeShowTable(routing_table)))
+
+ def assertSourceRoutingConfiguration(self, deviceName, vdsm_net_name):
+ """assert that the IP rules and the routing tables pointed by them
+ are configured correctly in order to implement source routing"""
+ vdsm_net = self.vdsm_net.netinfo.networks[vdsm_net_name]
+ routing_table = str(StaticSourceRoute.generateTableId(
+ vdsm_net['addr']))
+
+ routes = [Route(network='0.0.0.0/0', via=IP_GATEWAY,
+ device=deviceName, table=routing_table),
+ Route(network=IP_NETWORK_AND_CIDR,
+ via=str(netaddr.IPAddress(vdsm_net['addr'])),
+ device=deviceName, table=routing_table)]
+ rules = [Rule(source=IP_NETWORK_AND_CIDR, table=routing_table),
+ Rule(destination=IP_NETWORK_AND_CIDR, table=routing_table,
+ srcDevice=deviceName)]
+
+ for route in routes:
+ self.assertRouteExists(route, routing_table)
+ for rule in rules:
+ self.assertTrue(ruleExists(rule))
+ return routes, rules
def assertMtu(self, mtu, *elems):
for elem in elems:
@@ -1504,10 +1539,11 @@
dummy.setIP(nic, IP_ADDRESS, IP_CIDR)
try:
dummy.setLinkUp(nic)
-
- rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE),
- Rule(destination=IP_NETWORK_AND_CIDR, table=IP_TABLE,
- srcDevice=nic)]
+ routing_table = str(StaticSourceRoute.generateTableId(
+ IP_ADDRESS))
+ rules = [Rule(source=IP_NETWORK_AND_CIDR, table=routing_table),
+ Rule(destination=IP_NETWORK_AND_CIDR,
+ table=routing_table, srcDevice=nic)]
for rule in rules:
self.assertFalse(ruleExists(rule))
ruleAdd(rule)
@@ -1526,16 +1562,18 @@
try:
dummy.setLinkUp(nic)
+ routing_table = str(StaticSourceRoute.generateTableId(
+ IP_ADDRESS))
routes = [Route(network='0.0.0.0/0', via=IP_GATEWAY,
- device=nic, table=IP_TABLE),
- Route(network=IP_NETWORK_AND_CIDR,
- via=IP_ADDRESS, device=nic, table=IP_TABLE)]
+ device=nic, table=routing_table),
+ Route(network=IP_NETWORK_AND_CIDR, via=IP_ADDRESS,
+ device=nic, table=routing_table)]
for route in routes:
- self.assertFalse(routeExists(route))
+ self.assertRouteDoesNotExist(route)
routeAdd(route)
self.assertTrue(routeExists(route))
routeDel(route)
- self.assertFalse(routeExists(route))
+ self.assertRouteDoesNotExist(route)
finally:
addrFlush(nic)
@@ -1556,20 +1594,8 @@
deviceName = NETWORK_NAME if bridged else nics[0]
- # Assert that routes and rules exist
- routes = [Route(network='0.0.0.0/0', via=IP_GATEWAY,
- device=deviceName, table=IP_TABLE),
- Route(network=IP_NETWORK_AND_CIDR,
- 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,
- srcDevice=deviceName)]
-
- for route in routes:
- self.assertTrue(routeExists(route))
- for rule in rules:
- self.assertTrue(ruleExists(rule))
+ routes, rules = self.assertSourceRoutingConfiguration(
+ deviceName, NETWORK_NAME)
status, msg = self.vdsm_net.setupNetworks(
{NETWORK_NAME: {'remove': True}},
@@ -1578,7 +1604,7 @@
# Assert that routes and rules don't exist
for route in routes:
- self.assertFalse(routeExists(route))
+ self.assertRouteDoesNotExist(route)
for rule in rules:
self.assertFalse(ruleExists(rule))
@@ -1830,26 +1856,36 @@
self.assertEqual(status, SUCCESS, msg)
self.assertNetworkExists(NETWORK_NAME)
- net = self.vdsm_net.netinfo.networks[NETWORK_NAME]
- self.assertEqual(net['bootproto4'], 'dhcp')
+ vdsm_net = self.vdsm_net.netinfo.networks[NETWORK_NAME]
+ self.assertEqual(vdsm_net['bootproto4'], 'dhcp')
if bridged:
- self.assertEqual(net['cfg']['BOOTPROTO'], 'dhcp')
+ self.assertEqual(vdsm_net['cfg']['BOOTPROTO'], 'dhcp')
devs = self.vdsm_net.netinfo.bridges
self.assertIn(NETWORK_NAME, devs)
self.assertEqual(devs[NETWORK_NAME]['cfg']['BOOTPROTO'],
'dhcp')
+ device_name = NETWORK_NAME
else:
devs = self.vdsm_net.netinfo.nics
self.assertIn(right, devs)
self.assertEqual(devs[right]['cfg']['BOOTPROTO'], 'dhcp')
+ device_name = right
+
+ routes, rules = self.assertSourceRoutingConfiguration(
+ device_name, NETWORK_NAME)
network = {NETWORK_NAME: {'remove': True}}
status, msg = self.vdsm_net.setupNetworks(network, {}, NOCHK)
self.assertEqual(status, SUCCESS, msg)
self.assertNetworkDoesntExist(NETWORK_NAME)
+ # Assert that routes and rules don't exist
+ for route in routes:
+ self.assertRouteDoesNotExist(route)
+ for rule in rules:
+ self.assertFalse(ruleExists(rule))
@permutations([['default'], ['local']])
@cleanupNet
diff --git a/vdsm/network/sourceroute.py b/vdsm/network/sourceroute.py
index da78ac4..e1c03f0 100644
--- a/vdsm/network/sourceroute.py
+++ b/vdsm/network/sourceroute.py
@@ -48,9 +48,10 @@
self.routes = None
self.rules = None
- def _generateTableId(self):
+ @staticmethod
+ def generateTableId(ipaddr):
# TODO: Future proof for IPv6
- return netaddr.IPAddress(self.ipaddr).value
+ return netaddr.IPAddress(ipaddr).value
def _buildRoutes(self):
return [Route(network='0.0.0.0/0', via=self.gateway,
@@ -71,7 +72,7 @@
self.ipaddr = ipaddr
self.mask = mask
self.gateway = gateway
- self.table = self._generateTableId()
+ self.table = StaticSourceRoute.generateTableId(self.ipaddr)
network = netaddr.IPNetwork(str(self.ipaddr) + '/' + str(self.mask))
self.network = "%s/%s" % (network.network, network.prefixlen)
--
To view, visit http://gerrit.ovirt.org/34339
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib510bb56c205dffc98aa2bae3f55c1b4c7f6f56f
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ido Barkan <[email protected]>
Gerrit-Reviewer: Assaf Muller <[email protected]>
Gerrit-Reviewer: Dan Kenigsberg <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches
