Hello Dan Kenigsberg,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/38771
to review the following change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
network: models: tag non-VDSM devices used by VDSM
When we setup network over pre-existing vlaned bond, models.py skip configuration of pre-existing devices under a vlan (because they are not to be changed) in order to not ifdown possibly used devices. Due to this behaviour we don't persist mentioned networks and therefore we are not able to restore them.
Now we check if device is a part of vdsm, if not, we configure and persist it.
Change-Id: I4d3ed2b45d618d404bb340100ca07bc57b2f1136 Signed-off-by: Petr Horáček phoracek@redhat.com Reviewed-on: https://gerrit.ovirt.org/38572 Reviewed-by: Dan Kenigsberg danken@redhat.com Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1154399 --- M tests/functional/networkTests.py M vdsm/network/models.py 2 files changed, 55 insertions(+), 2 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/71/38771/1
diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py index 3b3f474..8af47e4 100644 --- a/tests/functional/networkTests.py +++ b/tests/functional/networkTests.py @@ -40,10 +40,11 @@ ruleExists, Route, Rule, addrFlush, LinkType, getLinks, routeShowTable)
-from vdsm.constants import EXT_BRCTL +from vdsm.constants import EXT_BRCTL, EXT_IFUP from vdsm.utils import RollbackContext, execCmd from vdsm.netinfo import (bridges, operstate, prefix2netmask, getRouteDeviceTo, - getDhclientIfaces, BONDING_SLAVES, BONDING_MASTERS) + getDhclientIfaces, BONDING_SLAVES, BONDING_MASTERS, + NET_CONF_PREF) from vdsm import ipwrapper from vdsm.utils import pgrep
@@ -2184,3 +2185,54 @@ self.assertNetworkDoesntExist(NETWORK_NAME) self.assertBondDoesntExist(BONDING_NAME, [nic]) self.vdsm_net.save_config() + + @cleanupNet + @ValidateRunningAsRoot + def test_setupNetworks_on_external_vlaned_bond(self): + with dummyIf(1) as (nic, ): + with open(NET_CONF_PREF + nic, 'w') as f: + f.write("""DEVICE=%s +MASTER=%s +SLAVE=yes +ONBOOT=yes +MTU=1500 +NM_CONTROLLED=no""" % (nic, BONDING_NAME)) + with open(NET_CONF_PREF + BONDING_NAME, 'w') as f: + f.write("""DEVICE=%s +BONDING_OPTS='mode=802.3ad miimon=150' +ONBOOT=yes +BOOTPROTO=none +DEFROUTE=yes +NM_CONTROLLED=no +HOTPLUG=no""" % BONDING_NAME) + with open(NET_CONF_PREF + BONDING_NAME + '.' + VLAN_ID, 'w') as f: + f.write("""DEVICE=%s.%s +VLAN=yes +ONBOOT=yes +BOOTPROTO=static +NM_CONTROLLED=no +HOTPLUG=no""" % (BONDING_NAME, VLAN_ID)) + + rc, _, _ = execCmd([EXT_IFUP, BONDING_NAME]) + rc, _, _ = execCmd([EXT_IFUP, BONDING_NAME + '.' + VLAN_ID]) + + status, msg = self.vdsm_net.setupNetworks( + {NETWORK_NAME: {'bonding': BONDING_NAME, 'bridged': True, + 'vlan': VLAN_ID}}, {}, NOCHK) + self.assertEqual(status, SUCCESS, msg) + self.assertNetworkExists(NETWORK_NAME) + + self.vdsm_net.save_config() + self.vdsm_net.restoreNetConfig() + + self.assertNetworkExists(NETWORK_NAME) + self.assertBondExists(BONDING_NAME, [nic]) + self.assertVlanExists(BONDING_NAME + '.' + VLAN_ID) + + status, msg = self.vdsm_net.setupNetworks( + {NETWORK_NAME: {'remove': True}}, + {BONDING_NAME: {'remove': True}}, NOCHK) + self.assertEqual(status, SUCCESS, msg) + self.assertNetworkDoesntExist(NETWORK_NAME) + self.assertBondDoesntExist(BONDING_NAME, [nic]) + self.vdsm_net.save_config() diff --git a/vdsm/network/models.py b/vdsm/network/models.py index 2d13409..11666a0 100644 --- a/vdsm/network/models.py +++ b/vdsm/network/models.py @@ -204,6 +204,7 @@ # is already applied in any way, we can skip the configuring. if (self.vlan and self.name in netinfo.bondings() and + self.name in self.configurator.runningConfig.bonds and netinfo.operstate(self.name) == netinfo.OPERSTATE_UP and netinfo.ifaceUsed(self.name) and self.mtu <= netinfo.getMtu(self.name) and
automation@ovirt.org has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 1:
* Update tracker::#1154399::OK * Check Bug-Url::OK * Check Public Bug::#1154399::OK, public bug * Check Product::#1154399::OK, Correct product oVirt * Check TR::#1154399::OK, correct target release 3.5.3 * Check merged to previous::OK, change not open on any previous branch
Petr Horáček has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 1: Verified+1
Passed network's functional tests except unrelated ChangedOperstate errors. Passed net* unit tests except unrelated testGetBondingOptions.
Dan Kenigsberg has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 1: Code-Review+2
Ido Barkan has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 1:
(2 comments)
sorry for replying so late. I hope I didn't miss the train. I'll -1 just to not forget reading your replies.
https://gerrit.ovirt.org/#/c/38771/1/tests/functional/networkTests.py File tests/functional/networkTests.py:
Line 2187: self.vdsm_net.save_config() Line 2188: Line 2189: @cleanupNet Line 2190: @ValidateRunningAsRoot Line 2191: def test_setupNetworks_on_external_vlaned_bond(self): maybe the train left already.
but still. does this test fail of the fix is not there? Line 2192: with dummyIf(1) as (nic, ): Line 2193: with open(NET_CONF_PREF + nic, 'w') as f: Line 2194: f.write("""DEVICE=%s Line 2195: MASTER=%s
Line 2212: BOOTPROTO=static Line 2213: NM_CONTROLLED=no Line 2214: HOTPLUG=no""" % (BONDING_NAME, VLAN_ID)) Line 2215: Line 2216: rc, _, _ = execCmd([EXT_IFUP, BONDING_NAME]) might be worthwhile to check that those commands succeeded so the test won't fail on setupNetworks call and we will ask ourselves why. Line 2217: rc, _, _ = execCmd([EXT_IFUP, BONDING_NAME + '.' + VLAN_ID]) Line 2218: Line 2219: status, msg = self.vdsm_net.setupNetworks( Line 2220: {NETWORK_NAME: {'bonding': BONDING_NAME, 'bridged': True,
Ido Barkan has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 1: Code-Review-1
automation@ovirt.org has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 2:
* Update tracker::#1154399::OK * Check Bug-Url::OK * Check Public Bug::#1154399::OK, public bug * Check Product::#1154399::OK, Correct product oVirt * Check TR::#1154399::OK, correct target release 3.5.2 * Check merged to previous::OK, change not open on any previous branch
Petr Horáček has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 2: Verified+1
Passed network's functional tests except unrelated ChangedOperstate errors. Passed net* unit tests except unrelated testGetBondingOptions.
Test fails on ovirt-3.5 branch without this patch.
Petr Horáček has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 1:
(2 comments)
https://gerrit.ovirt.org/#/c/38771/1/tests/functional/networkTests.py File tests/functional/networkTests.py:
Line 2187: self.vdsm_net.save_config() Line 2188: Line 2189: @cleanupNet Line 2190: @ValidateRunningAsRoot Line 2191: def test_setupNetworks_on_external_vlaned_bond(self):
maybe the train left already.
yep, it fails without this patch. Line 2192: with dummyIf(1) as (nic, ): Line 2193: with open(NET_CONF_PREF + nic, 'w') as f: Line 2194: f.write("""DEVICE=%s Line 2195: MASTER=%s
Line 2212: BOOTPROTO=static Line 2213: NM_CONTROLLED=no Line 2214: HOTPLUG=no""" % (BONDING_NAME, VLAN_ID)) Line 2215: Line 2216: rc, _, _ = execCmd([EXT_IFUP, BONDING_NAME])
might be worthwhile to check that those commands succeeded so the test won'
Done Line 2217: rc, _, _ = execCmd([EXT_IFUP, BONDING_NAME + '.' + VLAN_ID]) Line 2218: Line 2219: status, msg = self.vdsm_net.setupNetworks( Line 2220: {NETWORK_NAME: {'bonding': BONDING_NAME, 'bridged': True,
Dan Kenigsberg has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 1:
(1 comment)
https://gerrit.ovirt.org/#/c/38771/1/tests/functional/networkTests.py File tests/functional/networkTests.py:
Line 2212: BOOTPROTO=static Line 2213: NM_CONTROLLED=no Line 2214: HOTPLUG=no""" % (BONDING_NAME, VLAN_ID)) Line 2215: Line 2216: rc, _, _ = execCmd([EXT_IFUP, BONDING_NAME])
might be worthwhile to check that those commands succeeded so the test won'
+1, but this must be done on the master branch first. Line 2217: rc, _, _ = execCmd([EXT_IFUP, BONDING_NAME + '.' + VLAN_ID]) Line 2218: Line 2219: status, msg = self.vdsm_net.setupNetworks( Line 2220: {NETWORK_NAME: {'bonding': BONDING_NAME, 'bridged': True,
Ido Barkan has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 2: Code-Review+1
automation@ovirt.org has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 3:
* Update tracker::#1154399::OK * Check Bug-Url::OK * Check Public Bug::#1154399::OK, public bug * Check Product::#1154399::OK, Correct product oVirt * Check TR::#1154399::OK, correct target release 3.5.2 * Check merged to previous::OK, change not open on any previous branch
Ido Barkan has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 3: Code-Review+1
Yaniv Bronhaim has submitted this change and it was merged.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
network: models: tag non-VDSM devices used by VDSM
When we setup network over pre-existing vlaned bond, models.py skip configuration of pre-existing devices under a vlan (because they are not to be changed) in order to not ifdown possibly used devices. Due to this behaviour we don't persist mentioned networks and therefore we are not able to restore them.
Now we check if device is a part of vdsm, if not, we configure and persist it.
Change-Id: I4d3ed2b45d618d404bb340100ca07bc57b2f1136 Signed-off-by: Petr Horáček phoracek@redhat.com Reviewed-on: https://gerrit.ovirt.org/38572 Reviewed-by: Dan Kenigsberg danken@redhat.com Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=1154399 Reviewed-on: https://gerrit.ovirt.org/38771 Reviewed-by: Ido Barkan ibarkan@redhat.com --- M tests/functional/networkTests.py M vdsm/network/models.py 2 files changed, 55 insertions(+), 2 deletions(-)
Approvals: Ido Barkan: Looks good to me, but someone else must approve Petr Horáček: Verified Dan Kenigsberg: Looks good to me, approved
automation@ovirt.org has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 4:
* Update tracker::#1154399::OK * Set MODIFIED::bug 1154399::::#1154399::::IGNORE, not all related patches are closed, check 38865
oVirt Jenkins CI Server has posted comments on this change.
Change subject: network: models: tag non-VDSM devices used by VDSM ......................................................................
Patch Set 4:
Build Successful
http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms-el6-x86_64_merged/195/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms-fc21-x86_64_merged/19/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms-el7-x86_64_merged/195/ : SUCCESS
http://jenkins.ovirt.org/job/vdsm_3.5_create-rpms-fc20-x86_64_merged/187/ : SUCCESS
vdsm-patches@lists.fedorahosted.org