From 75bf31493fc1d65c588a3be5d29b0d0e4733683f Mon Sep 17 00:00:00 2001 From: Helga Velroyen Date: Tue, 4 Jun 2013 17:24:49 +0200 Subject: [PATCH] Verify: xen toolstack, hypervisor and hvparams This patch extends the node verification by: - Adding a check for the xen toolstack when the hypervisor is verified. - Factoring out the hypervisor verification in a subfunction to increase testability. - Factoring out the hvparams verification in a subfunction for the same reason. Signed-off-by: Helga Velroyen Reviewed-by: Thomas Thrainer --- lib/backend.py | 79 +++++++++++++++++----- lib/hypervisor/hv_base.py | 5 +- lib/hypervisor/hv_chroot.py | 6 +- lib/hypervisor/hv_fake.py | 6 +- lib/hypervisor/hv_kvm.py | 5 +- lib/hypervisor/hv_lxc.py | 5 +- lib/hypervisor/hv_xen.py | 64 +++++++++++++++++- lib/server/noded.py | 3 +- test/py/ganeti.backend_unittest.py | 35 +++++++++- test/py/ganeti.hypervisor.hv_xen_unittest.py | 94 +++++++++++++++++++++++--- 10 files changed, 264 insertions(+), 38 deletions(-) diff --git a/lib/backend.py b/lib/backend.py index 6bb7dff..8b24151 100644 --- a/lib/backend.py +++ b/lib/backend.py @@ -726,6 +726,65 @@ def _CheckExclusivePvs(pvi_list): return res +def _VerifyHypervisors(what, vm_capable, result, all_hvparams, + get_hv_fn=hypervisor.GetHypervisor): + """Verifies the hypervisor. Appends the results to the 'results' list. + + @type what: C{dict} + @param what: a dictionary of things to check + @type vm_capable: boolean + @param vm_capable: whether or not this not is vm capable + @type result: dict + @param result: dictionary of verification results; results of the + verifications in this function will be added here + @type all_hvparams: dict of dict of string + @param all_hvparams: dictionary mapping hypervisor names to hvparams + @type get_hv_fn: function + @param get_hv_fn: function to retrieve the hypervisor, to improve testability + + """ + if not vm_capable: + return + + if constants.NV_HYPERVISOR in what: + result[constants.NV_HYPERVISOR] = {} + for hv_name in what[constants.NV_HYPERVISOR]: + hvparams = all_hvparams[hv_name] + try: + val = get_hv_fn(hv_name).Verify(hvparams=hvparams) + except errors.HypervisorError, err: + val = "Error while checking hypervisor: %s" % str(err) + result[constants.NV_HYPERVISOR][hv_name] = val + + +def _VerifyHvparams(what, vm_capable, result, + get_hv_fn=hypervisor.GetHypervisor): + """Verifies the hvparams. Appends the results to the 'results' list. + + @type what: C{dict} + @param what: a dictionary of things to check + @type vm_capable: boolean + @param vm_capable: whether or not this not is vm capable + @type result: dict + @param result: dictionary of verification results; results of the + verifications in this function will be added here + @type get_hv_fn: function + @param get_hv_fn: function to retrieve the hypervisor, to improve testability + + """ + if not vm_capable: + return + + if constants.NV_HVPARAMS in what: + result[constants.NV_HVPARAMS] = [] + for source, hv_name, hvparms in what[constants.NV_HVPARAMS]: + try: + logging.info("Validating hv %s, %s", hv_name, hvparms) + get_hv_fn(hv_name).ValidateParameters(hvparms) + except errors.HypervisorError, err: + result[constants.NV_HVPARAMS].append((source, hv_name, str(err))) + + def VerifyNode(what, cluster_name): """Verify the status of the local node. @@ -750,6 +809,7 @@ def VerifyNode(what, cluster_name): - node-net-test: list of nodes we should check node daemon port connectivity with - hypervisor: list with hypervisors to run the verify for + @rtype: dict @return: a dictionary with the same keys as the input dict, and values representing the result of the checks @@ -760,23 +820,8 @@ def VerifyNode(what, cluster_name): port = netutils.GetDaemonPort(constants.NODED) vm_capable = my_name not in what.get(constants.NV_VMNODES, []) - if constants.NV_HYPERVISOR in what and vm_capable: - result[constants.NV_HYPERVISOR] = tmp = {} - for hv_name in what[constants.NV_HYPERVISOR]: - try: - val = hypervisor.GetHypervisor(hv_name).Verify() - except errors.HypervisorError, err: - val = "Error while checking hypervisor: %s" % str(err) - tmp[hv_name] = val - - if constants.NV_HVPARAMS in what and vm_capable: - result[constants.NV_HVPARAMS] = tmp = [] - for source, hv_name, hvparms in what[constants.NV_HVPARAMS]: - try: - logging.info("Validating hv %s, %s", hv_name, hvparms) - hypervisor.GetHypervisor(hv_name).ValidateParameters(hvparms) - except errors.HypervisorError, err: - tmp.append((source, hv_name, str(err))) + _VerifyHypervisors(what, vm_capable, result, all_hvparams) + _VerifyHvparams(what, vm_capable, result) if constants.NV_FILELIST in what: fingerprints = utils.FingerprintFiles(map(vcluster.LocalizeVirtualPath, diff --git a/lib/hypervisor/hv_base.py b/lib/hypervisor/hv_base.py index f06a1c2..5135b4e 100644 --- a/lib/hypervisor/hv_base.py +++ b/lib/hypervisor/hv_base.py @@ -263,9 +263,12 @@ class BaseHypervisor(object): return (cls.ANCILLARY_FILES, cls.ANCILLARY_FILES_OPT) - def Verify(self): + def Verify(self, hvparams=None): """Verify the hypervisor. + @type hvparams: dict of strings + @param hvparams: hypervisor parameters to be verified against + @return: Problem description if something is wrong, C{None} otherwise """ diff --git a/lib/hypervisor/hv_chroot.py b/lib/hypervisor/hv_chroot.py index 04007f5..2e8bb20 100644 --- a/lib/hypervisor/hv_chroot.py +++ b/lib/hypervisor/hv_chroot.py @@ -276,11 +276,15 @@ class ChrootManager(hv_base.BaseHypervisor): user=constants.SSH_CONSOLE_USER, command=["chroot", root_dir]) - def Verify(self): + def Verify(self, hvparams=None): """Verify the hypervisor. For the chroot manager, it just checks the existence of the base dir. + @type hvparams: dict of strings + @param hvparams: hypervisor parameters to be verified against, not used + in for chroot + @return: Problem description if something is wrong, C{None} otherwise """ diff --git a/lib/hypervisor/hv_fake.py b/lib/hypervisor/hv_fake.py index cccd57d..27d929c 100644 --- a/lib/hypervisor/hv_fake.py +++ b/lib/hypervisor/hv_fake.py @@ -233,12 +233,16 @@ class FakeHypervisor(hv_base.BaseHypervisor): message=("Console not available for fake" " hypervisor")) - def Verify(self): + def Verify(self, hvparams=None): """Verify the hypervisor. For the fake hypervisor, it just checks the existence of the base dir. + @type hvparams: dict of strings + @param hvparams: hypervisor parameters to be verified against; not used + for fake hypervisors + @return: Problem description if something is wrong, C{None} otherwise """ diff --git a/lib/hypervisor/hv_kvm.py b/lib/hypervisor/hv_kvm.py index ca8fcac..526a866 100644 --- a/lib/hypervisor/hv_kvm.py +++ b/lib/hypervisor/hv_kvm.py @@ -2044,11 +2044,14 @@ class KVMHypervisor(hv_base.BaseHypervisor): message=("No serial shell for instance %s" % instance.name)) - def Verify(self): + def Verify(self, hvparams=None): """Verify the hypervisor. Check that the required binaries exist. + @type hvparams: dict of strings + @param hvparams: hypervisor parameters to be verified against, not used here + @return: Problem description if something is wrong, C{None} otherwise """ diff --git a/lib/hypervisor/hv_lxc.py b/lib/hypervisor/hv_lxc.py index 922b26a..369ed58 100644 --- a/lib/hypervisor/hv_lxc.py +++ b/lib/hypervisor/hv_lxc.py @@ -417,11 +417,14 @@ class LXCHypervisor(hv_base.BaseHypervisor): user=constants.SSH_CONSOLE_USER, command=["lxc-console", "-n", instance.name]) - def Verify(self): + def Verify(self, hvparams=None): """Verify the hypervisor. For the LXC manager, it just checks the existence of the base dir. + @type hvparams: dict of strings + @param hvparams: hypervisor parameters to be verified against; not used here + @return: Problem description if something is wrong, C{None} otherwise """ diff --git a/lib/hypervisor/hv_xen.py b/lib/hypervisor/hv_xen.py index 1b689cd..eefce00 100644 --- a/lib/hypervisor/hv_xen.py +++ b/lib/hypervisor/hv_xen.py @@ -630,17 +630,33 @@ class XenHypervisor(hv_base.BaseHypervisor): command=[pathutils.XEN_CONSOLE_WRAPPER, constants.XEN_CMD, instance.name]) - def Verify(self): + def Verify(self, hvparams=None): """Verify the hypervisor. For Xen, this verifies that the xend process is running. + @type hvparams: dict of strings + @param hvparams: hypervisor parameters to be verified against + @return: Problem description if something is wrong, C{None} otherwise """ - result = self._RunXen(["info"]) + if hvparams is None: + return "Could not verify the hypervisor, because no hvparams were" \ + " provided." + + if constants.HV_XEN_CMD in hvparams: + xen_cmd = hvparams[constants.HV_XEN_CMD] + try: + self._CheckToolstack(xen_cmd) + except errors.HypervisorError: + return "The configured xen toolstack '%s' is not available on this" \ + " node." % xen_cmd + + result = self._RunXen(["info"], hvparams=hvparams) if result.failed: - return "'xm info' failed: %s, %s" % (result.fail_reason, result.output) + return "Retrieving information from xen failed: %s, %s" % \ + (result.fail_reason, result.output) return None @@ -800,6 +816,48 @@ class XenHypervisor(hv_base.BaseHypervisor): finally: utils.RunCmd([constants.XEN_CMD, "debug", "R"]) + def _CheckToolstack(self, xen_cmd): + """Check whether the given toolstack is available on the node. + + @type xen_cmd: string + @param xen_cmd: xen command (e.g. 'xm' or 'xl') + + """ + binary_found = self._CheckToolstackBinary(xen_cmd) + if not binary_found: + raise errors.HypervisorError("No '%s' binary found on node." % xen_cmd) + elif xen_cmd == constants.XEN_CMD_XL: + if not self._CheckToolstackXlConfigured(): + raise errors.HypervisorError("Toolstack '%s' is not enabled on this" + "node." % xen_cmd) + + def _CheckToolstackBinary(self, xen_cmd): + """Checks whether the xen command's binary is found on the machine. + + """ + if xen_cmd not in constants.KNOWN_XEN_COMMANDS: + raise errors.HypervisorError("Unknown xen command '%s'." % xen_cmd) + result = self._run_cmd_fn(["which", xen_cmd]) + return not result.failed + + def _CheckToolstackXlConfigured(self): + """Checks whether xl is enabled on an xl-capable node. + + @rtype: bool + @returns: C{True} if 'xl' is enabled, C{False} otherwise + + """ + result = self._run_cmd_fn([constants.XEN_CMD_XL, "help"]) + if not result.failed: + return True + elif result.failed: + if "toolstack" in result.stderr: + return False + # xl fails for some other reason than the toolstack + else: + raise errors.HypervisorError("Cannot run xen ('%s'). Error: %s." + % (constants.XEN_CMD_XL, result.stderr)) + class XenPvmHypervisor(XenHypervisor): """Xen PVM hypervisor interface""" diff --git a/lib/server/noded.py b/lib/server/noded.py index 40ec1ec..5c33244 100644 --- a/lib/server/noded.py +++ b/lib/server/noded.py @@ -738,7 +738,8 @@ class NodeRequestHandler(http.server.HttpServerHandler): """Run a verify sequence on this node. """ - return backend.VerifyNode(params[0], params[1]) + (what, cluster_name) = params + return backend.VerifyNode(what, cluster_name) @classmethod def perspective_node_verify_light(cls, params): diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ganeti.backend_unittest.py index c144f01..b6af4cd 100755 --- a/test/py/ganeti.backend_unittest.py +++ b/test/py/ganeti.backend_unittest.py @@ -77,11 +77,22 @@ class TestX509Certificates(unittest.TestCase): class TestNodeVerify(testutils.GanetiTestCase): + + def setUp(self): + testutils.GanetiTestCase.setUp(self) + self._mock_hv = None + + def _GetHypervisor(self, hv_name): + self._mock_hv = hypervisor.GetHypervisor(hv_name) + self._mock_hv.ValidateParameters = mock.Mock() + self._mock_hv.Verify = mock.Mock() + return self._mock_hv + def testMasterIPLocalhost(self): # this a real functional test, but requires localhost to be reachable local_data = (netutils.Hostname.GetSysName(), constants.IP4_ADDRESS_LOCALHOST) - result = backend.VerifyNode({constants.NV_MASTERIP: local_data}, None) + result = backend.VerifyNode({constants.NV_MASTERIP: local_data}, None, {}) self.failUnless(constants.NV_MASTERIP in result, "Master IP data not returned") self.failUnless(result[constants.NV_MASTERIP], "Cannot reach localhost") @@ -92,12 +103,32 @@ class TestNodeVerify(testutils.GanetiTestCase): bad_data = ("master.example.com", "192.0.2.1") # we just test that whatever TcpPing returns, VerifyNode returns too netutils.TcpPing = lambda a, b, source=None: False - result = backend.VerifyNode({constants.NV_MASTERIP: bad_data}, None) + result = backend.VerifyNode({constants.NV_MASTERIP: bad_data}, None, {}) self.failUnless(constants.NV_MASTERIP in result, "Master IP data not returned") self.failIf(result[constants.NV_MASTERIP], "Result from netutils.TcpPing corrupted") + def testVerifyHvparams(self): + test_hvparams = {constants.HV_XEN_CMD: constants.XEN_CMD_XL} + test_what = {constants.NV_HVPARAMS: \ + [("mynode", constants.HT_XEN_PVM, test_hvparams)]} + result = {} + backend._VerifyHvparams(test_what, True, result, + get_hv_fn=self._GetHypervisor) + self._mock_hv.ValidateParameters.assert_called_with(test_hvparams) + + def testVerifyHypervisors(self): + hvname = constants.HT_XEN_PVM + hvparams = {constants.HV_XEN_CMD: constants.XEN_CMD_XL} + all_hvparams = {hvname: hvparams} + test_what = {constants.NV_HYPERVISOR: [hvname]} + result = {} + backend._VerifyHypervisors( + test_what, True, result, all_hvparams=all_hvparams, + get_hv_fn=self._GetHypervisor) + self._mock_hv.Verify.assert_called_with(hvparams=hvparams) + def _DefRestrictedCmdOwner(): return (os.getuid(), os.getgid()) diff --git a/test/py/ganeti.hypervisor.hv_xen_unittest.py b/test/py/ganeti.hypervisor.hv_xen_unittest.py index f069d0f..3b692da 100755 --- a/test/py/ganeti.hypervisor.hv_xen_unittest.py +++ b/test/py/ganeti.hypervisor.hv_xen_unittest.py @@ -412,6 +412,55 @@ class TestXenHypervisorListInstances(unittest.TestCase): mock_run_cmd.assert_called_with([expected_xen_cmd, self.XEN_LIST]) +class TestXenHypervisorCheckToolstack(unittest.TestCase): + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.cfg_name = "xen_config" + self.cfg_path = utils.PathJoin(self.tmpdir, self.cfg_name) + self.hv = hv_xen.XenHypervisor() + + def tearDown(self): + shutil.rmtree(self.tmpdir) + + def testBinaryNotFound(self): + RESULT_FAILED = utils.RunResult(1, None, "", "", "", None, None) + mock_run_cmd = mock.Mock(return_value=RESULT_FAILED) + hv = hv_xen.XenHypervisor(_cfgdir=NotImplemented, + _run_cmd_fn=mock_run_cmd) + result = hv._CheckToolstackBinary("xl") + self.assertFalse(result) + + def testCheckToolstackXlConfigured(self): + RESULT_OK = utils.RunResult(0, None, "", "", "", None, None) + mock_run_cmd = mock.Mock(return_value=RESULT_OK) + hv = hv_xen.XenHypervisor(_cfgdir=NotImplemented, + _run_cmd_fn=mock_run_cmd) + result = hv._CheckToolstackXlConfigured() + self.assertTrue(result) + + def testCheckToolstackXlNotConfigured(self): + RESULT_FAILED = utils.RunResult( + 1, None, "", + "ERROR: A different toolstack (xm) have been selected!", + "", None, None) + mock_run_cmd = mock.Mock(return_value=RESULT_FAILED) + hv = hv_xen.XenHypervisor(_cfgdir=NotImplemented, + _run_cmd_fn=mock_run_cmd) + result = hv._CheckToolstackXlConfigured() + self.assertFalse(result) + + def testCheckToolstackXlFails(self): + RESULT_FAILED = utils.RunResult( + 1, None, "", + "ERROR: The pink bunny hid the binary.", + "", None, None) + mock_run_cmd = mock.Mock(return_value=RESULT_FAILED) + hv = hv_xen.XenHypervisor(_cfgdir=NotImplemented, + _run_cmd_fn=mock_run_cmd) + self.assertRaises(errors.HypervisorError, hv._CheckToolstackXlConfigured) + + class TestXenHypervisorWriteConfigFile(unittest.TestCase): def setUp(self): self.tmpdir = tempfile.mkdtemp() @@ -436,6 +485,41 @@ class TestXenHypervisorWriteConfigFile(unittest.TestCase): self.fail("Exception was not raised") +class TestXenHypervisorVerify(unittest.TestCase): + + def setUp(self): + output = testutils.ReadTestData("xen-xm-info-4.0.1.txt") + self._result_ok = utils.RunResult(0, None, output, "", "", None, None) + + def testVerify(self): + hvparams = {constants.HV_XEN_CMD : constants.XEN_CMD_XL} + mock_run_cmd = mock.Mock(return_value=self._result_ok) + hv = hv_xen.XenHypervisor(_cfgdir=NotImplemented, + _run_cmd_fn=mock_run_cmd) + hv._CheckToolstack = mock.Mock(return_value=True) + result = hv.Verify(hvparams) + self.assertTrue(result is None) + + def testVerifyToolstackNotOk(self): + hvparams = {constants.HV_XEN_CMD : constants.XEN_CMD_XL} + mock_run_cmd = mock.Mock(return_value=self._result_ok) + hv = hv_xen.XenHypervisor(_cfgdir=NotImplemented, + _run_cmd_fn=mock_run_cmd) + hv._CheckToolstack = mock.Mock() + hv._CheckToolstack.side_effect = errors.HypervisorError("foo") + result = hv.Verify(hvparams) + self.assertTrue(result is not None) + + def testVerifyFailing(self): + result_failed = utils.RunResult(1, None, "", "", "", None, None) + mock_run_cmd = mock.Mock(return_value=result_failed) + hv = hv_xen.XenHypervisor(_cfgdir=NotImplemented, + _run_cmd_fn=mock_run_cmd) + hv._CheckToolstack = mock.Mock(return_value=True) + result = hv.Verify() + self.assertTrue(result is not None) + + class _TestXenHypervisor(object): TARGET = NotImplemented CMD = NotImplemented @@ -565,16 +649,6 @@ class _TestXenHypervisor(object): "testinstance.example.com", ]) - def testVerify(self): - output = testutils.ReadTestData("xen-xm-info-4.0.1.txt") - hv = self._GetHv(run_cmd=compat.partial(self._SuccessCommand, - output)) - self.assertTrue(hv.Verify() is None) - - def testVerifyFailing(self): - hv = self._GetHv(run_cmd=self._FailingCommand) - self.assertTrue("failed:" in hv.Verify()) - def _StartInstanceCommand(self, inst, paused, failcreate, cmd): if cmd == [self.CMD, "info"]: output = testutils.ReadTestData("xen-xm-info-4.0.1.txt") -- 1.7.10.4