Assaf Muller has uploaded a new change for review.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists
Added functional tests for both functions as well.
Change-Id: Iaf7c28e85f53f51879750308f3a9f93fc635cf33 Signed-off-by: Assaf Muller amuller@redhat.com --- M lib/vdsm/ipwrapper.py M tests/functional/networkTests.py 2 files changed, 80 insertions(+), 1 deletion(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/40/18040/1
diff --git a/lib/vdsm/ipwrapper.py b/lib/vdsm/ipwrapper.py index 6eade35..66458a4 100644 --- a/lib/vdsm/ipwrapper.py +++ b/lib/vdsm/ipwrapper.py @@ -106,6 +106,12 @@ for word in str(self).split(): yield word
+ def __eq__(self, other): + return self.__dict__ == other.__dict__ + + def __ne__(self, other): + return not self.__eq__(other) +
class Rule(object): def __init__(self, table, source=None, destination=None, srcDevice=None): @@ -191,6 +197,12 @@ for word in str(self).split(): yield word
+ def __eq__(self, other): + return self.__dict__ == other.__dict__ + + def __ne__(self, other): + return not self.__eq__(other) +
class IPRoute2Error(Exception): pass @@ -233,6 +245,20 @@ _execCmd(command)
+def _validate(validator, entry): + try: + validator.fromText(entry) + except: + return False + else: + return True + + +def routeExists(route): + return route in [Route.fromText(entry) for entry in routeShowTable('all') + if _validate(Route, entry)] + + def ruleList(): command = [_IP_BINARY.cmd, 'rule'] return _execCmd(command) @@ -248,3 +274,8 @@ command = [_IP_BINARY.cmd, 'rule', 'del'] command += rule _execCmd(command) + + +def ruleExists(rule): + return rule in [Rule.fromText(entry) for entry in ruleList() + if _validate(Rule, entry)] diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 5fbbc73..10e42dc 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -22,13 +22,24 @@ expandPermutations, permutations) from testValidation import RequireDummyMod, ValidateRunningAsRoot
-from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy +import dummyUtils from dummyUtils import dummyIf +from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy + +from vdsm.ipwrapper import ruleAdd, ruleDel, routeAdd, routeDel, routeExists, \ + ruleExists, Route, Rule
NETWORK_NAME = 'test-network' VLAN_ID = '27' BONDING_NAME = 'bond0' +IP_ADDRESS = '240.0.0.1' +IP_NETWORK = '240.0.0.0' +IP_CIDR = '24' +IP_NETWORK_AND_CIDR = IP_NETWORK + '/' + IP_CIDR +IP_GATEWAY = '240.0.0.254' +IP_TABLE = '4026531841' # Current implementation converts ip to its 32 bit int + # representation
def setupModule(): @@ -394,3 +405,40 @@ nics=nics, opts={'bridged': bridged}) self.assertEqual(status, neterrors.ERR_BAD_BRIDGE, msg) + + @RequireDummyMod + @ValidateRunningAsRoot + def testRuleExists(self): + with dummyIf(1) as nics: + nic = nics[0] + dummyUtils.setIP(nic, IP_ADDRESS + '/' + IP_CIDR) + dummyUtils.setLinkUp(nic) + + rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE), + Rule(destination=IP_NETWORK_AND_CIDR, table=IP_TABLE, + srcDevice=nic)] + for rule in rules: + self.assertFalse(ruleExists(rule)) + ruleAdd(rule) + self.assertTrue(ruleExists(rule)) + ruleDel(rule) + self.assertFalse(ruleExists(rule)) + + @RequireDummyMod + @ValidateRunningAsRoot + def testRouteExists(self): + with dummyIf(1) as nics: + nic = nics[0] + dummyUtils.setIP(nic, IP_ADDRESS + '/' + IP_CIDR) + dummyUtils.setLinkUp(nic) + + routes = [Route(network='0.0.0.0/0', ipaddr=IP_GATEWAY, + device=nic, table=IP_TABLE), + Route(network=IP_NETWORK_AND_CIDR, + ipaddr=IP_ADDRESS, device=nic, table=IP_TABLE)] + for route in routes: + self.assertFalse(routeExists(route)) + routeAdd(route) + self.assertTrue(routeExists(route)) + routeDel(route) + self.assertFalse(routeExists(route))
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 1:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3902/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3819/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3013/ : SUCCESS
Giuseppe Vallarelli has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 1: Code-Review-1
(4 comments)
It's pretty good very minor comments.
.................................................... File lib/vdsm/ipwrapper.py Line 246: Line 247: Line 248: def _validate(validator, entry): Line 249: try: Line 250: validator.fromText(entry) I don't understand this choice of having fromText raising an exception and using the exception as a signal of meaning entry valid or invalid. fromText should return already this boolean instead of having clients of this method wrapping it in try..except. Line 251: except: Line 252: return False Line 253: else: Line 254: return True
.................................................... File tests/functional/networkTests.py Line 21: from testrunner import (VdsmTestCase as TestCaseBase, Line 22: expandPermutations, permutations) Line 23: from testValidation import RequireDummyMod, ValidateRunningAsRoot Line 24: Line 25: import dummyUtils I found the Utils word a bit overused, why not simply dummy we already have an utils module in tests/functional Line 26: from dummyUtils import dummyIf Line 27: from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy Line 28: Line 29: from vdsm.ipwrapper import ruleAdd, ruleDel, routeAdd, routeDel, routeExists, \
Line 25: import dummyUtils Line 26: from dummyUtils import dummyIf Line 27: from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy Line 28: Line 29: from vdsm.ipwrapper import ruleAdd, ruleDel, routeAdd, routeDel, routeExists, \ Don't use \ use parenthesis instead (...) Line 30: ruleExists, Route, Rule Line 31: Line 32: Line 33: NETWORK_NAME = 'test-network'
Line 410: @ValidateRunningAsRoot Line 411: def testRuleExists(self): Line 412: with dummyIf(1) as nics: Line 413: nic = nics[0] Line 414: dummyUtils.setIP(nic, IP_ADDRESS + '/' + IP_CIDR) you're creating IP_ADDRESS '/' + IP_CIDR twice across the two tests can we do only once? You can set up vars shared by more tests in the setUp. Line 415: dummyUtils.setLinkUp(nic) Line 416: Line 417: rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE), Line 418: Rule(destination=IP_NETWORK_AND_CIDR, table=IP_TABLE,
Giuseppe Vallarelli has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 1:
(1 comment)
Added a comment for cleaning up added rules/routes.
.................................................... File tests/functional/networkTests.py Line 419: srcDevice=nic)] Line 420: for rule in rules: Line 421: self.assertFalse(ruleExists(rule)) Line 422: ruleAdd(rule) Line 423: self.assertTrue(ruleExists(rule)) if an assertion fails how do we cleanup the added rule? Line 424: ruleDel(rule) Line 425: self.assertFalse(ruleExists(rule)) Line 426: Line 427: @RequireDummyMod
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 1:
(5 comments)
.................................................... File lib/vdsm/ipwrapper.py Line 246: Line 247: Line 248: def _validate(validator, entry): Line 249: try: Line 250: validator.fromText(entry) fromText returns a class instance if successful. I view it as a constructor so I go with the Java/C#/C++ way of throwing exceptions in constructors to raise a red flag if something goes wrong. Line 251: except: Line 252: return False Line 253: else: Line 254: return True
.................................................... File tests/functional/networkTests.py Line 21: from testrunner import (VdsmTestCase as TestCaseBase, Line 22: expandPermutations, permutations) Line 23: from testValidation import RequireDummyMod, ValidateRunningAsRoot Line 24: Line 25: import dummyUtils Done Line 26: from dummyUtils import dummyIf Line 27: from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy Line 28: Line 29: from vdsm.ipwrapper import ruleAdd, ruleDel, routeAdd, routeDel, routeExists, \
Line 25: import dummyUtils Line 26: from dummyUtils import dummyIf Line 27: from utils import cleanupNet, restoreNetConfig, SUCCESS, VdsProxy Line 28: Line 29: from vdsm.ipwrapper import ruleAdd, ruleDel, routeAdd, routeDel, routeExists, \ Done Line 30: ruleExists, Route, Rule Line 31: Line 32: Line 33: NETWORK_NAME = 'test-network'
Line 410: @ValidateRunningAsRoot Line 411: def testRuleExists(self): Line 412: with dummyIf(1) as nics: Line 413: nic = nics[0] Line 414: dummyUtils.setIP(nic, IP_ADDRESS + '/' + IP_CIDR) Done Line 415: dummyUtils.setLinkUp(nic) Line 416: Line 417: rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE), Line 418: Rule(destination=IP_NETWORK_AND_CIDR, table=IP_TABLE,
Line 419: srcDevice=nic)] Line 420: for rule in rules: Line 421: self.assertFalse(ruleExists(rule)) Line 422: ruleAdd(rule) Line 423: self.assertTrue(ruleExists(rule)) Done Line 424: ruleDel(rule) Line 425: self.assertFalse(ruleExists(rule)) Line 426: Line 427: @RequireDummyMod
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 2: Verified+1
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 2:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3994/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3911/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3105/ : SUCCESS
Antoni Segura Puimedon has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 2:
(3 comments)
.................................................... File lib/vdsm/ipwrapper.py Line 254: Line 255: def _validate(validator, entry): Line 256: try: Line 257: validator.fromText(entry) Line 258: except: What kind of exception are you likely or do you expect to get? This would catch even base exceptions. Line 259: return False Line 260: else: Line 261: return True Line 262:
Line 261: return True Line 262: Line 263: Line 264: def routeExists(route): Line 265: return route in [Route.fromText(entry) for entry in routeShowTable('all') I'd prefer this to be a generator than a list comprehension. Line 266: if _validate(Route, entry)] Line 267: Line 268: Line 269: def ruleList():
Line 283: _execCmd(command) Line 284: Line 285: Line 286: def ruleExists(rule): Line 287: return rule in [Rule.fromText(entry) for entry in ruleList() I'd prefer this to be a generator than a list comprehension.
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 2:
(3 comments)
.................................................... File lib/vdsm/ipwrapper.py Line 254: Line 255: def _validate(validator, entry): Line 256: try: Line 257: validator.fromText(entry) Line 258: except: Done Line 259: return False Line 260: else: Line 261: return True Line 262:
Line 261: return True Line 262: Line 263: Line 264: def routeExists(route): Line 265: return route in [Route.fromText(entry) for entry in routeShowTable('all') Done Line 266: if _validate(Route, entry)] Line 267: Line 268: Line 269: def ruleList():
Line 283: _execCmd(command) Line 284: Line 285: Line 286: def ruleExists(rule): Line 287: return rule in [Rule.fromText(entry) for entry in ruleList() Done
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 3:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4034/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3139/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3946/ : SUCCESS
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 3: Verified+1
Antoni Segura Puimedon has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 3:
(2 comments)
.................................................... File lib/vdsm/ipwrapper.py Line 251: command += route Line 252: _execCmd(command) Line 253: Line 254: Line 255: def _getValidEntries(ofType, outOf): How about this:
def _getValidEntries(constructor, iterator): for element in iterator: try: yield constructor(iterator) except ValueError: pass
So using code reads:
return route in _getValidEntries(Route.fromText, routeShowTable('all')) return rule in _getValidEntries(Rule.fromText, ruleList()) Line 256: for entry in outOf: Line 257: try: Line 258: yield ofType.fromText(entry) Line 259: except ValueError:
Line 260: pass Line 261: Line 262: Line 263: def routeExists(route): Line 264: return route in _getValidEntries(ofType=Route, sorry that my comment was not clearer. I meant something easier:
return route in (Route.fromText(entry) for entry in routeShowTable('all') if _validate(Route, entry))
The same below. Anyway, I guess this is okay too. Line 265: outOf=routeShowTable('all')) Line 266: Line 267: Line 268: def ruleList():
Giuseppe Vallarelli has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 3: Code-Review-1
(4 comments)
.................................................... File lib/vdsm/ipwrapper.py Line 105: def __iter__(self): Line 106: for word in str(self).split(): Line 107: yield word Line 108: Line 109: def __eq__(self, other): I see that you define the same couple of methods __eq__ and __ne__ for two objects Route and Rule. You can create a class decorator to avoid this duplication here's an example:
In [9]: def equals(cls): def __eq__(self, other): return self.__dict__ == other.__dict__ cls.__eq__ = __eq__ return cls
In [10]: @equals class Pippo(object): def __init__(self, a, b): self.a = a self.b = b ....:
In [11]: c = Pippo(1, 2)
In [12]: d = Pippo(3, 4)
In [13]: c == d Out[13]: False
In [14]: e = Pippo(1, 2)
In [15]: c == e Out[15]: True Line 110: return self.__dict__ == other.__dict__ Line 111: Line 112: def __ne__(self, other): Line 113: return not self.__eq__(other)
.................................................... File tests/functional/networkTests.py Line 410: @RequireDummyMod Line 411: @ValidateRunningAsRoot Line 412: def testRuleExists(self): Line 413: with dummyIf(1) as nics: Line 414: nic = nics[0] I anticipate Toni by saying nic, = nics Line 415: dummy.setIP(nic, IP_ADDRESS, IP_CIDR) Line 416: dummy.setLinkUp(nic) Line 417: Line 418: rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE),
Line 428: @RequireDummyMod Line 429: @ValidateRunningAsRoot Line 430: def testRouteExists(self): Line 431: with dummyIf(1) as nics: Line 432: nic = nics[0] Same as previously stated. Line 433: dummy.setIP(nic, IP_ADDRESS, IP_CIDR) Line 434: dummy.setLinkUp(nic) Line 435: Line 436: routes = [Route(network='0.0.0.0/0', ipaddr=IP_GATEWAY,
.................................................... File tests/functional/utils.py Line 79: raise Line 80: return wrapper Line 81: Line 82: Line 83: def saveRules(): Can you use ipwrapper.ruleList also in cleanupRules as you did in restoreRules?
When I've read saveRules() with ipwrapper.ruleList() in the implementation I've been a little surprised. Line 84: return ipwrapper.ruleList() Line 85: Line 86: Line 87: def restoreRules(base):
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 3: -Verified
(6 comments)
.................................................... File lib/vdsm/ipwrapper.py Line 105: def __iter__(self): Line 106: for word in str(self).split(): Line 107: yield word Line 108: Line 109: def __eq__(self, other): Done Line 110: return self.__dict__ == other.__dict__ Line 111: Line 112: def __ne__(self, other): Line 113: return not self.__eq__(other)
Line 251: command += route Line 252: _execCmd(command) Line 253: Line 254: Line 255: def _getValidEntries(ofType, outOf): Done Line 256: for entry in outOf: Line 257: try: Line 258: yield ofType.fromText(entry) Line 259: except ValueError:
Line 260: pass Line 261: Line 262: Line 263: def routeExists(route): Line 264: return route in _getValidEntries(ofType=Route, Done Line 265: outOf=routeShowTable('all')) Line 266: Line 267: Line 268: def ruleList():
.................................................... File tests/functional/networkTests.py Line 410: @RequireDummyMod Line 411: @ValidateRunningAsRoot Line 412: def testRuleExists(self): Line 413: with dummyIf(1) as nics: Line 414: nic = nics[0] Alright, but once all the rest of the functional tests are in I want to create a Dummy class. It will implement all the current stuff in dummy.py + __enter__ and __exit__ so in these tests that currently go: "with dummyIf(1) as nics:" It'd instead go: "with Dummy() as nic:" Line 415: dummy.setIP(nic, IP_ADDRESS, IP_CIDR) Line 416: dummy.setLinkUp(nic) Line 417: Line 418: rules = [Rule(source=IP_NETWORK_AND_CIDR, table=IP_TABLE),
Line 428: @RequireDummyMod Line 429: @ValidateRunningAsRoot Line 430: def testRouteExists(self): Line 431: with dummyIf(1) as nics: Line 432: nic = nics[0] Done Line 433: dummy.setIP(nic, IP_ADDRESS, IP_CIDR) Line 434: dummy.setLinkUp(nic) Line 435: Line 436: routes = [Route(network='0.0.0.0/0', ipaddr=IP_GATEWAY,
.................................................... File tests/functional/utils.py Line 79: raise Line 80: return wrapper Line 81: Line 82: Line 83: def saveRules(): The idea is to wrap a single line in a function to replace a comment, but this isn't important enough to argue about. Line 84: return ipwrapper.ruleList() Line 85: Line 86: Line 87: def restoreRules(base):
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 4: Verified+1
Addressed all review comments.
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4182/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3287/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4103/ : SUCCESS
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 5:
Build Successful
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4223/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3328/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4144/ : SUCCESS
Dan Kenigsberg has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 5: Code-Review-1
(6 comments)
.................................................... File lib/vdsm/ipwrapper.py Line 38: Line 39: def equals(cls): Line 40: def __eq__(self, other): Line 41: return self.__dict__ == other.__dict__ Line 42: cls.__eq__ = __eq__ For completeness, you should make sure that type(other) is cls. Line 43: return cls Line 44: Line 45: Line 46: @equals
Line 138: self.detached = detached Line 139: Line 140: @classmethod Line 141: def parse(cls, text): Line 142: detached = '[detached]' in text please provide a unit test with [detached] in text. Frankly, I fear wild in-text searches. They end up finding the text in unexpectedly wrong places. Line 143: rule = [entry for entry in text.split() if entry != '[detached]'] Line 144: parameters = rule[1:] Line 145: Line 146: if len(rule) % 2 == 0:
Line 182: source = None Line 183: if destination == 'all': Line 184: destination = None Line 185: srcDevice = data.get('dev') or data.get('iif') Line 186: detached = data.get('detached') do we want to have detached that is None? Line 187: Line 188: return cls(table, source=source, destination=destination, Line 189: srcDevice=srcDevice, detached=detached) Line 190:
Line 248: command += route Line 249: _execCmd(command) Line 250: Line 251: Line 252: def _getValidEntries(constructor, list): One should not override a basic type such as "list". Particularly if "list" may be any other iterable, too. Line 253: for entry in list: Line 254: try: Line 255: yield constructor(entry) Line 256: except ValueError:
.................................................... File tests/functional/utils.py Line 70: Line 71: @wraps(func) Line 72: def wrapper(*args, **kwargs): Line 73: try: Line 74: # Save the rules before the test redundant comment, imho. Line 75: base = ipwrapper.ruleList() Line 76: Line 77: func(*args, **kwargs) Line 78: except Exception:
Line 83: Line 84: Line 85: def restoreRules(base): Line 86: current = ipwrapper.ruleList() Line 87: added = list(set(current) - set(base)) there is no need to convert the set into a list. Line 88: for rule in added: Line 89: ipwrapper.ruleDel(ipwrapper.Rule.fromText(rule)) Line 90: Line 91:
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 5:
(6 comments)
.................................................... File lib/vdsm/ipwrapper.py Line 38: Line 39: def equals(cls): Line 40: def __eq__(self, other): Line 41: return self.__dict__ == other.__dict__ Line 42: cls.__eq__ = __eq__ Done Line 43: return cls Line 44: Line 45: Line 46: @equals
Line 138: self.detached = detached Line 139: Line 140: @classmethod Line 141: def parse(cls, text): Line 142: detached = '[detached]' in text Done Line 143: rule = [entry for entry in text.split() if entry != '[detached]'] Line 144: parameters = rule[1:] Line 145: Line 146: if len(rule) % 2 == 0:
Line 182: source = None Line 183: if destination == 'all': Line 184: destination = None Line 185: srcDevice = data.get('dev') or data.get('iif') Line 186: detached = data.get('detached') The two ways to construct a Rule instance put a True or False value, so we don't really want a None value in here, no. Line 187: Line 188: return cls(table, source=source, destination=destination, Line 189: srcDevice=srcDevice, detached=detached) Line 190:
Line 248: command += route Line 249: _execCmd(command) Line 250: Line 251: Line 252: def _getValidEntries(constructor, list): Done Line 253: for entry in list: Line 254: try: Line 255: yield constructor(entry) Line 256: except ValueError:
.................................................... File tests/functional/utils.py Line 70: Line 71: @wraps(func) Line 72: def wrapper(*args, **kwargs): Line 73: try: Line 74: # Save the rules before the test Done Line 75: base = ipwrapper.ruleList() Line 76: Line 77: func(*args, **kwargs) Line 78: except Exception:
Line 83: Line 84: Line 85: def restoreRules(base): Line 86: current = ipwrapper.ruleList() Line 87: added = list(set(current) - set(base)) Done Line 88: for rule in added: Line 89: ipwrapper.ruleDel(ipwrapper.Rule.fromText(rule)) Line 90: Line 91:
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 6: Verified+1
oVirt Jenkins CI Server has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 6: Verified-1
Build Failed
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4391/ : FAILURE
http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3494/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4310/ : SUCCESS
Assaf Muller has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 6:
Jenkins failure is unrelated to patch.
Dan Kenigsberg has posted comments on this change.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Patch Set 6: Code-Review+2
Dan Kenigsberg has submitted this change and it was merged.
Change subject: Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists ......................................................................
Functional Tests [2/3]: Added ipwrapper.ruleExists+routeExists
Added functional tests for both functions as well.
We can now parse rules that contain '[detached]' - If you add rules for devices that don't exist (Or add rules for devices that do exist and then delete the device) then 'ip rule' outputs the rule with '[detached]'. We couldn't parse such rules. This is important because when cleaning up rules that the test added we first get rid of the dummy nic and only then get rid of rules.
Change-Id: Iaf7c28e85f53f51879750308f3a9f93fc635cf33 Signed-off-by: Assaf Muller amuller@redhat.com Reviewed-on: http://gerrit.ovirt.org/18040 Reviewed-by: Dan Kenigsberg danken@redhat.com --- M lib/vdsm/ipwrapper.py M tests/functional/networkTests.py M tests/functional/utils.py M tests/ipwrapperTests.py 4 files changed, 127 insertions(+), 12 deletions(-)
Approvals: Assaf Muller: Verified Dan Kenigsberg: Looks good to me, approved
vdsm-patches@lists.fedorahosted.org