Ido Barkan has uploaded a new change for review.

Change subject: net: restore-nets should support ONBOOT=no
......................................................................

net: restore-nets should support ONBOOT=no

on 3.5.x devices were persisted with ONBOOT=no. Hence, the selective
restoration would call setupNetworks with only a subset of the
networks and create a partial running config.
To fix this, we fix all ifcfg files to have ONBOOT=yes and verify
all devices are up before computing what has changed since last
call to setSafeNetworkConfig. This will make the following call to
setupNetwork to merely update running config with the 'fixed'
networks, if at all.

Change-Id: I6fa697942e6f26ee833d703437e84144c6de1334
Signed-off-by: Ido Barkan <[email protected]>
---
M tests/functional/networkTests.py
M vdsm/vdsm-restore-net-config
2 files changed, 184 insertions(+), 25 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/82/43382/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index adc7d1c..3977c9f 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -20,6 +20,7 @@
 from functools import wraps
 import os.path
 import json
+import re
 import netaddr
 
 from hookValidation import ValidatesHook
@@ -52,6 +53,7 @@
 
 import caps
 from network import errors
+from network.configurators.ifcfg import stop_devices
 
 
 NETWORK_NAME = 'test-network'
@@ -1527,20 +1529,13 @@
                  'netmask': IP_MASK, 'bonding': BOND_MISSING},
         }
 
-        def _assert_all_nets_exist(in_running_conf=True):
+        def _assert_all_nets_exist():
             self.vdsm_net.refreshNetinfo()
-            self.assertNetworkExists(
-                NET_UNCHANGED, assert_in_running_conf=in_running_conf)
-            self.assertNetworkExists(
-                NET_CHANGED, assert_in_running_conf=in_running_conf)
-            self.assertNetworkExists(
-                NET_MISSING, assert_in_running_conf=in_running_conf)
-            self.assertBondExists(
-                BOND_UNCHANGED, [nic_a],
-                assert_in_running_conf=in_running_conf)
-            self.assertBondExists(
-                BOND_MISSING, [nic_b],
-                assert_in_running_conf=in_running_conf)
+            self.assertNetworkExists(NET_UNCHANGED)
+            self.assertNetworkExists(NET_CHANGED)
+            self.assertNetworkExists(NET_MISSING)
+            self.assertBondExists(BOND_UNCHANGED, [nic_a])
+            self.assertBondExists(BOND_MISSING, [nic_b])
 
         with dummyIf(2) as nics:
             nic_a, nic_b = nics
@@ -1567,7 +1562,7 @@
                 with nonChangingOperstate(NET_UNCHANGED):
                     self.vdsm_net.restoreNetConfig()
 
-                _assert_all_nets_exist(False)
+                _assert_all_nets_exist()
             finally:
                 self.setupNetworks(
                     {NET_UNCHANGED: {'remove': True},
@@ -1584,6 +1579,117 @@
                 self.assertBondDoesntExist(BOND_UNCHANGED, [nic_a])
 
     @cleanupNet
+    @RequireDummyMod
+    @ValidateRunningAsRoot
+    def testSelectiveRestoreDuringUpgrade(self):
+        if vdsm.config.config.get('vars', 'net_persistence') == 'ifcfg':
+            raise SkipTest(
+                "with ifcfg persistence, vdsm-restore-net-config selective"
+                "restoration is not supported")
+
+        BOND_UNCHANGED = 'bond100'
+        BOND_CHANGED = 'bond101'
+        IP_MGMT = '240.0.0.100'
+        NET_MGMT = NETWORK_NAME + '100'
+        NET_UNCHANGED = NETWORK_NAME + '101'
+        NET_CHANGED = NETWORK_NAME + '102'
+        NET_ADDITIONAL = NETWORK_NAME + '103'
+        IP_ADDR_UNCHANGED = '240.0.0.101'
+        IP_ADDR_CHANGED = '204.0.0.102'
+        IP_ADDR_ADDITIONAL = '204.0.0.102'
+        nets = {
+            NET_MGMT:
+                {'bootproto': 'none', 'ipaddr': IP_MGMT,
+                 'netmask': IP_MASK, 'defaultRoute': True},
+            NET_UNCHANGED:
+                {'bootproto': 'none',
+                 'ipaddr': IP_ADDR_UNCHANGED,
+                 'netmask': IP_MASK, 'bonding': BOND_UNCHANGED,
+                 'defaultRoute': False},
+            NET_CHANGED:
+                {'bootproto': 'none',
+                 'ipaddr': IP_ADDR_CHANGED,
+                 'netmask': IP_MASK, 'bonding': BOND_CHANGED},
+        }
+        net_additional_attrs = {
+            'bootproto': 'none', 'ipaddr': IP_ADDR_ADDITIONAL,
+            'netmask': IP_MASK}
+
+        def _assert_all_nets_exist():
+            self.vdsm_net.refreshNetinfo()
+            self.assertNetworkExists(NET_MGMT)
+            self.assertNetworkExists(NET_UNCHANGED)
+            self.assertNetworkExists(NET_CHANGED)
+            self.assertBondExists(BOND_UNCHANGED, [nic_b])
+            self.assertBondExists(BOND_CHANGED, [nic_c], options='mode=4')
+
+        def _simulate_boot_after_upgrade():
+            # all non-management devices are down and have ONBOOT=no from older
+            # vdsm versions.
+            device_names = (NET_UNCHANGED, BOND_UNCHANGED, nic_b, NET_CHANGED,
+                            BOND_CHANGED, nic_c)
+            stop_devices((NET_CONF_PREF + name for name in device_names))
+            for dev in device_names:
+                with open(NET_CONF_PREF + dev) as f:
+                    content = f.read()
+                # also test the case that a bond is different from it's
+                # backup
+                content = re.sub('ONBOOT=yes', 'ONBOOT=no', content)
+                if dev == BOND_CHANGED:
+                    content = re.sub('mode=4', 'mode=0', content)
+                with open(NET_CONF_PREF + dev, 'w') as f:
+                    f.write(content)
+            # we don't have running config during boot
+            RunningConfig().delete()
+
+        with dummyIf(4) as nics:
+            nic_a, nic_b, nic_c, nic_d = nics
+            nets[NET_MGMT]['nic'] = nic_a
+            net_additional_attrs['nic'] = nic_d
+            bonds = {BOND_UNCHANGED: {'nics': [nic_b]},
+                     BOND_CHANGED: {'nics': [nic_c], 'options': "mode=4"}
+                     }
+            status, msg = self.setupNetworks(nets, bonds, NOCHK)
+            self.assertEquals(status, SUCCESS, msg)
+            _assert_all_nets_exist()
+            try:
+                self.vdsm_net.save_config()
+
+                _simulate_boot_after_upgrade()
+
+                with nonChangingOperstate(NET_MGMT):
+                    self.vdsm_net.restoreNetConfig()
+
+                status, msg = self.setupNetworks(
+                    {NET_ADDITIONAL: net_additional_attrs}, {}, NOCHK)
+                self.assertEquals(status, SUCCESS, msg)
+                _assert_all_nets_exist()
+                # verify that the running config now has all desired networks
+                self.assertEqual({NET_MGMT, NET_CHANGED, NET_UNCHANGED,
+                                  NET_ADDITIONAL},
+                                 set(self.vdsm_net.config.networks.keys()))
+                self.assertEqual({BOND_CHANGED, BOND_UNCHANGED},
+                                 set(self.vdsm_net.config.bonds.keys()))
+
+            finally:
+                status, msg = self.setupNetworks(
+                    {NET_MGMT: {'remove': True},
+                     NET_UNCHANGED: {'remove': True},
+                     NET_CHANGED: {'remove': True},
+                     NET_ADDITIONAL: {'remove': True}},
+                    {BOND_CHANGED: {'remove': True},
+                     BOND_UNCHANGED: {'remove': True}},
+                    NOCHK)
+                self.assertEquals(status, SUCCESS, msg)
+                self.vdsm_net.save_config()
+                self.assertNetworkDoesntExist(NET_MGMT)
+                self.assertNetworkDoesntExist(NET_UNCHANGED)
+                self.assertNetworkDoesntExist(NET_CHANGED)
+                self.assertNetworkDoesntExist(NET_ADDITIONAL)
+                self.assertBondDoesntExist(BOND_CHANGED, [nic_b])
+                self.assertBondDoesntExist(BOND_UNCHANGED, [nic_a])
+
+    @cleanupNet
     @permutations([[True], [False]])
     @RequireDummyMod
     @ValidateRunningAsRoot
diff --git a/vdsm/vdsm-restore-net-config b/vdsm/vdsm-restore-net-config
index 0065f5d..df37419 100755
--- a/vdsm/vdsm-restore-net-config
+++ b/vdsm/vdsm-restore-net-config
@@ -19,9 +19,11 @@
 # Refer to the README and COPYING files for full details of the license
 #
 import argparse
+import glob
 import logging
 import logging.config
 import os
+import re
 import time
 import errno
 
@@ -73,6 +75,11 @@
 
     persistent_config = PersistentConfig()
     available_config = _filter_available(persistent_config)
+
+    _verify_all_devices_are_up(list(_owned_ifcfg_files()))
+
+    _update_running_config(persistent_config)
+
     _wait_for_for_all_devices_up(
         available_config.networks.keys() + available_config.bonds.keys())
     changed_config = _filter_changed_nets_bonds(available_config)
@@ -83,6 +90,49 @@
         logging.debug('Calling setupNetworks with networks (%s) and bond 
(%s).',
                       nets, bonds)
         setupNetworks(nets, bonds, connectivityCheck=False, _inRollback=True)
+
+
+def _verify_all_devices_are_up(owned_ifcfg_files):
+    """REQUIRED_FOR upgrade from 4.16<=vdsm<=4.16.20
+    Were ifcfg files were created with ONBOOT=no.
+    """
+    for ifcfg_file in owned_ifcfg_files:
+        _upgrade_onboot(ifcfg_file)
+    ifcfg.start_devices(owned_ifcfg_files)
+
+
+def _upgrade_onboot(ifcfg_file):
+    with open(ifcfg_file) as f:
+        old_content = f.read()
+    new_content = re.sub('^ONBOOT=no$', 'ONBOOT=yes', old_content,
+                         flags=re.MULTILINE)
+    if new_content != old_content:
+        logging.debug("updating %s to ONBOOT=yes", ifcfg_file)
+        with open(ifcfg_file, 'w') as f:
+            f.write(new_content)
+
+
+def _update_running_config(persistent_config):
+    """We must recreate RunningConfig so that following setSafeNetworkConfig
+    will persist a valid configuration.
+    """
+    running_config = RunningConfig()
+    for net, net_attr in persistent_config.networks.iteritems():
+        running_config.setNetwork(net, net_attr)
+    for bonding, bonding_attr in persistent_config.bonds.iteritems():
+        running_config.setBonding(bonding, bonding_attr)
+    running_config.save()
+
+
+def _owned_ifcfg_files():
+    for fpath in glob.iglob(netinfo.NET_CONF_DIR + '/*'):
+        if not os.path.isfile(fpath):
+            continue
+
+        with open(fpath) as f:
+            content = f.read()
+        if _owned_ifcfg_content(content):
+            yield fpath
 
 
 def _convert_to_blocking_dhcp(networks):
@@ -199,26 +249,29 @@
     def oper_up(link):
         return bool(link.flags & 1 << 6)
 
-    return set([l.name for l in ipwrapper.getLinks() if
-                l.name in links and
-                _owned(l.name) and
-                _onboot(l.name) and
-                not oper_up(l)])
+    return set(l.name for l in ipwrapper.getLinks() if
+               l.name in links and
+               _owned_ifcfg(l.name) and
+               _onboot_ifcfg(l.name) and
+               not oper_up(l))
 
 
-def _owned(link_name):
-    predicate = lambda content: (
-        content.startswith('# Generated by VDSM version') or
-        content.startswith('# automatically generated by vdsm'))
-    return _ifcfg_predicate(link_name, predicate)
+def _owned_ifcfg(link_name):
+    return _ifcfg_predicate(link_name, _owned_ifcfg_content)
 
 
-def _onboot(link_name):
+def _onboot_ifcfg(link_name):
     predicate = lambda content: any(
         line == 'ONBOOT=yes' for line in content.splitlines())
     return _ifcfg_predicate(link_name, predicate)
 
 
+def _owned_ifcfg_content(content):
+    return content.startswith(
+        '# Generated by VDSM version') or content.startswith(
+        '# automatically generated by vdsm')
+
+
 def _ifcfg_predicate(link_name, predicate):
     try:
         with open(netinfo.NET_CONF_PREF + link_name) as conf:


-- 
To view, visit https://gerrit.ovirt.org/43382
To unsubscribe, visit https://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6fa697942e6f26ee833d703437e84144c6de1334
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.5
Gerrit-Owner: Ido Barkan <[email protected]>
_______________________________________________
vdsm-patches mailing list
[email protected]
https://lists.fedorahosted.org/mailman/listinfo/vdsm-patches

Reply via email to