Nitpicks present, otherwise LGTM! On Thu, Jul 24, 2014 at 2:32 AM, Yuto KAWAMURA(kawamuray) < [email protected]> wrote:
> Now LXC hypervisor requires LXC >= 1.0.0 for all commands used by it. > The LXC hypervisor now requires ... > Adding _VerifyLXCCommands() to provide checking for each commands > This patch adds ... which checks for the existence of each command and satisfactory version. > existence and version satisfaction. > > Signed-off-by: Yuto KAWAMURA(kawamuray) <[email protected]> > --- > lib/hypervisor/hv_lxc.py | 66 > ++++++++++++++++++++++++++++ > test/py/ganeti.hypervisor.hv_lxc_unittest.py | 50 +++++++++++++++++++++ > 2 files changed, 116 insertions(+) > > diff --git a/lib/hypervisor/hv_lxc.py b/lib/hypervisor/hv_lxc.py > index 3e1269e..f7c54c6 100644 > --- a/lib/hypervisor/hv_lxc.py > +++ b/lib/hypervisor/hv_lxc.py > @@ -27,6 +27,7 @@ import os > import os.path > import logging > import sys > +import re > > from ganeti import constants > from ganeti import errors # pylint: disable=W0611 > @@ -59,6 +60,15 @@ class LXCHypervisor(hv_base.BaseHypervisor): > _PROC_CGROUPS_FILE = "/proc/cgroups" > _PROC_SELF_CGROUP_FILE = "/proc/self/cgroup" > > + _LXC_MIN_VERSION_REQUIRED = "1.0.0" > + _LXC_COMMANDS_REQUIRED = [ > + "lxc-console", > + "lxc-ls", > + "lxc-start", > + "lxc-stop", > + "lxc-wait", > + ] > + > _DEVS = [ > "c 1:3", # /dev/null > "c 1:5", # /dev/zero > @@ -87,6 +97,9 @@ class LXCHypervisor(hv_base.BaseHypervisor): > constants.HV_LXC_STARTUP_WAIT: hv_base.OPT_NONNEGATIVE_INT_CHECK, > } > > + # Let beta version following micro version, but don't care about it > + _LXC_VERSION_RE = re.compile(r"^(\d+)\.(\d+)\.(\d+)") > + > def __init__(self): > hv_base.BaseHypervisor.__init__(self) > utils.EnsureDirs([ > @@ -699,6 +712,57 @@ class LXCHypervisor(hv_base.BaseHypervisor): > user=constants.SSH_CONSOLE_USER, > command=["lxc-console", "-n", > instance.name]) > > + @classmethod > + def _ParseLXCVersion(cls, version_string): > + """Return a parsed result of lxc version string. > + > + @return tuple of major, minor and micro version number > + @rtype tuple(int, int, int) > + > + """ > + match = cls._LXC_VERSION_RE.match(version_string) > + return tuple(map(int, match.groups())) if match else None > + > + @classmethod > + def _VerifyLXCCommands(cls): > + """Verify the validity of lxc command line tools. > + > + """ > Please document the return result with at least a @rtype declaration. > + version_required = cls._ParseLXCVersion(cls._LXC_MIN_VERSION_REQUIRED) > + msgs = [] > + for cmd in cls._LXC_COMMANDS_REQUIRED: > + try: > + # lxc-ls needs special checking procedure. > + # there is two different version of lxc-ls, one is written in > python > s/is/are/ > + # and the other is written in shell script. > + # we have to ensure python version of lxc-ls command is installed. > ... the python ... the lxc-ls command or lxc-ls > + if cmd == "lxc-ls": > + help_string = utils.RunCmd(["lxc-ls", "--help"]).output > + if "--running" not in help_string: > + # shell script version has no --running switch > + msgs.append("Python version of 'lxc-ls' command is required." > The Python ... > + " Maybe lxc was installed without > --enable-python") > + else: > + result = utils.RunCmd([cmd, "--version"]) > + if result.failed: > + msgs.append("Can't get version info from %s: %s" % > + (cmd, result.output)) > + else: > + version_str = result.stdout.strip() > + version = cls._ParseLXCVersion(version_str) > + if version: > + if version < version_required: > + msgs.append("LXC version >= %s is required but command %s > has" > + " version %s" % > + (cls._LXC_MIN_VERSION_REQUIRED, cmd, > version_str)) > + else: > + msgs.append("Can't parse version info from %s output: %s" % > + (cmd, version_str)) > + except errors.OpExecError: > + msgs.append("Required command %s not found" % cmd) > + > + return msgs > + > def Verify(self, hvparams=None): > """Verify the hypervisor. > > @@ -721,6 +785,8 @@ class LXCHypervisor(hv_base.BaseHypervisor): > except errors.HypervisorError, err: > msgs.append(str(err)) > > + msgs.extend(self._VerifyLXCCommands()) > + > return self._FormatVerifyResults(msgs) > > @classmethod > diff --git a/test/py/ganeti.hypervisor.hv_lxc_unittest.py b/test/py/ > ganeti.hypervisor.hv_lxc_unittest.py > index 63f9711..42b25ba 100755 > --- a/test/py/ganeti.hypervisor.hv_lxc_unittest.py > +++ b/test/py/ganeti.hypervisor.hv_lxc_unittest.py > @@ -24,6 +24,7 @@ > import unittest > > from ganeti import constants > +from ganeti import errors > from ganeti import objects > from ganeti import hypervisor > from ganeti import utils > @@ -173,5 +174,54 @@ class TestCgroupReadData(unittest.TestCase): > getval_mock.return_value = "128" > self.assertEqual(self.hv._GetCgroupMemoryLimit("instance1"), 128) > > \n > +class TestVerifyLXCCommands(unittest.TestCase): > + def setUp(self): > + runcmd_mock = mock.Mock(return_value="") > + self.RunCmdPatch = patch_object(utils, "RunCmd", runcmd_mock) > + self.RunCmdPatch.start() > + version_patch = patch_object(LXCHypervisor, > "_LXC_MIN_VERSION_REQUIRED", > + "1.2.3") > + self._LXC_MIN_VERSION_REQUIRED_Patch = version_patch > + self._LXC_MIN_VERSION_REQUIRED_Patch.start() > + self.hvc = LXCHypervisor > + > + def tearDown(self): > + self.RunCmdPatch.stop() > + self._LXC_MIN_VERSION_REQUIRED_Patch.stop() > + > + def testParseLXCVersion(self): > + self.assertEqual(self.hvc._ParseLXCVersion("1.0.0"), (1, 0, 0)) > + self.assertEqual(self.hvc._ParseLXCVersion("1.0.0.alpha1"), (1, 0, 0)) > + self.assertEqual(self.hvc._ParseLXCVersion("1.0"), None) > + self.assertEqual(self.hvc._ParseLXCVersion("1.2a.0"), None) > + > + @patch_object(LXCHypervisor, "_LXC_COMMANDS_REQUIRED", ["lxc-stop"]) > + def testCommandVersion(self): > + utils.RunCmd.return_value = RunResultOk("1.2.3\n") > + self.assertFalse(self.hvc._VerifyLXCCommands()) > + utils.RunCmd.return_value = RunResultOk("1.10.0\n") > + self.assertFalse(self.hvc._VerifyLXCCommands()) > + utils.RunCmd.return_value = RunResultOk("1.2.2\n") > + self.assertTrue(self.hvc._VerifyLXCCommands()) > + > + @patch_object(LXCHypervisor, "_LXC_COMMANDS_REQUIRED", ["lxc-stop"]) > + def testCommandVersionInvalid(self): > + utils.RunCmd.return_value = utils.RunResult(1, None, "", "", [], > None, None) > + self.assertTrue(self.hvc._VerifyLXCCommands()) > + utils.RunCmd.return_value = RunResultOk("1.2a.0\n") > + self.assertTrue(self.hvc._VerifyLXCCommands()) + > + @patch_object(LXCHypervisor, "_LXC_COMMANDS_REQUIRED", ["lxc-stop"]) > + def testCommandNotExists(self): > + utils.RunCmd.side_effect = errors.OpExecError > + self.assertTrue(self.hvc._VerifyLXCCommands()) > + > + @patch_object(LXCHypervisor, "_LXC_COMMANDS_REQUIRED", ["lxc-ls"]) > + def testVerifyLXCLs(self): > + utils.RunCmd.return_value = RunResultOk("garbage\n--running\ngarbage") > + self.assertFalse(self.hvc._VerifyLXCCommands()) + utils.RunCmd.return_value = RunResultOk("foo") > + self.assertTrue(self.hvc._VerifyLXCCommands()) + > if __name__ == "__main__": > testutils.GanetiTestProgram() > -- > 1.8.5.5 >
