Change in vdsm[master]: Functional test for Multiple Gateways source routing

danken at redhat.com danken at redhat.com
Sun Oct 5 16:45:19 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: Functional test for Multiple Gateways source routing
......................................................................


Patch Set 4: Code-Review-1

(4 comments)

http://gerrit.ovirt.org/#/c/33612/4/tests/functional/dhcp.py
File tests/functional/dhcp.py:

Line 51:         self.proc = execCmd([
Line 52:             _DNSMASQ_BINARY.cmd, '--dhcp-authoritative',
Line 53:             '-p', '0', '--dhcp-range=' + dhcpRangeFrom + ',' +
Line 54:             dhcpRangeTo + ',2m',
Line 55:             '--dhcp-option=3,%s' % (router,) if router else '--dhcp-option=3',
Consider

  '--dhcp-option=3' + ',%s' % (router,) if router else ''

as it has less repetition.
Line 56:             '-O', '6',
Line 57:             '-i', interface, '-I', 'lo', '-d',
Line 58:             '--bind-interfaces'], sync=False)
Line 59:         sleep(_START_CHECK_TIMEOUT)


http://gerrit.ovirt.org/#/c/33612/4/tests/functional/networkTests.py
File tests/functional/networkTests.py:

Line 94:         dummy.remove(nic)
Line 95: 
Line 96: 
Line 97: @contextmanager
Line 98: def dnsmasqDhcp(interface, router=None):
Could you remind me why the new art is unavoidable? Cannot we always pass router=GATEWAY?
Line 99:     """Manages the life cycle of dnsmasq as a DHCP server."""
Line 100:     dhcpServer = dhcp.Dnsmasq()
Line 101:     try:
Line 102:         dhcpServer.start(interface, DHCP_RANGE_FROM, DHCP_RANGE_TO,


Line 1946:                              srcDevice=str(device_name))
Line 1947:                 rule2 = Rule(table=routing_table,
Line 1948:                              source=IP_NETWORK_AND_CIDR)
Line 1949: 
Line 1950:                 # DHCP source routing configuration is async hence we wait
Where's the patient wait? Didn't you need to use until.retry?
Line 1951:                 # patiently
Line 1952:                 self.assertTrue(ruleExists(rule1))
Line 1953:                 self.assertTrue(ruleExists(rule2))
Line 1954:                 routes = [Route.fromText(r)


http://gerrit.ovirt.org/#/c/33612/4/tests/functional/utils.py
File tests/functional/utils.py:

Line 225:             result['info']
Line 226: 
Line 227:     def updateVmPolicy(self, vmId, vcpuLimit):
Line 228:         result = self.vdscli.updateVmPolicy([vmId, vcpuLimit])
Line 229:         return result['status']['code'], result['status']['message']
Please avoid unrelated whitespace changes


-- 
To view, visit http://gerrit.ovirt.org/33612
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ib510bb56c205dffc98aa2bae3f55c1b4c7f6f56f
Gerrit-PatchSet: 4
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Ido Barkan <ibarkan at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Assaf Muller <amuller at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list