Am 22. Februar 2011 15:37 schrieb Rene Nussbaumer <[email protected]>:
> On Tue, Feb 22, 2011 at 2:40 PM, Michael Hanselmann <[email protected]> wrote:
>> 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.
>
> I don't get that
>>> type([False for _ in range(2)])
<type 'list'>
>>> type(False for _ in range(2))
<type 'generator'>
>>> + # This shouldn't fail
>>> + AssertCommand(["gnt-cluster", "epo", "-f", "--all"])
>>
>> Won't this shut down all machines?
>
> No, at this stage the QA no OOB script, if the the nodes do not
> support OOB they will not been shut down and just beeing noted on the
> output that they need manual care.
Please add a check asserting this.
>>> + # 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.
>
> That was intentional, because the command would have failed in one of
> the instances failed stopped, so we wouldn't reach that point. But I
> can change … to ADMIN_down I guess?
Yes, please.
>>> --- /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”.
>
> Okay, done. Don't get the splitting of utils then ;).
utils.py is smaller this way.
>>> + def _FakePing(*args, **kwargs):
>>> + return True
>>
>> Here you should verify that live_port, etc. is set correctly.
>
> I don't see the point in that. This function should just emulate a
> host up, not checking anything.
You should make sure the helper function calls the function with the
right parameters. See
ganeti.jqueue_unittest.py:_FakeQueueForProc.acquire for example.
>>> + def _FakeSleep(secs):
>>
>> Assert that secs is >= 0 and <= some value.
>
> Same applies here.
You want to make sure the wait time calculation doesn't go wrong.
>>> + 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)
>
> But this is not doing the same. I want to have exactly 1 call for
> every node, not 0, not 2, just 1. If I simply add them into the set I
> don't know how many times it was called.
You don't need to. _FakeSeenPing will already fail if it's called more
than once.
Michael
