Use bulk-adding of keys in renew-crypto
authorHelga Velroyen <helgav@google.com>
Thu, 12 Nov 2015 12:48:59 +0000 (13:48 +0100)
committerHelga Velroyen <helgav@google.com>
Tue, 17 Nov 2015 15:36:02 +0000 (16:36 +0100)
This patch makes renew-crypto actually use the bulk-adding
function of SSH keys rather than adding each key
individually. This patch also adds a unit tests where the
bulk-adding is tested with a diverse set of keys (master
candidates, potential master candidates, normal nodes).

Signed-off-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Klaus Aehlig <aehlig@google.com>

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

index c847ca8..ce5bb6a 100644 (file)
@@ -1514,6 +1514,10 @@ def AddNodeSshKeyBulk(node_list,
     ssconf_store = ssconf.SimpleStore()
 
   for node_info in node_list:
+    # replacement not necessary for keys that are not supposed to be in the
+    # list of public keys
+    if not node_info.to_public_keys:
+      continue
     # Check and fix sanity of key file
     keys_by_name = ssh.QueryPubKeyFile([node_info.name], key_file=pub_key_file)
     keys_by_uuid = ssh.QueryPubKeyFile([node_info.uuid], key_file=pub_key_file)
@@ -2009,6 +2013,10 @@ def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
   all_node_errors = []
 
   # process non-master nodes
+
+  # keys to add in bulk at the end
+  node_keys_to_add = []
+
   for node_uuid, node_name in node_uuid_name_map:
     if node_name == master_node_name:
       continue
@@ -2070,16 +2078,20 @@ def RenewSshKeys(node_uuids, node_names, master_candidate_uuids,
       ssh.AddPublicKey(node_uuid, pub_key, key_file=pub_key_file)
 
     logging.debug("Add ssh key of node '%s'.", node_name)
-    node_errors = AddNodeSshKey(
-        node_uuid, node_name, potential_master_candidates,
-        to_authorized_keys=master_candidate,
-        to_public_keys=potential_master_candidate,
-        get_public_keys=True,
-        pub_key_file=pub_key_file, ssconf_store=ssconf_store,
-        noded_cert_file=noded_cert_file,
-        run_cmd_fn=run_cmd_fn)
-    if node_errors:
-      all_node_errors = all_node_errors + node_errors
+    node_info = SshAddNodeInfo(name=node_name,
+                               uuid=node_uuid,
+                               to_authorized_keys=master_candidate,
+                               to_public_keys=potential_master_candidate,
+                               get_public_keys=True)
+    node_keys_to_add.append(node_info)
+
+  node_errors = AddNodeSshKeyBulk(
+      node_keys_to_add, potential_master_candidates,
+      pub_key_file=pub_key_file, ssconf_store=ssconf_store,
+      noded_cert_file=noded_cert_file,
+      run_cmd_fn=run_cmd_fn)
+  if node_errors:
+    all_node_errors = all_node_errors + node_errors
 
   # Renewing the master node's key
 
index 6185592..43d2dde 100755 (executable)
@@ -1269,6 +1269,54 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
       self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(
           key_map[node_info.name])
 
+  def _GetNewNumberedNode(self, num):
+    """Returns the properties of a node.
+
+    This will in round-robin style return a master candidate, a
+    potential master candiate and a normal node.
+
+    """
+    is_master_candidate = num % 3 == 0
+    is_potential_master_candidate = num % 3 == 0 or num % 3 == 1
+    is_master = False
+    return ("new_node_name_%s" % num,
+            "new_node_uuid_%s" % num,
+            "new_node_key_%s" % num,
+            is_master_candidate, is_potential_master_candidate, is_master)
+
+  def testAddDiverseNodeBulk(self):
+    """Tests adding keys of several nodes with several qualities.
+
+    This tests subsumes previous tests. However, we leave the previous
+    tests here, because debugging problems with this all-embracing test
+    is much more tedious than having one of the one-purpose tests fail.
+
+    """
+    num_nodes = 9
+    (node_list, key_map) = self._SetupNodeBulk(
+        num_nodes, self._GetNewNumberedNode)
+
+    backend.AddNodeSshKeyBulk(node_list,
+                              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:
+      if node_info.to_authorized_keys:
+        self._ssh_file_manager.AssertAllNodesHaveAuthorizedKey(
+            key_map[node_info.name])
+      else:
+        self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(
+            key_map[node_info.name])
+      if node_info.to_public_keys:
+        self._ssh_file_manager.AssertPotentialMasterCandidatesOnlyHavePublicKey(
+            node_info.name)
+      else:
+        self._ssh_file_manager.AssertNoNodeHasPublicKey(
+            node_info.uuid, key_map[node_info.name])
+
   def testPromoteToMasterCandidate(self):
     # Get one of the potential master candidates
     node_name, node_info = \