Change in vdsm[master]: restore_nets: during rollback, ignore removal of a missing bond

danken at redhat.com danken at redhat.com
Thu Feb 26 17:08:01 UTC 2015


Dan Kenigsberg has uploaded a new change for review.

Change subject: restore_nets: during rollback, ignore removal of a missing bond
......................................................................

restore_nets: during rollback, ignore removal of a missing bond

If a bond is removed outside vdsm, attempting to restore networks to the
last known-good set of configuration currently fails.

With this patch, if vdsm is asked to remove an already-missing bond
device during an attempt to restore networking, this descripency is
ignored, and restore_nets would go on attempting to restore further
networks, hopefully restoring connectivity to the host.

The accompanying test verifies that restore_nets survive after such
removal. It also verifies that adding a network on top of an
alraedy-existing bond ends with the bonding device being consumed into
the running (and later, persistent) config.

Change-Id: I7b863ffd829026f9ea3030ac77e57cf6521fe6ba
Signed-off-by: Dan Kenigsberg <danken at redhat.com>
---
M tests/functional/networkTests.py
M vdsm/network/api.py
2 files changed, 50 insertions(+), 8 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/11/38211/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 116d91c..61ec152 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -30,7 +30,8 @@
 from vdsm.ipwrapper import (routeExists, ruleExists, addrFlush, LinkType,
                             getLinks, routeShowTable)
 from vdsm.netinfo import (bridges, operstate, getRouteDeviceTo,
-                          _get_dhclient_ifaces, BONDING_SLAVES)
+                          _get_dhclient_ifaces, BONDING_SLAVES,
+                          BONDING_MASTERS)
 from vdsm.netlink import monitor
 from vdsm import sysctl
 from vdsm.utils import CommandPath, RollbackContext, execCmd, pgrep, running
@@ -2284,3 +2285,37 @@
             self.assertEqual(status, SUCCESS, msg)
             self.assertNetworkDoesntExist(NETWORK_NAME)
             self.assertBondDoesntExist(BONDING_NAME, nics)
+
+    @cleanupNet
+    @ValidateRunningAsRoot
+    def test_setupNetworks_on_external_bond(self):
+        with dummyIf(1) as (nic, ):
+            with open(BONDING_MASTERS, 'w') as bonds:
+                bonds.write('+%s\n' % BONDING_NAME)
+            try:
+                with open(BONDING_SLAVES % BONDING_NAME, 'w') as f:
+                    f.write('+%s\n' % nic)
+                status, msg = self.vdsm_net.setupNetworks(
+                    {NETWORK_NAME:
+                        {'bonding': BONDING_NAME, 'bridged': False}},
+                    {BONDING_NAME: {'nics': [nic]}}, NOCHK)
+                self.assertEqual(status, SUCCESS, msg)
+                self.assertNetworkExists(NETWORK_NAME)
+                self.assertBondExists(BONDING_NAME, [nic])
+            finally:
+                with open(BONDING_MASTERS, 'w') as bonds:
+                    bonds.write('-%s\n' % BONDING_NAME)
+
+            self.vdsm_net.save_config()
+            self.vdsm_net.restoreNetConfig()
+
+            self.assertNetworkExists(NETWORK_NAME)
+            self.assertBondExists(BONDING_NAME, [nic])
+
+            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/api.py b/vdsm/network/api.py
index 2706cfe..ad0ee8c 100755
--- a/vdsm/network/api.py
+++ b/vdsm/network/api.py
@@ -554,7 +554,6 @@
                     "than custom properties). specified attributes: %s" % (
                         networkAttrs,))
 
-    currentBondings = netinfo.bondings()
     currentNicsSet = set(netinfo.nics())
     for bonding, bondingAttrs in bondings.iteritems():
         Bond.validateName(bonding)
@@ -562,9 +561,6 @@
             Bond.validateOptions(bonding, bondingAttrs['options'])
 
         if bondingAttrs.get('remove', False):
-            if bonding not in currentBondings:
-                raise ConfigNetworkError(ne.ERR_BAD_BONDING, "Cannot remove "
-                                         "bonding %s: Doesn't exist" % bonding)
             continue
 
         nics = bondingAttrs.get('nics', None)
@@ -576,7 +572,7 @@
                                      "Unknown nics in: %r" % list(nics))
 
 
-def _handleBondings(bondings, configurator):
+def _handleBondings(bondings, configurator, in_rollback):
     """ Add/Edit/Remove bond interface """
     logger = logging.getLogger("_handleBondings")
 
@@ -586,6 +582,16 @@
     addition = []
     for name, attrs in bondings.items():
         if 'remove' in attrs:
+            if not name in _netinfo.bondings:
+                if in_rollback:
+                    logger.error(
+                        'Cannot remove bonding %s during rollback: '
+                        'does not exist', name)
+                    continue
+                else:
+                    raise ConfigNetworkError(
+                        ne.ERR_BAD_BONDING,
+                        "Cannot remove bonding %s: does not exist" % bonding)
             bond = Bond.objectivize(name, configurator, attrs.get('options'),
                                     attrs.get('nics'), mtu=None,
                                     _netinfo=_netinfo,
@@ -824,7 +830,8 @@
     connectivity_check_networks = set()
 
     logger.debug("Applying...")
-    with ConfiguratorClass(options.get('_inRollback', False)) as configurator:
+    in_rollback = options.get('_inRollback', False)
+    with ConfiguratorClass(in_rollback) as configurator:
         # Remove edited networks and networks with 'remove' attribute
         for network, attrs in networks.items():
             if network in _netinfo.networks:
@@ -856,7 +863,7 @@
             else:
                 connectivity_check_networks.add(network)
 
-        _handleBondings(bondings, configurator)
+        _handleBondings(bondings, configurator, in_rollback)
 
         _add_missing_networks(configurator, networks, bondings, force,
                               logger, _netinfo)


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

Gerrit-MessageType: newchange
Gerrit-Change-Id: I7b863ffd829026f9ea3030ac77e57cf6521fe6ba
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>


More information about the vdsm-patches mailing list