This patch changes the backend unittests in these aspects:
- They now use the newly introduced SSH file manager. In
particular, the creating of test data is now done
by the file manager.
- We add four smaller unit test which replace the big
(and hardly maintainable) unit test for adding keys.
On first sight, these four tests seem to duplicate
some code. This is true on the on hand, but that will
be addressed in this patch series as there are more
refactorings ahead. On the other hand, those four
tests differ in subtle ways, and there was not a good
way to consolidate the code without creating one big
test again.
- We remove the now superfluous big unit test for adding
keys again.
- In the course of this refactoring, we simplify some
parts where the original code was dealing with lists
of keys, but where we only actually need one key.
Signed-off-by: Helga Velroyen <[email protected]>
---
test/py/ganeti.backend_unittest.py | 305 +++++++++++++++++++++++--------------
1 file changed, 190 insertions(+), 115 deletions(-)
diff --git a/test/py/ganeti.backend_unittest.py
b/test/py/ganeti.backend_unittest.py
index 0709065..bb601f5 100755
--- a/test/py/ganeti.backend_unittest.py
+++ b/test/py/ganeti.backend_unittest.py
@@ -36,6 +36,7 @@ import os
import shutil
import tempfile
import testutils
+import testutils_ssh
import unittest
from ganeti import backend
@@ -956,12 +957,15 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
_SSH_PORT = 22
def setUp(self):
+ self._ssh_file_manager = testutils_ssh.FakeSshFileManager()
testutils.GanetiTestCase.setUp(self)
self._ssh_add_authorized_patcher = testutils \
.patch_object(ssh, "AddAuthorizedKeys")
self._ssh_remove_authorized_patcher = testutils \
.patch_object(ssh, "RemoveAuthorizedKeys")
self._ssh_add_authorized_mock = self._ssh_add_authorized_patcher.start()
+ self._ssh_add_authorized_mock.side_effect = \
+ self._ssh_file_manager.AddAuthorizedKeys
self._ssconf_mock = mock.Mock()
self._ssconf_mock.GetNodeList = mock.Mock()
@@ -969,15 +973,51 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
self._ssconf_mock.GetClusterName = mock.Mock()
self._run_cmd_mock = mock.Mock()
+ self._run_cmd_mock.side_effect = self._ssh_file_manager.RunCommand
self._ssh_remove_authorized_mock = \
self._ssh_remove_authorized_patcher.start()
+ self._ssh_remove_authorized_mock.side_effect = \
+ self._ssh_file_manager.RemoveAuthorizedKeys
+
+ self._ssh_add_public_key_patcher = testutils \
+ .patch_object(ssh, "AddPublicKey")
+ self._ssh_add_public_key_mock = \
+ self._ssh_add_public_key_patcher.start()
+ self._ssh_add_public_key_mock.side_effect = \
+ self._ssh_file_manager.AddPublicKey
+
+ self._ssh_remove_public_key_patcher = testutils \
+ .patch_object(ssh, "RemovePublicKey")
+ self._ssh_remove_public_key_mock = \
+ self._ssh_remove_public_key_patcher.start()
+ self._ssh_remove_public_key_mock.side_effect = \
+ self._ssh_file_manager.RemovePublicKey
+
+ self._ssh_query_pub_key_file_patcher = testutils \
+ .patch_object(ssh, "QueryPubKeyFile")
+ self._ssh_query_pub_key_file_mock = \
+ self._ssh_query_pub_key_file_patcher.start()
+ self._ssh_query_pub_key_file_mock.side_effect = \
+ self._ssh_file_manager.QueryPubKeyFile
+
+ self._ssh_replace_name_by_uuid_patcher = testutils \
+ .patch_object(ssh, "ReplaceNameByUuid")
+ self._ssh_replace_name_by_uuid_mock = \
+ self._ssh_replace_name_by_uuid_patcher.start()
+ self._ssh_replace_name_by_uuid_mock.side_effect = \
+ self._ssh_file_manager.ReplaceNameByUuid
+
self.noded_cert_file = testutils.TestDataFilename("cert1.pem")
def tearDown(self):
super(testutils.GanetiTestCase, self).tearDown()
self._ssh_add_authorized_patcher.stop()
self._ssh_remove_authorized_patcher.stop()
+ self._ssh_add_public_key_patcher.stop()
+ self._ssh_remove_public_key_patcher.stop()
+ self._ssh_query_pub_key_file_patcher.stop()
+ self._ssh_replace_name_by_uuid_patcher.stop()
def _SetupTestData(self, number_of_nodes=15, number_of_pot_mcs=5,
number_of_mcs=5):
@@ -996,21 +1036,15 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
self._ssconf_mock.GetClusterName.reset_mock()
self._run_cmd_mock.reset_mock()
- for i in range(number_of_nodes):
- node_name = "node_name_%s" % i
- node_uuid = "node_uuid_%s" % i
- self._ssh_port_map[node_name] = self._SSH_PORT
- self._all_nodes.append(node_name)
-
- if i in range(number_of_mcs + number_of_pot_mcs):
- ssh.AddPublicKey("node_uuid_%s" % i, "key%s" % i,
- key_file=self._pub_key_file)
- self._potential_master_candidates.append(node_name)
+ self._ssh_file_manager.InitAllNodes(15, 10, 5)
+ self._master_node = self._ssh_file_manager.GetMasterNodeName()
+ self._ssh_port_map = self._ssh_file_manager.GetSshPortMap(self._SSH_PORT)
+ self._potential_master_candidates = \
+ self._ssh_file_manager.GetAllPotentialMasterCandidateNodeNames()
+ self._master_candidate_uuids = \
+ self._ssh_file_manager.GetAllMasterCandidateUuids()
+ self._all_nodes = self._ssh_file_manager.GetAllNodeNames()
- if i in range(number_of_mcs):
- self._master_candidate_uuids.append(node_uuid)
-
- self._master_node = "node_name_%s" % (number_of_mcs / 2)
self._ssconf_mock.GetNodeList.return_value = self._all_nodes
def _TearDownTestData(self):
@@ -1066,6 +1100,8 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
test_node_uuid = "node_uuid_7"
self._SetupTestData()
+ ssh.AddPublicKey(test_node_uuid, "some_old_key",
+ key_file=self._pub_key_file)
backend._GenerateNodeSshKey(test_node_uuid, test_node_name,
self._ssh_port_map,
@@ -1080,103 +1116,143 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
for call in calls:
self.assertTrue(constants.SSHS_GENERATE in call)
- def testAddNodeSshKeyValid(self):
- new_node_name = "new_node_name"
- new_node_uuid = "new_node_uuid"
- new_node_key1 = "new_node_key1"
- new_node_key2 = "new_node_key2"
-
- for (to_authorized_keys, to_public_keys, get_public_keys) in \
- [(True, True, False), (False, True, False),
- (True, True, True), (False, True, True)]:
-
- self._SetupTestData()
+ def _AddNewNodeToTestData(self, name, uuid, key, pot_mc, mc, master):
+ self._ssh_file_manager.SetOrAddNode(name, uuid, key, pot_mc, mc, master)
- # set up public key file, ssconf store, and node lists
- if to_public_keys:
- for key in [new_node_key1, new_node_key2]:
- ssh.AddPublicKey(new_node_name, key, key_file=self._pub_key_file)
- self._potential_master_candidates.append(new_node_name)
-
- self._ssh_port_map[new_node_name] = self._SSH_PORT
-
- backend.AddNodeSshKey(new_node_uuid, new_node_name,
- self._potential_master_candidates,
- self._ssh_port_map,
- to_authorized_keys=to_authorized_keys,
- to_public_keys=to_public_keys,
- get_public_keys=get_public_keys,
- 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)
+ if pot_mc:
+ ssh.AddPublicKey(name, key, key_file=self._pub_key_file)
+ self._potential_master_candidates.append(name)
- calls_per_node = self._GetCallsPerNode()
+ self._ssh_port_map[name] = self._SSH_PORT
- # one sample node per type (master candidate, potential master candidate,
- # normal node)
- mc_idx = 3
- pot_mc_idx = 7
- normal_idx = 12
- sample_nodes = [mc_idx, pot_mc_idx, normal_idx]
- pot_sample_nodes = [mc_idx, pot_mc_idx]
+ def testAddMasterCandidate(self):
+ new_node_name = "new_node_name"
+ new_node_uuid = "new_node_uuid"
+ new_node_key = "new_node_key"
+ is_master_candidate = True
+ is_potential_master_candidate = True
+ is_master = False
- if to_authorized_keys:
- for node_idx in sample_nodes:
- self.assertTrue(self._KeyReceived(
- calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_AUTHORIZED_KEYS, new_node_key1),
- "Node %i did not receive authorized key '%s' although it should"
- " have." % (node_idx, new_node_key1))
- else:
- for node_idx in sample_nodes:
- self.assertFalse(self._KeyReceived(
- calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_AUTHORIZED_KEYS, new_node_key1),
- "Node %i received authorized key '%s', although it should not
have."
- % (node_idx, new_node_key1))
+ self._SetupTestData()
+ self._AddNewNodeToTestData(
+ new_node_name, new_node_uuid, new_node_key,
+ is_potential_master_candidate, is_master_candidate,
+ is_master)
+
+ backend.AddNodeSshKey(new_node_uuid, new_node_name,
+ self._potential_master_candidates,
+ self._ssh_port_map,
+ to_authorized_keys=is_master_candidate,
+ to_public_keys=is_potential_master_candidate,
+ get_public_keys=is_potential_master_candidate,
+ 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)
+
+ self._ssh_file_manager.PotentialMasterCandidatesOnlyHavePublicKey(
+ new_node_name)
+ self.assertTrue(self._ssh_file_manager.AllNodesHaveAuthorizedKey(
+ new_node_key))
+
+ self._TearDownTestData()
+
+ def testAddPotentialMasterCandidate(self):
+ new_node_name = "new_node_name"
+ new_node_uuid = "new_node_uuid"
+ new_node_key = "new_node_key"
+ is_master_candidate = False
+ is_potential_master_candidate = True
+ is_master = False
- if to_public_keys:
- for node_idx in pot_sample_nodes:
- self.assertTrue(self._KeyReceived(
- calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_PUBLIC_KEYS, new_node_key1),
- "Node %i did not receive public key '%s', although it should have."
- % (node_idx, new_node_key1))
- else:
- for node_idx in sample_nodes:
- self.assertFalse(self._KeyReceived(
- calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_PUBLIC_KEYS, new_node_key1),
- "Node %i did receive public key '%s', although it should have."
- % (node_idx, new_node_key1))
+ self._SetupTestData()
+ self._AddNewNodeToTestData(
+ new_node_name, new_node_uuid, new_node_key,
+ is_potential_master_candidate, is_master_candidate,
+ is_master)
+
+ backend.AddNodeSshKey(new_node_uuid, new_node_name,
+ self._potential_master_candidates,
+ self._ssh_port_map,
+ to_authorized_keys=is_master_candidate,
+ to_public_keys=is_potential_master_candidate,
+ get_public_keys=is_potential_master_candidate,
+ 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)
+
+ self._ssh_file_manager.PotentialMasterCandidatesOnlyHavePublicKey(
+ new_node_name)
+ self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey(
+ new_node_key))
+
+ self._TearDownTestData()
+
+ def testAddNormalNode(self):
+ new_node_name = "new_node_name"
+ new_node_uuid = "new_node_uuid"
+ new_node_key = "new_node_key"
+ is_master_candidate = False
+ is_potential_master_candidate = False
+ is_master = False
- if get_public_keys:
- for node_idx in sample_nodes:
- if node_idx in pot_sample_nodes:
- self.assertTrue(self._KeyReceived(
- calls_per_node, new_node_name,
- constants.SSHS_SSH_PUBLIC_KEYS, "key%s" % node_idx),
- "The new node '%s' did not receive public key of node %i,"
- " although it should have." %
- (new_node_name, node_idx))
- else:
- self.assertFalse(self._KeyReceived(
- calls_per_node, new_node_name,
- constants.SSHS_SSH_PUBLIC_KEYS, "key%s" % node_idx),
- "The new node '%s' did receive public key of node %i,"
- " although it should not have." %
- (new_node_name, node_idx))
- else:
- new_node_name not in calls_per_node
+ self._SetupTestData()
+ self._AddNewNodeToTestData(
+ new_node_name, new_node_uuid, new_node_key,
+ is_potential_master_candidate, is_master_candidate,
+ is_master)
+
+ self.assertRaises(
+ AssertionError, backend.AddNodeSshKey, new_node_uuid, new_node_name,
+ self._potential_master_candidates, self._ssh_port_map,
+ to_authorized_keys=is_master_candidate,
+ to_public_keys=is_potential_master_candidate,
+ get_public_keys=is_potential_master_candidate,
+ 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)
+
+ self.assertTrue(self._ssh_file_manager.NoNodeHasPublicKey(
+ new_node_uuid, new_node_key))
+ self.assertTrue(self._ssh_file_manager.NoNodeHasAuthorizedKey(
+ new_node_key))
+
+ self._TearDownTestData()
+
+ def testPromoteToMasterCandidate(self):
+ self._SetupTestData()
- self._TearDownTestData()
+ # Get one of the potential master candidates
+ node_name, node_uuid, node_key, pot_mc, mc, master = \
+ self._ssh_file_manager.GetAllPotentialMasterCandidates()[0]
+ # Update it's role to master candidate in the test data
+ self._ssh_file_manager.SetOrAddNode(node_name, node_uuid, node_key,
+ pot_mc, True, master)
+
+ backend.AddNodeSshKey(node_uuid, node_name,
+ self._potential_master_candidates,
+ self._ssh_port_map,
+ to_authorized_keys=True,
+ to_public_keys=False,
+ get_public_keys=False,
+ 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)
+
+ self._ssh_file_manager.PotentialMasterCandidatesOnlyHavePublicKey(
+ node_name)
+ self.assertTrue(self._ssh_file_manager.AllNodesHaveAuthorizedKey(
+ node_key))
+
+ self._TearDownTestData()
def testRemoveNodeSshKeyValid(self):
node_name = "node_name"
node_uuid = "node_uuid"
- node_key1 = "node_key1"
- node_key2 = "node_key2"
+ node_key = "node_key"
for (from_authorized_keys, from_public_keys,
clear_authorized_keys, clear_public_keys) in \
@@ -1193,11 +1269,10 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
# set up public key file, ssconf store, and node lists
if from_public_keys or from_authorized_keys:
- for key in [node_key1, node_key2]:
- ssh.AddPublicKey(node_uuid, key, key_file=self._pub_key_file)
+ ssh.AddPublicKey(node_uuid, node_key, key_file=self._pub_key_file)
self._potential_master_candidates.append(node_name)
if from_authorized_keys:
- ssh.AddAuthorizedKeys(self._pub_key_file, [node_key1, node_key2])
+ ssh.AddAuthorizedKeys(self._pub_key_file, [node_key])
self._ssh_port_map[node_name] = self._SSH_PORT
@@ -1231,42 +1306,42 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
for node_idx in sample_nodes:
self.assertTrue(self._KeyRemoved(
calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_AUTHORIZED_KEYS, node_key1),
+ constants.SSHS_SSH_AUTHORIZED_KEYS, node_key),
"Node %i did not get request to remove authorized key '%s'"
- " although it should have." % (node_idx, node_key1))
+ " although it should have." % (node_idx, node_key))
else:
for node_idx in sample_nodes:
self.assertFalse(self._KeyRemoved(
calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_AUTHORIZED_KEYS, node_key1),
+ constants.SSHS_SSH_AUTHORIZED_KEYS, node_key),
"Node %i got requested to remove authorized key '%s', although it"
- " should not have." % (node_idx, node_key1))
+ " should not have." % (node_idx, node_key))
if from_public_keys:
for node_idx in pot_sample_nodes:
self.assertTrue(self._KeyRemoved(
calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_PUBLIC_KEYS, node_key1),
+ constants.SSHS_SSH_PUBLIC_KEYS, node_key),
"Node %i did not receive request to remove public key '%s',"
- " although it should have." % (node_idx, node_key1))
+ " although it should have." % (node_idx, node_key))
self.assertTrue(self._KeyRemoved(
calls_per_node, node_name,
- constants.SSHS_SSH_PUBLIC_KEYS, node_key1),
+ constants.SSHS_SSH_PUBLIC_KEYS, node_key),
"Node %s did not receive request to remove its own public key '%s',"
- " although it should have." % (node_name, node_key1))
+ " although it should have." % (node_name, node_key))
for node_idx in list(set(sample_nodes) - set(pot_sample_nodes)):
self.assertFalse(self._KeyRemoved(
calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_PUBLIC_KEYS, node_key1),
+ constants.SSHS_SSH_PUBLIC_KEYS, node_key),
"Node %i received a request to remove public key '%s',"
- " although it should not have." % (node_idx, node_key1))
+ " although it should not have." % (node_idx, node_key))
else:
for node_idx in sample_nodes:
self.assertFalse(self._KeyRemoved(
calls_per_node, "node_name_%i" % node_idx,
- constants.SSHS_SSH_PUBLIC_KEYS, node_key1),
+ constants.SSHS_SSH_PUBLIC_KEYS, node_key),
"Node %i received a request to remove public key '%s',"
- " although it should not have." % (node_idx, node_key1))
+ " although it should not have." % (node_idx, node_key))
if clear_authorized_keys:
for node_idx in list(set(sample_nodes) - set([mc_idx])):
--
2.2.0.rc0.207.ga3a616c
