Change in vdsm[ovirt-3.3]: vdsm: allow hooks to pass down dictionaries in json format

danken at redhat.com danken at redhat.com
Fri Jan 24 10:25:48 UTC 2014


Hello Miguel Angel Ajo Pelayo,

I'd like you to do a code review.  Please visit

    http://gerrit.ovirt.org/23660

to review the following change.

Change subject: vdsm: allow hooks to pass down dictionaries in json format
......................................................................

vdsm: allow hooks to pass down dictionaries in json format

This will allow to pass, and recover network settings in the
*_network_setup hook as json dictionaries using the same
method that is used for XML. In this case the environment
variable _hook_json is used.

before_network_setup hook exposes the requested network
configuration that can be patched by the hook.
after_network_setup exposes the final system network
configuration.

Change-Id: Ie07c511e9740fd19a1c27baf87e91f9a427d0dcd
Signed-off-by: Miguel Angel Ajo <miguelangel at ajo.es>
Reviewed-on: http://gerrit.ovirt.org/20330
Reviewed-by: Dan Kenigsberg <danken at redhat.com>
Tested-by: Dan Kenigsberg <danken at redhat.com>
Tested-by: Miguel Angel Ajo Pelayo <majopela at redhat.com>
---
M tests/functional/networkTests.py
M vdsm/configNetwork.py
M vdsm/hooks.py
M vdsm/vdsmd.8.in
4 files changed, 117 insertions(+), 20 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/vdsm refs/changes/60/23660/1

diff --git a/tests/functional/networkTests.py b/tests/functional/networkTests.py
index 02e9144..0ad7c02 100644
--- a/tests/functional/networkTests.py
+++ b/tests/functional/networkTests.py
@@ -19,6 +19,7 @@
 from contextlib import contextmanager
 from threading import Thread
 import os.path
+import json
 import time
 
 import neterrors
@@ -1435,7 +1436,27 @@
     @cleanupNet
     @RequireDummyMod
     @ValidateRunningAsRoot
-    @ValidatesHook('before_network_setup', 'testBeforeNetworkSetup.sh')
+    @ValidatesHook('before_network_setup', 'testBeforeNetworkSetup.py', True,
+                   "#!/usr/bin/env python\n"
+                   "import json\n"
+                   "import os\n"
+                   "\n"
+                   "# get the filename where settings are stored\n"
+                   "hook_data = os.environ['_hook_json']\n"
+                   "network_config = None\n"
+                   "with open(hook_data, 'r') as network_config_file:\n"
+                   "    network_config = json.load(network_config_file)\n"
+                   "\n"
+                   "# setup an output config file\n"
+                   "cookie_file = open('%(cookiefile)s','w')\n"
+                   "cookie_file.write(str(network_config) + '\\n')\n"
+                   "network_config['request']['networks']['" + NETWORK_NAME +
+                   "']['bridged'] = True\n"
+                   "\n"
+                   "# output modified config back to the hook_data file\n"
+                   "with open(hook_data, 'w') as network_config_file:\n"
+                   "    json.dump(network_config, network_config_file)\n"
+                   )
     def testBeforeNetworkSetupHook(self, hook_cookiefile):
         with dummyIf(1) as nics:
             nic, = nics
@@ -1443,6 +1464,7 @@
                                        'bootproto': 'none'}}
             with self.vdsm_net.pinger():
                 self.vdsm_net.setupNetworks(networks, {}, {})
+                self.assertNetworkExists(NETWORK_NAME, bridged=True)
 
                 self.assertTrue(os.path.isfile(hook_cookiefile))
 
@@ -1453,7 +1475,10 @@
     @cleanupNet
     @RequireDummyMod
     @ValidateRunningAsRoot
-    @ValidatesHook('after_network_setup', 'testAfterNetworkSetup.sh')
+    @ValidatesHook('after_network_setup', 'testAfterNetworkSetup.sh', True,
+                   "#!/bin/sh\n"
+                   "cat $_hook_json > %(cookiefile)s\n"
+                   )
     def testAfterNetworkSetupHook(self, hook_cookiefile):
         with dummyIf(1) as nics:
             nic, = nics
@@ -1464,6 +1489,20 @@
 
                 self.assertTrue(os.path.isfile(hook_cookiefile))
 
+                with open(hook_cookiefile, 'r') as cookie_file:
+                    network_config = json.load(cookie_file)
+                    self.assertIn('networks', network_config['request'])
+                    self.assertIn('bondings', network_config['request'])
+                    self.assertIn(NETWORK_NAME,
+                                  network_config['request']['networks'])
+
+                    # when unified persistence is enabled we also provide
+                    # the current configuration to the hook
+
+                    if 'current' in network_config:
+                        self.assertTrue(NETWORK_NAME in
+                                        network_config['current']['networks'])
+
                 delete_networks = {NETWORK_NAME: {'remove': True}}
                 self.vdsm_net.setupNetworks(delete_networks,
                                             {}, {})
diff --git a/vdsm/configNetwork.py b/vdsm/configNetwork.py
index 8175742..45b6b64 100755
--- a/vdsm/configNetwork.py
+++ b/vdsm/configNetwork.py
@@ -451,6 +451,15 @@
     return bond
 
 
+def _buildSetupHookDict(req_networks, req_bondings, req_options):
+
+    hook_dict = {'request': {'networks': dict(req_networks),
+                             'bondings': dict(req_bondings),
+                             'options': dict(req_options)}}
+
+    return hook_dict
+
+
 def setupNetworks(networks, bondings, **options):
     """Add/Edit/Remove configuration for networks and bondings.
 
@@ -501,7 +510,14 @@
         logging.debug("Validating configuration")
         _validateNetworkSetup(dict(networks), dict(bondings))
 
-    hooks.before_network_setup()
+    results = hooks.before_network_setup(_buildSetupHookDict(networks,
+                                                             bondings,
+                                                             options))
+
+    # gather any changes that could have been done by the hook scripts
+    networks = results['request']['networks']
+    bondings = results['request']['bondings']
+    options = results['request']['options']
 
     logger.debug("Applying...")
     with Ifcfg() as configurator:
@@ -563,7 +579,7 @@
                 raise ConfigNetworkError(ne.ERR_LOST_CONNECTION,
                                          'connectivity check failed')
 
-    hooks.after_network_setup()
+    hooks.after_network_setup(_buildSetupHookDict(networks, bondings, options))
 
 
 def _vlanToInternalRepresentation(vlan):
diff --git a/vdsm/hooks.py b/vdsm/hooks.py
index ccc21a2..464e46a 100644
--- a/vdsm/hooks.py
+++ b/vdsm/hooks.py
@@ -20,13 +20,14 @@
 
 from vdsm import utils
 import glob
+import hashlib
+import itertools
+import json
+import logging
 import os
 import os.path
 import sys
 import tempfile
-import logging
-import itertools
-import hashlib
 
 from vdsm.constants import P_VDSM_HOOKS, P_VDSM
 
@@ -45,18 +46,26 @@
     return [s for s in glob.glob(path + '/*')
             if os.access(s, os.X_OK)]
 
+_DOMXML_HOOK = 1
+_JSON_HOOK = 2
 
-def _runHooksDir(domxml, dir, vmconf={}, raiseError=True, params={}):
+
+def _runHooksDir(data, dir, vmconf={}, raiseError=True, params={},
+                 hookType=_DOMXML_HOOK):
+
     scripts = _scriptsPerDir(dir)
     scripts.sort()
 
     if not scripts:
-        return domxml
+        return data
 
-    xmlfd, xmlname = tempfile.mkstemp()
+    data_fd, data_filename = tempfile.mkstemp()
     try:
-        os.write(xmlfd, domxml or '')
-        os.close(xmlfd)
+        if hookType == _DOMXML_HOOK:
+            os.write(data_fd, data or '')
+        elif hookType == _JSON_HOOK:
+            os.write(data_fd, json.dumps(data))
+        os.close(data_fd)
 
         scriptenv = os.environ.copy()
 
@@ -79,7 +88,10 @@
             scriptenv['vmId'] = vmconf.get('vmId')
         ppath = scriptenv.get('PYTHONPATH', '')
         scriptenv['PYTHONPATH'] = ':'.join(ppath.split(':') + [P_VDSM])
-        scriptenv['_hook_domxml'] = xmlname
+        if hookType == _DOMXML_HOOK:
+            scriptenv['_hook_domxml'] = data_filename
+        elif hookType == _JSON_HOOK:
+            scriptenv['_hook_json'] = data_filename
 
         errorSeen = False
         for s in scripts:
@@ -97,10 +109,13 @@
         if errorSeen and raiseError:
             raise HookError()
 
-        finalxml = file(xmlname).read()
+        final_data = file(data_filename).read()
     finally:
-        os.unlink(xmlname)
-    return finalxml
+        os.unlink(data_filename)
+    if hookType == _DOMXML_HOOK:
+        return final_data
+    elif hookType == _JSON_HOOK:
+        return json.loads(final_data)
 
 
 def before_device_create(devicexml, vmconf={}, customProperties={}):
@@ -304,12 +319,14 @@
     return _runHooksDir(None, 'after_vdsm_stop', raiseError=False)
 
 
-def before_network_setup():
-    return _runHooksDir(None, 'before_network_setup')
+def before_network_setup(network_config_dict):
+    return _runHooksDir(network_config_dict, 'before_network_setup',
+                        hookType=_JSON_HOOK)
 
 
-def after_network_setup():
-    return _runHooksDir(None, 'after_network_setup', raiseError=False)
+def after_network_setup(network_config_dict):
+    return _runHooksDir(network_config_dict, 'after_network_setup',
+                        raiseError=False, hookType=_JSON_HOOK)
 
 
 def _getScriptInfo(path):
diff --git a/vdsm/vdsmd.8.in b/vdsm/vdsmd.8.in
index f872bb0..8b514a9 100644
--- a/vdsm/vdsmd.8.in
+++ b/vdsm/vdsmd.8.in
@@ -74,6 +74,31 @@
 The uuid of the virtual machine may be deduced from that xml, but it is also
 available as the environment variable
 .B vmId.
+
+The before_network_setup and after_network_setup hooks do also include an
+extra environment variable
+.B _hook_json
+which holds a pointer to a file with the network parameters that vdsm is
+setting up (
+.B request
+)
+, the request may be modified by the before_network_setup hook as thus affect
+the operation ultimately taken place by Vdsm.
+
+The JSON format of this file has one section: request, this section
+contains networks, bondings and options, those parameters are specified
+in the setupNetworks VDSM API call.
+
+.nf
+{"request":
+    {
+    "networks": {"virtnet": {"bonding" : "bond0", "bridged": true, "vlan":27}},
+    "bondings": {"bond0": {"nics":["eth1","eth2"]}},
+    "options":  {"conectivityCheck":false}
+    }
+ }
+.fi
+
 Hooks that handle NIC hotplug, hotunplug and update device
 have the _hook_domxml variable but it contains the representation of the NIC
 rather than the VM. Hotplug/hotunplug disk hooks also have the _hook_dom_xml variable,


-- 
To view, visit http://gerrit.ovirt.org/23660
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ie07c511e9740fd19a1c27baf87e91f9a427d0dcd
Gerrit-PatchSet: 1
Gerrit-Project: vdsm
Gerrit-Branch: ovirt-3.3
Gerrit-Owner: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Miguel Angel Ajo Pelayo <majopela at redhat.com>


More information about the vdsm-patches mailing list