Do not remove authorized key of node itself
authorHelga Velroyen <helgav@google.com>
Fri, 6 Nov 2015 09:11:19 +0000 (10:11 +0100)
committerHelga Velroyen <helgav@google.com>
Fri, 6 Nov 2015 13:01:46 +0000 (14:01 +0100)
This fixes a small bug that if a node was demoted
from master candidate, that its own public key
was removed from its own authorized key file.

Signed-off-by: Helga Velroyen <helgav@google.com>
Reviewed-by: Lisa Velden <velden@google.com>

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

index cd94d92..f891ef6 100644 (file)
@@ -1686,6 +1686,9 @@ def RemoveNodeSshKey(node_uuid, node_name,
         if node not in online_nodes:
           logging.debug("Skipping offline node '%s'.", node)
           continue
+        if node == node_name:
+          logging.debug("Skipping node itself '%s'.", node_name)
+          continue
         ssh_port = ssh_port_map.get(node)
         if not ssh_port:
           raise errors.OpExecError("No SSH port information available for"
index 68b2eee..393239a 100755 (executable)
@@ -1195,10 +1195,11 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
                              run_cmd_fn=self._run_cmd_mock)
 
     self._ssh_file_manager.AssertNoNodeHasPublicKey(node_uuid, node_key)
-    self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_key)
+    self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey(
+        [node_name], node_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 testRemovePotentialMasterCandidate(self):
@@ -1268,7 +1269,8 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
 
     self._ssh_file_manager.AssertPotentialMasterCandidatesOnlyHavePublicKey(
         node_name)
-    self._ssh_file_manager.AssertNoNodeHasAuthorizedKey(node_info.key)
+    self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey(
+        [node_name], node_info.key)
 
   def testDemotePotentialMasterCandidateToNormalNode(self):
     (node_name, node_info) = \
@@ -1348,7 +1350,7 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
     offline_nodes = [node for node in self._all_nodes
                      if node not in self._online_nodes]
     self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey(
-        offline_nodes, node_info.key)
+        offline_nodes + [node_name], node_info.key)
 
   def testAddKeySuccessfullyOnNewNodeWithRetries(self):
     """Tests adding a new node's key when updating that node takes retries.
@@ -1524,7 +1526,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 testRemoveKeyFailedWithRetriesOnOtherNode(self):
     """Test removing keys even if one of the old nodes fails even with retries.
@@ -1552,7 +1555,7 @@ class TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
         noded_cert_file=self.noded_cert_file, run_cmd_fn=self._run_cmd_mock)
 
     self._ssh_file_manager.AssertNodeSetOnlyHasAuthorizedKey(
-        [other_node_name], node_info.key)
+        [other_node_name, node_name], node_info.key)
     self.assertTrue([error_msg for (node, error_msg) in error_msgs
                      if node == other_node_name])
 
@@ -1583,7 +1586,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 testRemoveKeyFailedWithRetriesOnTargetNode(self):
     """Test removing keys even if contacting the node fails with retries.