Change in vdsm[master]: tests : setupNetworks resizeBond, remove param validation.

Giuseppe Vallarelli has uploaded a new change for review.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

tests : setupNetworks resizeBond, remove param validation.

Added a couple of tests:

* testSetupNetworksResizeBond (functional test).
* testValidateNetSetupRemoveParamValidation (unit test).

Adding further coverage of setupNetworks and functions
on which it depends.

Change-Id: I6471a50152314e53e86e9f8e2e1359059fff17f0
Signed-off-by: Giuseppe Vallarelli 
---
M tests/configNetworkTests.py
M tests/functional/networkTests.py
M tests/functional/utils.py
3 files changed, 54 insertions(+), 0 deletions(-)

git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/22/17922/1

diff --git a/tests/configNetworkTests.py b/tests/configNetworkTests.py
index 84fad85..9e30400 100644
--- a/tests/configNetworkTests.py
+++ b/tests/configNetworkTests.py
@@ -163,3 +163,13 @@
         # bridgeless network exists
         self._addNetworkWithExc('test', dict(nics=['eth8'], bridged=False,
                                 _netinfo=fakeInfo), neterrors.ERR_BAD_PARAMS)
+
+    @MonkeyPatch(netinfo, 'NetInfo', lambda: None)
+    def testValidateNetSetupRemoveParamValidation(self):
+        attrs = dict(nic='dummy', remove=True,
+                     bridged=True)
+        networks = {'test-netowrk': attrs}
+        with self.assertRaises(neterrors.ConfigNetworkError) as cneContext:
+            configNetwork._validateNetworkSetup(networks, {})
+        self.assertEqual(cneContext.exception.errCode,
+                         neterrors.ERR_BAD_PARAMS)
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 2b0758d..86b67e6 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -445,3 +445,43 @@
                                                           {})

self.assertEquals(status, SUCCESS, msg)
+
+    @permutations([[True], [False]])
+    @RequireDummyMod
+    @ValidateRunningAsRoot
+    def testSetupNetworksResizeBond(self, bridged):
+        with dummyIf(3) as nics:
+            with self.vdsm_net.pinger():
+                bondings = {BONDING_NAME: dict(nics=nics[:1],
+                                               bridged=bridged)}
+                status, msg = self.vdsm_net.setupNetworks({}, bondings, {})
+
+                self.assertEquals(status, SUCCESS, msg)
+
+                self.vdsm_net.bondExists(BONDING_NAME, nics=nics[:1])
+
+                # Increase bond size
+                bondings[BONDING_NAME]['nics'] = nics
+                status, msg = self.vdsm_net.setupNetworks({}, bondings, {})
+
+                self.assertEquals(status, SUCCESS, msg)
+
+                self.vdsm_net.bondExists(BONDING_NAME, nics)
+
+                # Reduce bond size
+                reqmode = '3'
+                bondings[BONDING_NAME]['nics'] = nics[:2]
+                bondings[BONDING_NAME]['options'] = 'mode=%s' % reqmode
+                status, msg = self.vdsm_net.setupNetworks({}, bondings, {})
+
+                self.assertEquals(status, SUCCESS, msg)
+
+                self.vdsm_net.bondExists(BONDING_NAME, nics[:2])
+                # TODO
+                # self.assertEquals(self.vdsm_net.getBondMode(BONDING_NAME),
+                #                   reqmode)
+
+                bondings = {BONDING_NAME: dict(remove=True)}
+                status, msg = self.vdsm_net.setupNetworks({}, bondings, {})
+
+                self.assertEquals(status, SUCCESS, msg)
diff --git a/tests/functional/utils.py b/tests/functional/utils.py
index 8cb995c..6dc96e1 100644
--- a/tests/functional/utils.py
+++ b/tests/functional/utils.py
@@ -207,6 +207,10 @@
             return self.netinfo.nics[name]['mtu']
         return None

+    # TODO Add impl.
+    def getBondMode(self, bond):
+        pass
+
     @contextmanager
     def pinger(self):
         """Keeps pinging vdsm for operations that need it"""

Giuseppe Vallarelli has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 1: Verified+1

I need to enable an assertion checking the bond mode it will done once this patch will be merged http://gerrit.ovirt.org/#/c/17491/ by exposing bonding options in netinfo module.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 1:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3834/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3751/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2945/ : SUCCESS

Giuseppe Vallarelli has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 1: -Verified

Waiting for http://gerrit.ovirt.org/#/c/17491/ in order to add another assertion with the bond mode.

Giuseppe Vallarelli has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 2: Verified+1

Cherrypicked on top of the updated dependency:

Waiting for http://gerrit.ovirt.org/#/c/17491/ in order to add another assertion with the bond mode.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 2:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/3849/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/3766/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/2960/ : SUCCESS

Giuseppe Vallarelli has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 3: Verified+1

Added the missing assertion (bond mode) to the test.

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 3:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4237/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3342/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4158/ : SUCCESS

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 3: Code-Review-1

(2 comments)

....................................................
File tests/functional/networkTests.py
Line 1116: 
Line 1117:                 self.vdsm_net.bondExists(BONDING_NAME, nics)
Line 1118: 
Line 1119:                 # Reduce bond size
Line 1120:                 reqmode = '3'
I'd make reqmode a REQMODE, as it is a function-local constant.
Line 1121:                 bondings[BONDING_NAME]['nics'] = nics[:2]
Line 1122:                 bondings[BONDING_NAME]['options'] = 'mode=%s' % reqmode
Line 1123:                 status, msg = self.vdsm_net.setupNetworks({}, bondings, {})
Line 1124:

Line 1125:                 self.assertEquals(status, SUCCESS, msg)
Line 1126: 
Line 1127:                 self.vdsm_net.bondExists(BONDING_NAME, nics[:2])
Line 1128:                 self.assertEquals(self.vdsm_net.getBondMode(BONDING_NAME),
Line 1129:                                   ['broadcast', reqmode])
you should not hard-code the textual meaning of reqmode==3.
Making getBondMode return only the numeric value would be cleaner and just as safe. Text is only for humans ;-)
Line 1130: 
Line 1131:                 bondings = {BONDING_NAME: dict(remove=True)}
Line 1132:                 status, msg = self.vdsm_net.setupNetworks({}, bondings, {})
Line 1133:

Giuseppe Vallarelli has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 4: Verified+1

Addressed comments in the code review

Giuseppe Vallarelli has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 3:

(2 comments)

....................................................
File tests/functional/networkTests.py
Line 1116: 
Line 1117:                 self.vdsm_net.bondExists(BONDING_NAME, nics)
Line 1118: 
Line 1119:                 # Reduce bond size
Line 1120:                 reqmode = '3'
Done
Line 1121:                 bondings[BONDING_NAME]['nics'] = nics[:2]
Line 1122:                 bondings[BONDING_NAME]['options'] = 'mode=%s' % reqmode
Line 1123:                 status, msg = self.vdsm_net.setupNetworks({}, bondings, {})
Line 1124:

Line 1125:                 self.assertEquals(status, SUCCESS, msg)
Line 1126: 
Line 1127:                 self.vdsm_net.bondExists(BONDING_NAME, nics[:2])
Line 1128:                 self.assertEquals(self.vdsm_net.getBondMode(BONDING_NAME),
Line 1129:                                   ['broadcast', reqmode])
... and networking people know bonding modes by heart :-)
Line 1130: 
Line 1131:                 bondings = {BONDING_NAME: dict(remove=True)}
Line 1132:                 status, msg = self.vdsm_net.setupNetworks({}, bondings, {})
Line 1133:

[Jenkins CI RO]

oVirt Jenkins CI Server has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 4:

Build Successful

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit/4250/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_unit_tests_gerrit_el/3355/ : SUCCESS

http://jenkins.ovirt.org/job/vdsm_pep8_gerrit/4171/ : SUCCESS

[Dan Kenigsberg]

Dan Kenigsberg has posted comments on this change.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

Patch Set 4: Code-Review+2

[Dan Kenigsberg]

Dan Kenigsberg has submitted this change and it was merged.

Change subject: tests : setupNetworks resizeBond, remove param validation.
......................................................................

tests : setupNetworks resizeBond, remove param validation.

Added a couple of tests:

* testSetupNetworksResizeBond (functional test).
* testValidateNetSetupRemoveParamValidation (unit test).

Adding further coverage of setupNetworks and functions
on which it depends.

Change-Id: I6471a50152314e53e86e9f8e2e1359059fff17f0
Signed-off-by: Giuseppe Vallarelli 
Reviewed-on: http://gerrit.ovirt.org/17922
Reviewed-by: Dan Kenigsberg 
---
M tests/configNetworkTests.py
M tests/functional/networkTests.py
M tests/functional/utils.py
3 files changed, 55 insertions(+), 0 deletions(-)

Approvals:
  Giuseppe Vallarelli: Verified
  Dan Kenigsberg: Looks good to me, approved