Change in vdsm[master]: Initial commit for vdsm-tool testing infrastructure

danken at redhat.com danken at redhat.com
Mon Mar 17 17:32:52 UTC 2014


Dan Kenigsberg has posted comments on this change.

Change subject: Initial commit for vdsm-tool testing infrastructure
......................................................................


Patch Set 10: Code-Review-1

(7 comments)

http://gerrit.ovirt.org/#/c/25263/10/.gitignore
File .gitignore:

Line 33: init/upstart/vdsmd.upstart
Line 34: init/vdsmd_init_common.sh
Line 35: lib/vdsm/config.py
Line 36: lib/vdsm/constants.py
Line 37: lib/vdsm/tool/tests/logrotate-libvirtd
why are you creating these files in tree? you should run the test in a temp dir and clean after yourself.
Line 38: lib/vdsm/tool/tests/libvirtd.conf
Line 39: lib/vdsm/tool/tests/libvirtd
Line 40: lib/vdsm/tool/tests/qemu.conf
Line 41: lib/vdsm/tool/tests/qemu-sanlock.conf


http://gerrit.ovirt.org/#/c/25263/10/lib/vdsm/tool/configurator.py
File lib/vdsm/tool/configurator.py:

Line 114: 
Line 115:     SANLOCK_GROUPS = (QEMU_PROCESS_GROUP, VDSM_GROUP)
Line 116: 
Line 117:     def __init__(self, testenv=False):
Line 118:         self.testenv = testenv
it's very to have testenv as a boolean in one case, and as an env dictionary on another.
Line 119:         super(SanlockModuleConfigure, self).__init__()
Line 120: 
Line 121:     def getName(self):
Line 122:         return 'sanlock'


Line 131:         if not self.testenv and os.getuid() != 0:
Line 132:             raise UserWarning("Must run as root")
Line 133: 
Line 134:         rc, out, err = utils.execCmd(
Line 135:             [
why not tupple?
Line 136:                 '/usr/sbin/usermod',
Line 137:                 '-a',
Line 138:                 '-G',
Line 139:                 ','.join(self.SANLOCK_GROUPS),


Line 139:                 ','.join(self.SANLOCK_GROUPS),
Line 140:                 'sanlock'
Line 141:             ],
Line 142:             raw=True,
Line 143:             sudo=True if self.testenv else False,
Why run this configurator at all during tests? I find it a bit rude.
Line 144:         )
Line 145:         sys.stdout.write(out)
Line 146:         sys.stderr.write(err)
Line 147:         if rc != 0:


http://gerrit.ovirt.org/#/c/25263/10/lib/vdsm/tool/libvirt_configure.sh.in
File lib/vdsm/tool/libvirt_configure.sh.in:

Line 295:     do
Line 296:         echo "${end_conf_section}-${BY_VDSM_VERS}" >> "${arg}"
Line 297:     done
Line 298: 
Line 299:     if [ "${TESTING_ENV}" = '' ]; then
we use "-z" elsewhere in this file. let's conform.
Line 300:         local lnetwork=/etc/libvirt/qemu/networks/autostart/default.xml
Line 301:         rm -f "${lnetwork}"
Line 302:     fi
Line 303: 


Line 301:         rm -f "${lnetwork}"
Line 302:     fi
Line 303: 
Line 304:     if [ "${LIBVIRT_LOGROTATE}" = '' ]; then
Line 305:         local llogr=/etc/logrotate.d/libvirtd
this is equivalent to

 local llogr="${LIBVIRT_LOGROTATE:=/etc/logrotate.d/libvirtd}"
Line 306:     else
Line 307:         local llogr="${LIBVIRT_LOGROTATE}"
Line 308:     fi
Line 309: 


Line 338:         @CHMOD_PATH@ "${oldmod}" "${llogr}"
Line 339:     fi
Line 340: 
Line 341:     if [ "${TESTING_ENV}" = '' ]; then
Line 342:     restorecon "${llogr}"
indent.

But how about defining

skipontest() {
  if [ -z "${TESTING_ENV}" ] ; then
    "$@"
  else:
    echo skipping "$@"
}

It should be added to ovirt_store_config (in case the test happens to be run on ovirt-node).
Line 343:     fi
Line 344: 
Line 345:     # persist cmd is ovirt_store_config on ovirt-functions
Line 346:     if isOvirtNode; then


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iae0fec9b2057c5ba932f38ed5d34d654486043ef
Gerrit-PatchSet: 10
Gerrit-Project: vdsm
Gerrit-Branch: master
Gerrit-Owner: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: Antoni Segura Puimedon <asegurap at redhat.com>
Gerrit-Reviewer: Dan Kenigsberg <danken at redhat.com>
Gerrit-Reviewer: Yaniv Bronhaim <ybronhei at redhat.com>
Gerrit-Reviewer: automation at ovirt.org
Gerrit-Reviewer: mooli tayer <mtayer at redhat.com>
Gerrit-Reviewer: oVirt Jenkins CI Server
Gerrit-HasComments: Yes


More information about the vdsm-patches mailing list