LGTM On Thu, Dec 17, 2015 at 9:07 AM Helga Velroyen <[email protected]> wrote:
> +list > > FYI, interdiff: > > diff --git a/test/py/testutils_ssh.py b/test/py/testutils_ssh.py > index 7d22508..a38304d 100644 > --- a/test/py/testutils_ssh.py > +++ b/test/py/testutils_ssh.py > @@ -232,7 +232,7 @@ class FakeSshFileManager(object): > def GetAllNodesDiverse(self): > """This returns all nodes in a diverse order. > > - This will return all nodes, but makes sure that they ordered so that > + This will return all nodes, but makes sure that they are ordered so > that > the list will contain in a round-robin fashion, a master candidate, > a potential master candidate, a normal node, then again a master > candidate, etc. > > > On Wed, 2 Dec 2015 at 19:09 Lisa Velden <[email protected]> wrote: > >> On Tue, Nov 24, 2015 at 3:15 PM, 'Helga Velroyen' via ganeti-devel < >> [email protected]> wrote: >> >>> This patch adds a unit test where SSH keys of a diverse >>> set of nodes is removed. By 'diverse', we mean a set >>> consisting of master candidates, potential master >>> candidates, and normal nodes. >>> >>> It also fixes some minor bug that surfaced with that >>> test. >>> >>> Signed-off-by: Helga Velroyen <[email protected]> >>> --- >>> lib/backend.py | 4 +++- >>> test/py/ganeti.backend_unittest.py | 33 >>> +++++++++++++++++++++++++++++++++ >>> test/py/testutils_ssh.py | 37 >>> +++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 73 insertions(+), 1 deletion(-) >>> >>> diff --git a/lib/backend.py b/lib/backend.py >>> index cfc6ec8..35f8c47 100644 >>> --- a/lib/backend.py >>> +++ b/lib/backend.py >>> @@ -1930,7 +1930,9 @@ def RemoveNodeSshKeyBulk(node_list, >>> all_keys_to_remove = {} >>> if from_authorized_keys or from_public_keys: >>> for node_info in node_list: >>> - >>> + # Skip nodes that don't actually need any keys to be removed. >>> + if not (node_info.from_authorized_keys or >>> node_info.from_public_keys): >>> + continue >>> if keys_to_remove: >>> keys = keys_to_remove >>> else: >>> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ >>> ganeti.backend_unittest.py >>> index 80845b2..94424ce 100755 >>> --- a/test/py/ganeti.backend_unittest.py >>> +++ b/test/py/ganeti.backend_unittest.py >>> @@ -1509,6 +1509,39 @@ class >>> TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase): >>> self.assertEqual(1, >>> len(self._ssh_file_manager.GetAuthorizedKeysOfNode( >>> node_info.name))) >>> >>> + def testRemoveDiverseNodesBulk(self): >>> + node_list = [] >>> + key_map = {} >>> + for node_name, (node_uuid, node_key, is_potential_master_candidate, >>> + is_master_candidate, _) in \ >>> + self._ssh_file_manager.GetAllNodesDiverse()[:3]: >>> + node_list.append(backend.SshRemoveNodeInfo( >>> + uuid=node_uuid, >>> + name=node_name, >>> + from_authorized_keys=is_master_candidate, >>> + from_public_keys=is_potential_master_candidate, >>> + clear_authorized_keys=True, >>> + clear_public_keys=True)) >>> + key_map[node_name] = node_key >>> + >>> + backend.RemoveNodeSshKeyBulk(node_list, >>> + self._master_candidate_uuids, >>> + self._potential_master_candidates, >>> + pub_key_file=self._pub_key_file, >>> + ssconf_store=self._ssconf_mock, >>> + noded_cert_file=self.noded_cert_file, >>> + run_cmd_fn=self._run_cmd_mock) >>> + >>> + for node_info in node_list: >>> + self._ssh_file_manager.AssertNoNodeHasPublicKey( >>> + node_info.uuid, key_map[node_info.name]) >>> + self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey( >>> + [node_info.name], key_map[node_info.name]) >>> + self.assertEqual(0, >>> + len(self._ssh_file_manager.GetPublicKeysOfNode(node_info.name >>> ))) >>> + self.assertEqual(1, >>> + len(self._ssh_file_manager.GetAuthorizedKeysOfNode( >>> node_info.name))) >>> + >>> def testDemoteMasterCandidateToPotentialMasterCandidate(self): >>> node_name, node_info = >>> self._ssh_file_manager.GetAllMasterCandidates()[0] >>> self._ssh_file_manager.SetOrAddNode( >>> diff --git a/test/py/testutils_ssh.py b/test/py/testutils_ssh.py >>> index 7dbb6fb..d00b3c4 100644 >>> --- a/test/py/testutils_ssh.py >>> +++ b/test/py/testutils_ssh.py >>> @@ -203,6 +203,43 @@ class FakeSshFileManager(object): >>> in self._all_node_data.items() if not >>> node_info.is_master_candidate >>> and not node_info.is_potential_master_candidate] >>> >>> + def GetAllNodesDiverse(self): >>> + """This returns all nodes in a diverse order. >>> + >>> + This will return all nodes, but makes sure that they ordered so that >>> >> >> s/they ordered/they are ordered >> >> >>> + the list will contain in a round-robin fashion, a master candidate, >>> + a potential master candidate, a normal node, then again a master >>> + candidate, etc. >>> + >>> + """ >>> + master_candidates = self.GetAllMasterCandidates() >>> + potential_master_candidates = >>> self.GetAllPurePotentialMasterCandidates() >>> + normal_nodes = self.GetAllNormalNodes() >>> + >>> + mixed_list = [] >>> + >>> + i = 0 >>> + >>> + assert (len(self._all_node_data) == len(master_candidates) >>> + + len(potential_master_candidates) + len(normal_nodes)) >>> + >>> + while len(mixed_list) < len(self._all_node_data): >>> + if i % 3 == 0: >>> + if master_candidates: >>> + mixed_list.append(master_candidates[0]) >>> + master_candidates = master_candidates[1:] >>> + elif i % 3 == 1: >>> + if potential_master_candidates: >>> + mixed_list.append(potential_master_candidates[0]) >>> + potential_master_candidates = potential_master_candidates[1:] >>> + else: # i % 3 == 2 >>> + if normal_nodes: >>> + mixed_list.append(normal_nodes[0]) >>> + normal_nodes = normal_nodes[1:] >>> + i += 1 >>> + >>> + return mixed_list >>> + >>> def GetPublicKeysOfNode(self, node): >>> return self._public_keys[node] >>> >>> -- >>> 2.6.0.rc2.230.g3dd15c0 >>> >>> >> >> Rest LGTM >> >> -- >> 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 >> > -- > > 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
