LGTM On Tue, Dec 1, 2015 at 3:34 PM 'Helga Velroyen' via ganeti-devel < [email protected]> wrote:
> 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. > > -- Lisa Velden 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
