Hello Petr Horáček, Dan Kenigsberg, Edward Haas,
I'd like you to do a code review. Please visit
https://gerrit.ovirt.org/65304
to review the following change.
Change subject: net: Consume ifcfg files that have a non vdsm standard name ......................................................................
net: Consume ifcfg files that have a non vdsm standard name
VDSM assumes that the ifcfg files which represent network devices are named in the following format: ifcfg-<dev name>
If the host is set initially by Network Manager, the names of the files do no correspond to the expected format.
This patch adjusts the ifcfg file name to the mentioned format and erases any other ifcfg files that correspond to the same device.
This is not a NM friendly patch, it assumes that NM is not active while setupNetworks is issued.
Change-Id: I0bf70ba936d5de1f17a90742644719216018f674 Bug-Url: https://bugzilla.redhat.com/1367378 Signed-off-by: Edward Haas edwardh@redhat.com Reviewed-on: https://gerrit.ovirt.org/64096 Continuous-Integration: Jenkins CI Reviewed-by: Petr Horáček phoracek@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com --- M lib/vdsm/network/configurators/ifcfg.py M tests/network/func_net_basic_test.py 2 files changed, 70 insertions(+), 0 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/04/65304/1
diff --git a/lib/vdsm/network/configurators/ifcfg.py b/lib/vdsm/network/configurators/ifcfg.py index bd3eec1..f49693a 100644 --- a/lib/vdsm/network/configurators/ifcfg.py +++ b/lib/vdsm/network/configurators/ifcfg.py @@ -106,6 +106,8 @@ self.runningConfig = None
def configureBridge(self, bridge, **opts): + if not self.owned_device(bridge.name): + self.normalize_device_filename(bridge.name) self.configApplier.addBridge(bridge, **opts) ifdown(bridge.name) if bridge.port: @@ -114,12 +116,16 @@ _ifup(bridge)
def configureVlan(self, vlan, **opts): + if not self.owned_device(vlan.name): + self.normalize_device_filename(vlan.name) self.configApplier.addVlan(vlan, **opts) vlan.device.configure(**opts) self._addSourceRoute(vlan) _ifup(vlan)
def configureBond(self, bond, **opts): + if not self.owned_device(bond.name): + self.normalize_device_filename(bond.name) self.configApplier.addBonding(bond, **opts) if not vlans.is_vlanned(bond.name): for slave in bond.slaves: @@ -143,6 +149,9 @@ nicsToSet = frozenset(nic.name for nic in bond.slaves) currentNics = frozenset(_netinfo.getNicsForBonding(bond.name)) nicsToAdd = nicsToSet - currentNics + + if not self.owned_device(bond.name): + self.normalize_device_filename(bond.name)
# Create bond configuration in case it was a non ifcfg controlled bond. # Needed to be before slave configuration for initscripts to add slave @@ -181,6 +190,8 @@ 'switch': 'legacy'})
def configureNic(self, nic, **opts): + if not self.owned_device(nic.name): + self.normalize_device_filename(nic.name) self.configApplier.addNic(nic, **opts) self._addSourceRoute(nic) if nic.bond is None: @@ -189,6 +200,8 @@ _ifup(nic)
def removeBridge(self, bridge): + if not self.owned_device(bridge.name): + self.normalize_device_filename(bridge.name) DynamicSourceRoute.addInterfaceTracking(bridge) ifdown(bridge.name) self._removeSourceRoute(bridge, StaticSourceRoute) @@ -198,6 +211,8 @@ bridge.port.remove()
def removeVlan(self, vlan): + if not self.owned_device(vlan.name): + self.normalize_device_filename(vlan.name) DynamicSourceRoute.addInterfaceTracking(vlan) ifdown(vlan.name) self._removeSourceRoute(vlan, StaticSourceRoute) @@ -219,6 +234,8 @@ DynamicSourceRoute.addInterfaceTracking(netEnt)
def removeBond(self, bonding): + if not self.owned_device(bonding.name): + self.normalize_device_filename(bonding.name) to_be_removed = self._ifaceDownAndCleanup(bonding) if to_be_removed: self.configApplier.removeBonding(bonding.name) @@ -248,6 +265,8 @@ self.configApplier.dropBridgeParameter(bonding.name)
def removeNic(self, nic, remove_even_if_used=False): + if not self.owned_device(nic.name): + self.normalize_device_filename(nic.name) to_be_removed = self._ifaceDownAndCleanup(nic, remove_even_if_used) if to_be_removed: self.configApplier.removeNic(nic.name) @@ -300,6 +319,33 @@ else: return content.startswith(CONFFILE_HEADER_SIGNATURE)
+ @staticmethod + def normalize_device_filename(device): + """ + Attempts to detect a device ifcfg file and rename it to a vdsm + supported format. + In case of multiple ifcfg files that treat the same device, all except + the first are deleted. + """ + device_files = [] + paths = glob.iglob(NET_CONF_PREF + '*') + for ifcfg_file in paths: + with open(ifcfg_file) as f: + for line in f: + if line.startswith('#'): + continue + key, value = line.rstrip().split('=', 1) + if value and value[0] == '"' and value[-1] == '"': + value = value[1:-1] + if key.upper() == 'DEVICE': + if value == device: + device_files.append(ifcfg_file) + break + if device_files: + os.rename(device_files[0], NET_CONF_PREF + device) + for filepath in device_files[1:]: + utils.rmFile(filepath) +
class ConfigWriter(object): CONFFILE_HEADER = (CONFFILE_HEADER_SIGNATURE + ' ' + diff --git a/tests/network/func_net_basic_test.py b/tests/network/func_net_basic_test.py index f937202..213187a 100644 --- a/tests/network/func_net_basic_test.py +++ b/tests/network/func_net_basic_test.py @@ -20,6 +20,8 @@
from __future__ import absolute_import
+import os + from nose.plugins.attrib import attr
from .netfunctestlib import NetFuncTestCase, NOCHK @@ -70,6 +72,28 @@ __test__ = True switch = 'legacy'
+ def test_add_net_based_on_device_with_non_standard_ifcfg_file(self): + with dummy_device() as nic: + NETCREATE = {NETWORK_NAME: {'nic': nic, 'switch': self.switch}} + NETREMOVE = {NETWORK_NAME: {'remove': True}} + with self.setupNetworks(NETCREATE, {}, NOCHK): + self.setupNetworks(NETREMOVE, {}, NOCHK) + self.assertNoNetwork(NETWORK_NAME) + + NET_CONF_DIR = '/etc/sysconfig/network-scripts/' + NET_CONF_PREF = NET_CONF_DIR + 'ifcfg-' + + nic_ifcfg_file = NET_CONF_PREF + nic + self.assertTrue(os.path.exists(nic_ifcfg_file)) + nic_ifcfg_badname_file = nic_ifcfg_file + 'tail123' + os.rename(nic_ifcfg_file, nic_ifcfg_badname_file) + + # Up until now, we have set the test setup, now start the test. + with self.setupNetworks(NETCREATE, {}, NOCHK): + self.assertNetwork(NETWORK_NAME, NETCREATE[NETWORK_NAME]) + self.assertTrue(os.path.exists(nic_ifcfg_file)) + self.assertFalse(os.path.exists(nic_ifcfg_badname_file)) +
@attr(type='functional', switch='ovs') class NetworkBasicOvsTest(NetworkBasicTemplate):
gerrit-hooks has posted comments on this change.
Change subject: net: Consume ifcfg files that have a non vdsm standard name ......................................................................
Patch Set 1:
* #64096::Update tracker: OK * #1367378::Update tracker: OK * Check Bug-Url::OK * Check Public Bug::#1367378::OK, public bug * Check Product::#64096::IGNORE, not relevant for classification: Retired * Check Product::#1367378::OK, product: vdsm * Check TM::SKIP, not in a monitored branch (ovirt-3.6 ovirt-4.0) * Check merged to previous::IGNORE, Not in stable branch (['ovirt-3.6', 'ovirt-4.0'])
Francesco Romani has posted comments on this change.
Change subject: net: Consume ifcfg files that have a non vdsm standard name ......................................................................
Patch Set 1: Code-Review+2 Verified+1
same code as ovirt-4.0, patch applied in the same order ovirt-4.0.5 branched too early
Francesco Romani has posted comments on this change.
Change subject: net: Consume ifcfg files that have a non vdsm standard name ......................................................................
Patch Set 1: Continuous-Integration+1
run 'make check' manually and succesfully.
Francesco Romani has submitted this change and it was merged.
Change subject: net: Consume ifcfg files that have a non vdsm standard name ......................................................................
net: Consume ifcfg files that have a non vdsm standard name
VDSM assumes that the ifcfg files which represent network devices are named in the following format: ifcfg-<dev name>
If the host is set initially by Network Manager, the names of the files do no correspond to the expected format.
This patch adjusts the ifcfg file name to the mentioned format and erases any other ifcfg files that correspond to the same device.
This is not a NM friendly patch, it assumes that NM is not active while setupNetworks is issued.
Change-Id: I0bf70ba936d5de1f17a90742644719216018f674 Bug-Url: https://bugzilla.redhat.com/1367378 Signed-off-by: Edward Haas edwardh@redhat.com Reviewed-on: https://gerrit.ovirt.org/64096 Continuous-Integration: Jenkins CI Reviewed-by: Petr Horáček phoracek@redhat.com Reviewed-by: Dan Kenigsberg danken@redhat.com Reviewed-on: https://gerrit.ovirt.org/65304 Reviewed-by: Francesco Romani fromani@redhat.com Tested-by: Francesco Romani fromani@redhat.com Continuous-Integration: Francesco Romani fromani@redhat.com --- M lib/vdsm/network/configurators/ifcfg.py M tests/network/func_net_basic_test.py 2 files changed, 70 insertions(+), 0 deletions(-)
Approvals: Francesco Romani: Verified; Looks good to me, approved; Passed CI tests
gerrit-hooks has posted comments on this change.
Change subject: net: Consume ifcfg files that have a non vdsm standard name ......................................................................
Patch Set 2:
* #1367378::Update tracker: OK * Set MODIFIED::bug 1367378::::#1367378::::OK, already on MODIFIED
vdsm-patches@lists.fedorahosted.org