On Thu, Apr 16, 2015 at 05:31:58PM +0200, 'Helga Velroyen' via ganeti-devel
wrote:
So far the backend function NodeSshKeyAdd naively assumed
that all nodes are online and reachable via SSH. This
patch adds offline status information to the function's
signature to respect the offline status when updating
SSH keys.
Signed-off-by: Helga Velroyen <[email protected]>
---
lib/backend.py | 4 ++++
test/py/ganeti.backend_unittest.py | 41 ++++++++++++++++++++++++++++++++++++++
test/py/testutils_ssh.py | 4 ++++
3 files changed, 49 insertions(+)
diff --git a/lib/backend.py b/lib/backend.py
index 04b8350..b5f4001 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -1544,10 +1544,14 @@ def AddNodeSshKey(node_uuid, node_name,
all_nodes = ssconf_store.GetNodeList()
master_node = ssconf_store.GetMasterNode()
+ online_nodes = ssconf_store.GetOnlineNodeList()
for node in all_nodes:
if node == master_node:
continue
+ if node not in online_nodes:
+ logging.debug("Skipping offline node '%s'.", node)
+ continue
if node in potential_master_candidates:
run_cmd_fn(cluster_name, node, pathutils.SSH_UPDATE,
ssh_port_map.get(node), pot_mc_data,
diff --git a/test/py/ganeti.backend_unittest.py
b/test/py/ganeti.backend_unittest.py
index cd2e24c..56200ea 100755
--- a/test/py/ganeti.backend_unittest.py
+++ b/test/py/ganeti.backend_unittest.py
@@ -971,6 +971,7 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
self._ssconf_mock.GetNodeList = mock.Mock()
self._ssconf_mock.GetMasterNode = mock.Mock()
self._ssconf_mock.GetClusterName = mock.Mock()
+ self._ssconf_mock.GetOnlineNodeList = mock.Mock()
self._run_cmd_mock = mock.Mock()
self._run_cmd_mock.side_effect = self._ssh_file_manager.RunCommand
@@ -1034,6 +1035,7 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
self._ssconf_mock.GetNodeList.reset_mock()
self._ssconf_mock.GetMasterNode.reset_mock()
self._ssconf_mock.GetClusterName.reset_mock()
+ self._ssconf_mock.GetOnlineNodeList.reset_mock()
self._run_cmd_mock.reset_mock()
self._ssh_file_manager.InitAllNodes(15, 10, 5)
@@ -1046,6 +1048,7 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
self._all_nodes = self._ssh_file_manager.GetAllNodeNames()
self._ssconf_mock.GetNodeList.return_value = self._all_nodes
+ self._ssconf_mock.GetOnlineNodeList.return_value = self._all_nodes
def _TearDownTestData(self):
os.remove(self._pub_key_file)
@@ -1355,6 +1358,44 @@ class
TestAddRemoveGenerateNodeSshKey(testutils.GanetiTestCase):
self._TearDownTestData()
+ def testAddKeyWithOfflineNodes(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
+
+ self._SetupTestData()
+ self._AddNewNodeToTestData(
+ new_node_name, new_node_uuid, new_node_key,
+ is_potential_master_candidate, is_master_candidate,
+ is_master)
+
+ # 'randomly' mark some nodes as offline
+ self._online_nodes = [name for name in self._all_nodes
+ if '3' not in name and '5' not in name]
+ self._ssconf_mock.GetOnlineNodeList.return_value = self._online_nodes
+ 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)
+
+ for node in self._all_nodes:
+ if node in self._online_nodes:
+ self.assertTrue(self._ssh_file_manager.NodeHasAuthorizedKey(
+ node, new_node_key))
+ else:
+ self.assertFalse(self._ssh_file_manager.NodeHasAuthorizedKey(
+ node, new_node_key))
+
+ self._TearDownTestData()
class TestVerifySshSetup(testutils.GanetiTestCase):
diff --git a/test/py/testutils_ssh.py b/test/py/testutils_ssh.py
index 0220c50..a65c44e 100644
--- a/test/py/testutils_ssh.py
+++ b/test/py/testutils_ssh.py
@@ -150,6 +150,10 @@ class FakeSshFileManager(object):
port_map[node] = port
return port_map
+ def GetOfflineMap(self):
+ """Returns a dict indicating that all nodes are online."""
+ return dict([(name, False) for name in self._all_node_data.keys()])
This function doesn't seem to be used anywhere. Perhaps something left after
a refactoring?
+
def GetAllNodeNames(self):
return self._all_node_data.keys()
--
2.2.0.rc0.207.ga3a616c
Rest LGTM, thanks
