Am 22. Februar 2011 13:55 schrieb René Nussbaumer <[email protected]>:
> --- a/lib/client/gnt_cluster.py
> +++ b/lib/client/gnt_cluster.py
> +def _InstanceStart(opts, inst_list, start):
> […]
> + results = jex.GetResults()
> + bad_cnt = len([False for (success, _) in results if not success])
No need for a list, just use a generator.
> +class _RunWhenNodesReachableHelper:
> + """Helper class to make shared internal state sharing easier.
> +
> + @ivar success: Indicates if all action_cb calls were successful
> +
> + """
> + def __init__(self, node_list, action_cb, node2ip, port=None,
> + _ping_fn=netutils.TcpPing, _sleep_fn=time.sleep):
> + self.down = set(node_list)
> + self.up = set()
> + self.node2ip = node2ip
> + self.success = True
> + self.action_cb = action_cb
> + self._ping_fn = _ping_fn
> + self._sleep_fn = _sleep_fn
> +
> + if port is None:
> + self.port = netutils.GetDaemonPort(constants.NODED)
> + else:
> + self.port = port
Please make the port a mandatory argument and call GetDaemonPort in
the caller. Doing so reduces dependencies of the helper class.
> + def Wait(self, secs):
> + """Checks if a host is up or waits reamingin seconds.
s/reamingin/remaining
> --- a/man/gnt-cluster.rst
> +++ b/man/gnt-cluster.rst
> +EPO
> +~~~
> +
> +**epo** [--on] [--groups|--all] *arguments*
> +
> +Performans an emergency power-off on nodes given as arguments. If
> ``--groups``
s/Performans/Performs/
> +is given, arguments are node groups. If ``--all`` is provided, the whole
> +cluster will be shut down.
> --- a/qa/qa_cluster.py
> +++ b/qa/qa_cluster.py
> @@ -27,13 +27,14 @@ import tempfile
> import os.path
>
> from ganeti import constants
> +from ganeti import compat
> from ganeti import utils
>
> import qa_config
> import qa_utils
> import qa_error
>
> -from qa_utils import AssertEqual, AssertCommand
> +from qa_utils import AssertEqual, AssertCommand, GetCommandOutput
>
>
> def _RemoveFileFromAllNodes(filename):
> @@ -150,6 +151,37 @@ def TestClusterOob():
> "oob_program="])
>
>
> +def TestClusterEpo():
> + """gnt-cluster epo"""
> + master = qa_config.GetMasterNode()
> +
> + # Conflicting
> + AssertCommand(["gnt-cluster", "epo", "--groups", "--all"], fail=True)
> + # --all doesn't expect arguments
> + AssertCommand(["gnt-cluster", "epo", "--all", "some_arg"], fail=True)
> +
> + # Unless --all is given master is not allowed to be in the list
> + AssertCommand(["gnt-cluster", "epo", "-f", master["primary"]], fail=True)
> +
> + # This shouldn't fail
> + AssertCommand(["gnt-cluster", "epo", "-f", "--all"])
Won't this shut down all machines?
> + # All instances should have been stopped now
> + result_output = GetCommandOutput(master["primary"],
> + "gnt-instance list --no-header -o status")
> + AssertEqual(compat.any(status == "running"
> + for status in result_output.splitlines()), False)
You should rather check that all are stopped. They could be, after
all, in some error status.
> + # Now start everything again
> + AssertCommand(["gnt-cluster", "epo", "--on", "-f", "--all"])
Don't restart the QA machines. Can't you use a fake OOB script?
> --- /dev/null
> +++ b/test/ganeti.client.gnt_cluster_unittest.py
> +from ganeti.client import gnt_cluster
> +from ganeti.utils import retry
Just import “utils” and use the functions directly, e.g. “utils.Retry”.
> +class TestEpo(unittest.TestCase):
> + def setUp(self):
> + self.nodes = set()
> + self.nodes2ip = {}
> + self.ips2node = {}
> + for ip in range(1, 10):
> + name = "node%d" % ip
> + self.nodes.add(name)
> + self.nodes2ip[name] = ip
> + self.ips2node[ip] = name
This can be done with less code:
nodes2ip = dict(("node%s" % i, "192.0.2.%s" % i) for i in range(1, 10))
nodes = nodes.keys()
ip2node = dict((v, k) for (k, v) in nodes2ip.items())
> + def _FakeAction(*args):
> + return True
> +
> + def _FakePing(*args, **kwargs):
> + return True
Here you should verify that live_port, etc. is set correctly.
> + def _FakeSleep(secs):
Assert that secs is >= 0 and <= some value.
> + return
> +
> + def testPingFnRemoveHostsUp(self):
> + seen = {}
> + def _FakeSeenPing(ip, *args, **kwargs):
> + node = self.ips2node[ip]
> +
> + if node in seen:
> + seen[node] += 1
> + else:
> + seen[node] = 1
> + return True
If you make “seen” a set, this becomes much simpler.
self.assertFalse(node not in seen)
seen.add(node)
> + helper = gnt_cluster._RunWhenNodesReachableHelper(self.nodes,
> + self._FakeAction,
> + self.nodes2ip, port=0,
> + _ping_fn=_FakeSeenPing,
> +
> _sleep_fn=self._FakeSleep)
> +
> + nodes_len = len(self.nodes)
> + for num in range(nodes_len):
for num in enumerate(self.nodes):
> + helper.Wait(0)
> + if num < nodes_len - 1:
> + self.assertRaises(retry.RetryAgain, helper)
> + else:
> + helper()
> +
> + self.assert_(compat.all(count == 1 for count in seen.values()))
See above.
> + self.assertEqual(set(seen.keys()), self.nodes)
> + self.assertFalse(helper.down)
> + self.assertEqual(helper.up, self.nodes)
> +
> + def testActionReturnFalseSetsHelperFalse(self):
> + called = False
> + def _FalseAction(*args):
> + return called
> +
> + helper = gnt_cluster._RunWhenNodesReachableHelper(self.nodes,
> _FalseAction,
> + self.nodes2ip, port=0,
> +
> _ping_fn=self._FakePing,
> +
> _sleep_fn=self._FakeSleep)
> + for _ in self.nodes:
> + try:
> + helper()
> + except retry.RetryAgain:
> + called = True
Unfortunately there are no real closures in Python :-(
> + pass
The “pass” is pointless.
> + self.assertFalse(helper.success)
> +
> + def testMaybeInstanceStartup(self):
> + instances_arg = []
> + def _FakeInstanceStart(opts, instances, start):
> + instances_arg.append(set(instances))
> + return None
> +
> + inst_map = {
> + "inst1": set(["node1", "node2"]),
> + "inst2": set(["node1", "node3"]),
> + "inst3": set(["node2", "node1"]),
Can you also test with instances with one, three and four nodes? The
algorithm must be generic.
> + }
> +
> + fn = _FakeInstanceStart
> + self.assert_(gnt_cluster._MaybeInstanceStartup(None, inst_map, set(),
> + _instance_start_fn=fn))
> + self.assertFalse(instances_arg)
> + result = gnt_cluster._MaybeInstanceStartup(None, inst_map,
> set(["node1"]),
> + _instance_start_fn=fn)
> + self.assert_(result)
> + self.assertFalse(instances_arg)
> + result = gnt_cluster._MaybeInstanceStartup(None, inst_map,
> + set(["node1", "node3"]),
> + _instance_start_fn=fn)
> + self.assert_(result is None)
> + self.assertEqual(instances_arg.pop(0), set(["inst2"]))
> + self.assertFalse("inst2" in inst_map)
> + result = gnt_cluster._MaybeInstanceStartup(None, inst_map,
> + set(["node1", "node3"]),
> + _instance_start_fn=fn)
> + self.assert_(result)
> + self.assertFalse(instances_arg)
> + result = gnt_cluster._MaybeInstanceStartup(None, inst_map,
> + set(["node1", "node3",
> "node2"]),
> + _instance_start_fn=fn)
> + self.assert_(result is None)
> + self.assertEqual(instances_arg.pop(0), set(["inst1", "inst3"]))
> + self.assertFalse(inst_map)
I may have missed it, but is there a test verifying that the action
callback only gets started nodes?
Michael
