On Fri, 27 Nov 2015 at 15:54 Hrvoje Ribicic <[email protected]> wrote:
> On Tue, Nov 24, 2015 at 3:15 PM, 'Helga Velroyen' via ganeti-devel <
> [email protected]> wrote:
>
>> This patch updates the SSH testutils to match reality better.
>> So far, the test framework did not consider the fact that
>> the key of each node should be added to it's own
>> 'authorized_keys' file, even if the node is not a master
>> candidate. This patch fixes that to represent the production
>> behavior more accurately.
>>
>> Signed-off-by: Helga Velroyen <[email protected]>
>> ---
>> test/py/ganeti.backend_unittest.py | 13 ++++++++-----
>> test/py/testutils_ssh.py | 5 +++--
>> 2 files changed, 11 insertions(+), 7 deletions(-)
>>
>> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/
>> ganeti.backend_unittest.py
>> index 35ea9f4..ab3de9e 100755
>> --- a/test/py/ganeti.backend_unittest.py
>> +++ b/test/py/ganeti.backend_unittest.py
>> @@ -1385,10 +1385,11 @@ class
>> TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
>>
>> self._ssh_file_manager.AssertNoNodeHasPublicKey(
>> node_info.uuid, node_info.key)
>> - self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_info.key)
>> + self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey(
>> + node_name, node_info.key)
>>
>
> Shouldn't this be [node_name]?
> And if so, why did the test succeed?
>
I guess python stroke again. When fed with a string instead of a list
python checks:
if 'node_name_8' in 'node_name_8':
instead of:
if 'node_name_8' in ['node_name_8']:
It then does simply string substring matching and that holds incidentally
true for the same cases as the list version (at least for that particular
unit test).
To fix this, I fixed the occurrence of the string and made it a list, but
also added an assertion for the type in the testutils. Additionally, I
fixed a whitespace inconsistency. See the interdiff at the end of this mail.
>
>
>> self.assertEqual(0,
>> len(self._ssh_file_manager.GetPublicKeysOfNode(node_name)))
>> - self.assertEqual(0,
>> + self.assertEqual(1,
>> len(self._ssh_file_manager.GetAuthorizedKeysOfNode(node_name)))
>>
>> def testRemoveNormalNode(self):
>> @@ -1408,10 +1409,11 @@ class
>> TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
>>
>> self._ssh_file_manager.AssertNoNodeHasPublicKey(
>> node_info.uuid, node_info.key)
>> - self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_info.key)
>> + self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey(
>> + [node_name], node_info.key)
>> self.assertEqual(0,
>> len(self._ssh_file_manager.GetPublicKeysOfNode(node_name)))
>> - self.assertEqual(0,
>> + self.assertEqual(1,
>> len(self._ssh_file_manager.GetAuthorizedKeysOfNode(node_name)))
>>
>> def testDemoteMasterCandidateToPotentialMasterCandidate(self):
>> @@ -1458,7 +1460,8 @@ class
>> TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
>>
>> self._ssh_file_manager.AssertNoNodeHasPublicKey(
>> node_info.uuid, node_info.key)
>> - self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_info.key)
>> + self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey(
>> + [node_name], node_info.key)
>>
>> def _GetReducedOnlineNodeList(self):
>> """'Randomly' mark some nodes as offline."""
>> diff --git a/test/py/testutils_ssh.py b/test/py/testutils_ssh.py
>> index e4d76c1..7dbb6fb 100644
>> --- a/test/py/testutils_ssh.py
>> +++ b/test/py/testutils_ssh.py
>> @@ -126,8 +126,9 @@ class FakeSshFileManager(object):
>> self._public_keys[receiving_node_name][node_info.uuid] =
>> [node_info.key]
>>
>> def _FillAuthorizedKeyOfOneNode(self, receiving_node_name):
>> - for node_info in self._all_node_data.values():
>> - if node_info.is_master_candidate:
>> + for node_name, node_info in self._all_node_data.items():
>> + if node_info.is_master_candidate \
>> + or node_name == receiving_node_name:
>> self._authorized_keys[receiving_node_name].add(node_info.key)
>>
>> def InitAllNodes(self, num_nodes, num_pot_mcs, num_mcs):
>> --
>> 2.6.0.rc2.230.g3dd15c0
>>
>>
>
consider the following interdiff:
diff --git a/test/py/ganeti.backend_unittest.py b/test/py/
ganeti.backend_unittest.py
index ab3de9e..e90d9af 100755
--- a/test/py/ganeti.backend_unittest.py
+++ b/test/py/ganeti.backend_unittest.py
@@ -1386,7 +1386,7 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
self._ssh_file_manager.AssertNoNodeHasPublicKey(
node_info.uuid, node_info.key)
self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey(
- node_name, node_info.key)
+ [node_name], node_info.key)
self.assertEqual(0,
len(self._ssh_file_manager.GetPublicKeysOfNode(node_name)))
self.assertEqual(1,
diff --git a/test/py/testutils_ssh.py b/test/py/testutils_ssh.py
index 7dbb6fb..fa4df3d 100644
--- a/test/py/testutils_ssh.py
+++ b/test/py/testutils_ssh.py
@@ -280,6 +280,7 @@ class FakeSshFileManager(object):
@param query_node_key: key which is looked for
"""
+ assert isinstance(node_set, list)
for node_name in self._all_node_data.keys():
if node_name in node_set:
if not self.NodeHasAuthorizedKey(node_name, query_node_key):
@@ -589,7 +590,6 @@ class FakeSshFileManager(object):
@see: C{ssh.QueryPubKey}
"""
-
assert self._master_node_name
all_keys = target_uuids is None
if all_keys:
> 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.
>
> --
Helga Velroyen
Software Engineer
[email protected]
Google Germany GmbH
Dienerstraße 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.