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