Change in vdsm[master]: vdsm-tool: Change upgrade mechanism - vdsm-patches - Fedora mailing-lists

29 Apr 2014

Dima Kuznetsov has uploaded a new change for review.
Change subject: vdsm-tool: Change upgrade mechanism
......................................................................
vdsm-tool: Change upgrade mechanism
The upgrade decorator is a mechanism within vdsm-tool to allow commands
to only run once. It has flags to override, but being a function
decorator, it did not have any direct access to command line arguments
and did access via sys.argv[2:].
There is a patch that will be sent that will add flags to vdsm-tool and
will make break upgrade and its argument parsing. I'm proposing to
change the upgrade from being a decorated function, to a class deriving
from Upgrade with apply() method. This way args can be passed to the
upgrade code (via ctor), and deriving class and extend the argument
parser, this way, we'll also get a nice usage for upgrade commands:
$ vdsm-tool some-upgrade --help
Usage:
vdsm-tool [...]
Options:
[general upgrade options, e.g. --run-again]
[specific upgrade options]
Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a
Signed-off-by: Dima Kuznetsov dkuznets@redhat.com
---
M lib/vdsm/tool/unified_persistence.py
M lib/vdsm/tool/upgrade.py
M lib/vdsm/tool/upgrade_300_networks.py
3 files changed, 61 insertions(+), 114 deletions(-)
git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/93/27193/1

diff --git a/lib/vdsm/tool/unified_persistence.py b/lib/vdsm/tool/unified_persistence.py
index 3d4c8ab..10cdc41 100644
--- a/lib/vdsm/tool/unified_persistence.py
+++ b/lib/vdsm/tool/unified_persistence.py
@@ -23,7 +23,7 @@
 from ..netconfpersistence import RunningConfig
 from ..netinfo import NetInfo, getIfaceCfg, getDefaultGateway
 from . import expose
-from .upgrade import upgrade
+from .upgrade import Upgrade
UPGRADE_NAME = 'upgrade-unified-persistence'
@@ -36,7 +36,6 @@
 # bootproto = 'dhcp' if there's a lease on the NIC at the moment of upgrade
-@upgrade(UPGRADE_NAME)
 def run():
     networks, bondings = _getNetInfo()
     logging.debug('%s upgrade persisting networks %s and bondings %s',
@@ -156,6 +155,13 @@
     return config.get('vars', 'net_persistence') == 'unified'
+class UpgradeUnifiedPersistence(Upgrade):
+    def run(self):
+        if isNeeded():
+            run()
+        return 0
+
+
 @expose(UPGRADE_NAME)
 def unified_persistence(*args):
     """
@@ -164,6 +170,4 @@
     persistence model is set as unified in /usr/lib64/python2.X/site-packages/
     vdsm/config.py
     """
-    if isNeeded():
-        return run()
-    return 0
+    return UpgradeUnifiedPersistence(*args).apply()
diff --git a/lib/vdsm/tool/upgrade.py b/lib/vdsm/tool/upgrade.py
index 2e19701..822a24d 100644
--- a/lib/vdsm/tool/upgrade.py
+++ b/lib/vdsm/tool/upgrade.py
@@ -18,58 +18,31 @@
 #
import argparse
-from functools import wraps
 import logging
 import logging.config
-from logging.handlers import SysLogHandler
 import os
-import sys
-from ..constants import (P_VDSM_LOG, P_VDSM_LIB, P_VDSM_CONF, VDSM_USER,
-                         VDSM_GROUP)
+from ..constants import P_VDSM_LIB
 from ..utils import touchFile
-
-sys.path.append("/usr/share/vdsm")
-from storage.fileUtils import chown
class Upgrade(object):
     """
-        Code that needs to be run exactly once can use this class as
-        a general, simplistic mechanism. Usage:
+    This is a parent class to define upgrades, it defines its own argument
+    parser and allows any subclass extend it, the subclass is to define run()
+    method that will be called if needed upon upgrade.apply().
-        with Upgrade('someUpgrade') as upgrade:
-            if upgrade.isNeeded():
-                runSomeCode()
-                upgrade.seal()
-
-        Upgrade as a context manager automatically redirects logging
-        to the upgrade log. It also supplies the isNeeded and seal methods.
-        To call isNeeded and seal automatically use this module's upgrade
-        decorator.
+    upgrade.apply() parses the arguments, checks if upgrade should run,
+    and upon successful completion calls upgrade.seal() that marks upgrade as
+    done, to avoid executing it again.
     """
-    def __init__(self, upgradeName):
+    def __init__(self, upgradeName, *args):
         self._upgradeName = upgradeName
         self._upgradeFilePath = os.path.join(P_VDSM_LIB,
                                              'upgrade', self._upgradeName)
-        try:
-            # First load normal VDSM loggers, then the upgrade logger.
-            # This will override VDSM's root logger but will keep the other
-            # loggers intact. During an upgrade we add the update handler
-            # to all loggers.
-            logging.config.fileConfig(P_VDSM_CONF + 'logger.conf',
-                                      disable_existing_loggers=False)
-            logging.config.fileConfig(P_VDSM_CONF + 'upgrade.logger.conf',
-                                      disable_existing_loggers=False)
-            chown(
-                os.path.join(P_VDSM_LOG, 'upgrade.log'),
-                VDSM_USER,
-                VDSM_GROUP)
-        except Exception:
-            logging.getLogger('upgrade').addHandler(SysLogHandler('/dev/log'))
-            logging.exception("Could not init proper logging")
-        finally:
-            self.log = logging.getLogger('upgrade')
+        self.log = logging.getLogger('upgrade')
+        self._arg_parser = self._upgrade_argparser()
+        self._args = args
def isNeeded(self):
         return not os.path.exists(self._upgradeFilePath)
@@ -86,72 +59,32 @@
             self.log.debug("Upgrade %s successfully performed",
                            self._upgradeName)
-    def _attachUpgradeLog(self):
-        self._editOtherLoggers(logging.Logger.addHandler)
+    def _upgrade_argparser(self):
+        parser = argparse.ArgumentParser('vdsm-tool %s' % self._upgradeName)
+        parser.add_argument(
+            '--run-again',
+            dest='runAgain',
+            default=False,
+            action='store_true',
+            help='Run the upgrade again, even if it was ran before',
+        )
+        return parser
-    def _detachUpgradeLog(self):
-        self._editOtherLoggers(logging.Logger.removeHandler)
+    def run(self):
+        pass
-    def _editOtherLoggers(self, edit):
-        """
-        add/remove upgrade handler to/from all non-upgrade loggers
-        """
-        loggers = dict(logging.Logger.manager.loggerDict.items() +
-                       [('root', logging.getLogger())])
-        for name, logger in loggers.iteritems():
-            if name != 'upgrade':
-                for handler in logging.getLogger('upgrade').handlers:
-                    try:
-                        edit(logger, handler)  # Call logger.edit(handler)
-                    except TypeError:
-                        pass
-
-    def __enter__(self):
-        """
-        All logging will *also* be output to the upgrade log during
-        the scope of this Upgrade.
-        """
-        self._attachUpgradeLog()
-        return self
-
-    def __exit__(self, type, value, traceback):
-        self._detachUpgradeLog()
-
-
-def _parse_args(upgradeName):
-    parser = argparse.ArgumentParser('vdsm-tool %s' % upgradeName)
-    parser.add_argument(
-        '--run-again',
-        dest='runAgain',
-        default=False,
-        action='store_true',
-        help='Run the upgrade again, even if it was ran before',
-    )
-    return parser.parse_args(sys.argv[2:])
-
-
-def upgrade(upgradeName):
-    """
-    Used as a decorator for upgrades. Runs the upgrade with an Upgrade
-    context manager (documented above). Automatically calls isNeeded and seal
-    as needed.
-    """
-    def decorator(f):
-        @wraps(f)
-        def wrapper(*args, **kwargs):
-            with Upgrade(upgradeName) as upgrade:
-                cliArguments = _parse_args(upgradeName)
-                if cliArguments.runAgain or upgrade.isNeeded():
-                    upgrade.log.debug("Running upgrade %s", upgradeName)
-                    try:
-                        f(*args, **kwargs)
-                    except Exception:
-                        upgrade.log.exception("Failed to run %s",
-                                              upgradeName)
-                        return 1
-                    else:
-                        upgrade.seal()
-
-            return 0
-        return wrapper
-    return decorator
+    def apply(self):
+        res = self._arg_parser.parse_known_args(self._args)
+        self._parsed_ns = res[0]
+        self._parsed_args = res[1]
+        if self.isNeeded() or self._parsed_ns.runAgain:
+            self.log.debug("Running upgrade %s", self._upgradeName)
+            try:
+                self.run()
+            except Exception:
+                self.log.exception("Failed to run %s",
+                                   self._upgradeName)
+                return 1
+            else:
+                self.seal()
+        return 0
diff --git a/lib/vdsm/tool/upgrade_300_networks.py b/lib/vdsm/tool/upgrade_300_networks.py
index 95d4e17..250b04d 100644
--- a/lib/vdsm/tool/upgrade_300_networks.py
+++ b/lib/vdsm/tool/upgrade_300_networks.py
@@ -24,7 +24,7 @@
 from vdsm import netinfo
 from vdsm.constants import LEGACY_MANAGEMENT_NETWORKS
 from vdsm.tool import expose
-from vdsm.tool.upgrade import upgrade
+from vdsm.tool.upgrade import Upgrade
sys.path.append("/usr/share/vdsm")
 from network.configurators import ifcfg
@@ -42,7 +42,6 @@
     return not managementNetwork() and managementBridge()
-@upgrade(UPGRADE_NAME)
 def run(networks, bridges):
     configWriter = ifcfg.ConfigWriter()
@@ -61,6 +60,18 @@
             configWriter.removeLibvirtNetwork(network, skipBackup=True)
+class Upgrade300Networks(Upgrade):
+    def __init__(self, networks, bridges, *args):
+        Upgrade.__init__(self, *args)
+        self._networks = networks
+        self._bridges = bridges
+
+    def run(self):
+        if isNeeded(self._networks, self._bridges):
+            run(self._networks, self._bridges)
+        return 0
+
+
 @expose(UPGRADE_NAME)
 def upgrade_networks(*args):
     """
@@ -73,5 +84,4 @@
     networks = netinfo.networks()
     bridges = netinfo.bridges()
-    if isNeeded(networks, bridges):
-        run(networks, bridges)
+    return Upgrade300Networks(networks, bridges, *args).apply()
-- 
To view, visit http://gerrit.ovirt.org/27193
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I6e1d28570dedfeff9fe60624b1db72d8cadf136a
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Dima Kuznetsov dkuznets@redhat.com

    