LGTM with interdiff, thanks
On Wed, Nov 25, 2015 at 8:08 AM, Oleg Ponomarev <[email protected]>
wrote:
> Remarks fixed.
> Finally, I decide to add assert and therefore fixed python tests. Here is
> the interdiff:
>
> diff --git a/lib/hooksmaster.py b/lib/hooksmaster.py
> index 159aa14..4f1d7bd 100644
> --- a/lib/hooksmaster.py
> +++ b/lib/hooksmaster.py
> @@ -188,7 +188,7 @@ class HooksMaster(object):
> assert compat.all(key == "PATH" or key.startswith("GANETI_")
> for key in env)
> for node in node_list:
> - assert utils.UUID_RE.match(node)
> + assert utils.UUID_RE.match(node), "Invalid node uuid %s" % node
>
> return self.hooks_execution_fn(node_list, hpath, phase, env)
>
> diff --git a/test/py/cmdlib/cluster_unittest.py
> b/test/py/cmdlib/cluster_unittest.py
> index b77c87a..7b4b4e7 100644
> --- a/test/py/cmdlib/cluster_unittest.py
> +++ b/test/py/cmdlib/cluster_unittest.py
> @@ -1208,7 +1208,7 @@ class TestLUClusterVerifyClientCerts(CmdlibTestCase):
> def _AddNormalNode(self):
> self.normalnode = copy.deepcopy(self.master)
> self.normalnode.master_candidate = False
> - self.normalnode.uuid = "normal-node-uuid"
> + self.normalnode.uuid = "deadbeef-dead-beef-dead-beefdeadbeef"
> self.cfg.AddNode(self.normalnode, None)
>
> def testVerifyMasterCandidate(self):
> diff --git a/test/py/cmdlib/node_unittest.py
> b/test/py/cmdlib/node_unittest.py
> index 8dcad78..d6dc66a 100644
> --- a/test/py/cmdlib/node_unittest.py
> +++ b/test/py/cmdlib/node_unittest.py
> @@ -291,12 +291,12 @@ class TestLUNodeSetParams(CmdlibTestCase):
> self.node = self.cfg.AddNewNode(
> primary_ip='192.168.168.191',
> secondary_ip='192.168.168.192',
> - master_candidate=True, uuid='blue_bunny')
> + master_candidate=True,
> uuid='00000000-dead-beef-dead-beefdeadbeef')
>
> self.snode = self.cfg.AddNewNode(
> primary_ip='192.168.168.193',
> secondary_ip='192.168.168.194',
> - master_candidate=True, uuid='pink_bunny')
> + master_candidate=True,
> uuid='11111111-dead-beef-dead-beefdeadbeef')
>
> def testSetSecondaryIp(self):
> self.instance = self.cfg.AddNewInstance(primary_node=self.node,
> @@ -310,8 +310,8 @@ class TestLUNodeSetParams(CmdlibTestCase):
> self.assertEqual(sorted(self.wconfd.all_locks.items()), [
> ('cluster/BGL', 'shared'),
> ('instance/mock_inst_1.example.com', 'shared'),
> - ('node-res/blue_bunny', 'exclusive'),
> - ('node/blue_bunny', 'exclusive')])
> + ('node-res/00000000-dead-beef-dead-beefdeadbeef', 'exclusive'),
> + ('node/00000000-dead-beef-dead-beefdeadbeef', 'exclusive')])
>
> def testSetSecondaryIpNoLock(self):
> self.instance = self.cfg.AddNewInstance(primary_node=self.node,
> @@ -324,8 +324,8 @@ class TestLUNodeSetParams(CmdlibTestCase):
> self.assertEqual('254.254.254.254', self.node.secondary_ip)
> self.assertEqual(sorted(self.wconfd.all_locks.items()), [
> ('cluster/BGL', 'shared'),
> - ('node-res/blue_bunny', 'exclusive'),
> - ('node/blue_bunny', 'exclusive')])
> + ('node-res/00000000-dead-beef-dead-beefdeadbeef', 'exclusive'),
> + ('node/00000000-dead-beef-dead-beefdeadbeef', 'exclusive')])
>
>
> if __name__ == "__main__":
> diff --git a/test/py/ganeti.hooks_unittest.py b/test/py/
> ganeti.hooks_unittest.py
> index 899c411..9e297a3 100755
> --- a/test/py/ganeti.hooks_unittest.py
> +++ b/test/py/ganeti.hooks_unittest.py
> @@ -60,7 +60,8 @@ class FakeLU(cmdlib.LogicalUnit):
> return {}
>
> def BuildHooksNodes(self):
> - return ["a"], ["a"]
> + return (["aaaaaaaa-dead-beef-dead-beefdeadbeef"],
> + ["aaaaaaaa-dead-beef-dead-beefdeadbeef"])
>
>
> class TestHooksRunner(unittest.TestCase):
> @@ -299,7 +300,8 @@ class FakeEnvLU(cmdlib.LogicalUnit):
> return self.hook_env
>
> def BuildHooksNodes(self):
> - return (["a"], ["a"])
> + return (["aaaaaaaa-dead-beef-dead-beefdeadbeef"],
> + ["aaaaaaaa-dead-beef-dead-beefdeadbeef"])
>
>
> class FakeNoHooksLU(cmdlib.NoHooksLU):
> @@ -515,10 +517,11 @@ class
> FakeEnvWithCustomPostHookNodesLU(cmdlib.LogicalUnit):
> return {}
>
> def BuildHooksNodes(self):
> - return (["a"], ["a"])
> + return (["aaaaaaaa-dead-beef-dead-beefdeadbeef"],
> + ["aaaaaaaa-dead-beef-dead-beefdeadbeef"])
>
>
>
> On 11/24/2015 05:29 PM, Hrvoje Ribicic wrote:
>
> On Tue, Nov 24, 2015 at 5:18 PM, Oleg Ponomarev <[email protected]>
> wrote:
>
>> On 11/24/2015 05:06 PM, Hrvoje Ribicic wrote:
>>
>>> One possible improvement: we can consider asserting that all node_uuids
>>> passed are truly uuids whenever RunPhase is invoked. Or do you think this
>>> is too paranoid?
>>>
>>>
>> I am not sure that it worth it because it will break several tests which
>> use node names like "a" or still use names. Also passing node names is not
>> that critical because rpc runner can deal with them.
>>
>
> Since tests should reflect real use cases, they should probably be changed
> as a part of this patch as well.
>
> Hrvoje Ribicic
> Ganeti Engineering
> Google Germany GmbH
> Dienerstr. 12, 80331, München
>
> Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
>
> Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
> leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
> löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
>
> This e-mail is confidential. If you are not the right addressee please do
> not forward it, please inform the sender, and please erase this e-mail
> including any attachments. Thanks.
>
>
>
Hrvoje Ribicic
Ganeti Engineering
Google Germany GmbH
Dienerstr. 12, 80331, München
Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Diese E-Mail ist vertraulich. Wenn Sie nicht der richtige Adressat sind,
leiten Sie diese bitte nicht weiter, informieren Sie den Absender und
löschen Sie die E-Mail und alle Anhänge. Vielen Dank.
This e-mail is confidential. If you are not the right addressee please do
not forward it, please inform the sender, and please erase this e-mail
including any attachments. Thanks.
