Bulk-removing SSH keys of diverse set of nodes
authorHelga Velroyen <helgav@google.com>
Fri, 20 Nov 2015 10:16:58 +0000 (11:16 +0100)
committerHelga Velroyen <helgav@google.com>
Thu, 17 Dec 2015 08:13:02 +0000 (09:13 +0100)
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 <helgav@google.com>
Reviewed-by: Lisa Velden <velden@google.com>

lib/backend.py
test/py/ganeti.backend_unittest.py
test/py/testutils_ssh.py

index d4756be..3822a39 100644 (file)
@@ -1930,6 +1930,9 @@ def RemoveNodeSshKeyBulk(node_list,
   all_keys_to_remove = {}
   if from_authorized_keys or from_public_keys:
     for node_info in 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 node_info.name == master_node and not keys_to_remove:
         raise errors.SshUpdateError("Cannot remove the master node's keys.")
       if keys_to_remove:
       if node_info.name == master_node and not keys_to_remove:
         raise errors.SshUpdateError("Cannot remove the master node's keys.")
       if keys_to_remove:
index f9f0065..6a70592 100755 (executable)
@@ -1509,6 +1509,39 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
       self.assertEqual(1,
           len(self._ssh_file_manager.GetAuthorizedKeysOfNode(node_info.name)))
 
       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(
   def testDemoteMasterCandidateToPotentialMasterCandidate(self):
     node_name, node_info = self._ssh_file_manager.GetAllMasterCandidates()[0]
     self._ssh_file_manager.SetOrAddNode(
index fa4df3d..bc9b020 100644 (file)
@@ -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]
 
             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
+    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]
 
   def GetPublicKeysOfNode(self, node):
     return self._public_keys[node]