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.

Reply via email to