LGTM, thanks On Mon, Oct 6, 2014 at 8:09 PM, Yuto KAWAMURA(kawamuray) < [email protected]> wrote:
> This patch adds the class LXCVersion and the function _GetLXCVersion > which can be used to obtain version information from an LXC command. > _VerifyLXCCommands has been changed to use this function instead of > calling commands directly, and adjust unit tests for changes made. > > Signed-off-by: Yuto KAWAMURA(kawamuray) <[email protected]> > --- > lib/hypervisor/hv_lxc.py | 89 > ++++++++++++++++++++-------- > test/py/ganeti.hypervisor.hv_lxc_unittest.py | 18 +++--- > 2 files changed, 74 insertions(+), 33 deletions(-) > > diff --git a/lib/hypervisor/hv_lxc.py b/lib/hypervisor/hv_lxc.py > index 4981c9e..0c9bc6d 100644 > --- a/lib/hypervisor/hv_lxc.py > +++ b/lib/hypervisor/hv_lxc.py > @@ -62,6 +62,40 @@ def _CreateBlankFile(path, mode): > raise HypervisorError("Failed to create file %s: %s" % (path, err)) > > > +class LXCVersion(tuple): # pylint: disable=R0924 > + """LXC version class. > + > + """ > + # Let beta version following micro version, but don't care about it > + _VERSION_RE = re.compile(r"^(\d+)\.(\d+)\.(\d+)") > + > + @classmethod > + def _Parse(cls, version_string): > + """Parse a passed string as an LXC version string. > + > + @param version_string: a valid LXC version string > + @type version_string: string > + @raise ValueError: if version_string is an invalid LXC version string > + @rtype tuple(int, int, int) > + @return (major_num, minor_num, micro_num) > + > + """ > + match = cls._VERSION_RE.match(version_string) > + if match: > + return tuple(map(int, match.groups())) > + else: > + raise ValueError("'%s' is not a valid LXC version string" % > + version_string) > + > + def __new__(cls, version_string): > + version = super(LXCVersion, cls).__new__(cls, > cls._Parse(version_string)) > + version.original_string = version_string > + return version > + > + def __str__(self): > + return self.original_string > + > + > class LXCHypervisor(hv_base.BaseHypervisor): > """LXC-based virtualization. > > @@ -93,7 +127,7 @@ 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_MIN_VERSION_REQUIRED = LXCVersion("1.0.0") > _LXC_COMMANDS_REQUIRED = [ > "lxc-console", > "lxc-ls", > @@ -115,8 +149,6 @@ 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+)") > _REBOOT_TIMEOUT = 120 # secs > _REQUIRED_CGROUP_SUBSYSTEMS = [ > "cpuset", > @@ -834,15 +866,28 @@ class LXCHypervisor(hv_base.BaseHypervisor): > command=["lxc-console", "-n", > instance.name]) > > @classmethod > - def _ParseLXCVersion(cls, version_string): > - """Return a parsed result of lxc version string. > + def _GetLXCVersionFromCmd(cls, from_cmd): > + """Return the LXC version currently used in the system. > + > + Version information will be retrieved by command specified by > from_cmd. > > - @return: tuple of major, minor and micro version number > - @rtype: tuple(int, int, int) > + @param from_cmd: the lxc command used to retrieve a version > information > + @type from_cmd: string > + @rtype L{LXCVersion} > + @return a version object which represents the version retrieved from > the > + command > > """ > - match = cls._LXC_VERSION_RE.match(version_string) > - return tuple(map(int, match.groups())) if match else None > + result = utils.RunCmd([from_cmd, "--version"]) > + if result.failed: > + raise HypervisorError("Failed to get version info from command %s: > %s" % > + (from_cmd, result.output)) > + > + try: > + return LXCVersion(result.stdout.strip()) > + except ValueError, err: > + raise HypervisorError("Can't parse LXC version from %s: %s" % > + (from_cmd, err)) > > @classmethod > def _VerifyLXCCommands(cls): > @@ -853,7 +898,6 @@ class LXCHypervisor(hv_base.BaseHypervisor): > there is no problem. > > """ > - version_required = cls._ParseLXCVersion(cls._LXC_MIN_VERSION_REQUIRED) > msgs = [] > for cmd in cls._LXC_COMMANDS_REQUIRED: > try: > @@ -868,21 +912,16 @@ class LXCHypervisor(hv_base.BaseHypervisor): > msgs.append("The python version of 'lxc-ls' is required." > " 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)) > + try: > + version = cls._GetLXCVersionFromCmd(cmd) > + except HypervisorError, err: > + msgs.append(str(err)) > + continue > + > + if version < cls._LXC_MIN_VERSION_REQUIRED: > + msgs.append("LXC version >= %s is required but command %s has" > + " version %s" % > + (cls._LXC_MIN_VERSION_REQUIRED, cmd, version)) > except errors.OpExecError: > msgs.append("Required command %s not found" % cmd) > > diff --git a/test/py/ganeti.hypervisor.hv_lxc_unittest.py b/test/py/ > ganeti.hypervisor.hv_lxc_unittest.py > index 8cd08c7..208ddcf 100755 > --- a/test/py/ganeti.hypervisor.hv_lxc_unittest.py > +++ b/test/py/ganeti.hypervisor.hv_lxc_unittest.py > @@ -40,7 +40,7 @@ from ganeti import utils > > from ganeti.hypervisor import hv_base > from ganeti.hypervisor import hv_lxc > -from ganeti.hypervisor.hv_lxc import LXCHypervisor > +from ganeti.hypervisor.hv_lxc import LXCHypervisor, LXCVersion > > import mock > import os > @@ -67,6 +67,14 @@ def RunResultOk(stdout): > return utils.RunResult(0, None, stdout, "", [], None, None) > > > +class TestLXCVersion(unittest.TestCase): > + def testParseLXCVersion(self): > + self.assertEqual(LXCVersion("1.0.0"), (1, 0, 0)) > + self.assertEqual(LXCVersion("1.0.0.alpha1"), (1, 0, 0)) > + self.assertRaises(ValueError, LXCVersion, "1.0") > + self.assertRaises(ValueError, LXCVersion, "1.2a.0") > + > + > class LXCHypervisorTestCase(unittest.TestCase): > """Used to test classes instantiating the LXC hypervisor class. > > @@ -208,7 +216,7 @@ class TestVerifyLXCCommands(unittest.TestCase): > self.RunCmdPatch = patch_object(utils, "RunCmd", runcmd_mock) > self.RunCmdPatch.start() > version_patch = patch_object(LXCHypervisor, > "_LXC_MIN_VERSION_REQUIRED", > - "1.2.3") > + LXCVersion("1.2.3")) > self._LXC_MIN_VERSION_REQUIRED_Patch = version_patch > self._LXC_MIN_VERSION_REQUIRED_Patch.start() > self.hvc = LXCHypervisor > @@ -217,12 +225,6 @@ class TestVerifyLXCCommands(unittest.TestCase): > 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") > -- > 2.0.4 > >
