On Thu, Sep 25, 2014 at 4:53 PM, Petr Pudlak <[email protected]> wrote:

> On Tue, Sep 02, 2014 at 04:19:39PM +0200, 'Helga Velroyen' via
> ganeti-devel wrote:
>
>> This patch adjusts the SSH connectivity test that
>> 'gnt-cluster verify' does and introduces a couple of
>> sanity checks for the new SSH setup with individual
>> keys.
>>
>> Note that it won't be possible for this to always hold
>> through the entire patch series. I decided to put it in
>> anyway, because it a great debugging tool during the
>> development itself as keeping track of the states of
>> various key files is tedious manual work.
>>
>> Signed-off-by: Helga Velroyen <[email protected]>
>> ---
>> lib/backend.py                     | 114 ++++++++++++++++++++++++++++++
>> +++++--
>> lib/cmdlib/cluster.py              |  50 +++++++++++++++-
>> lib/cmdlib/node.py                 |   2 +-
>> lib/ssh.py                         |  28 +++++++++
>> src/Ganeti/Constants.hs            |   3 +
>> test/py/cmdlib/cluster_unittest.py |  67 ++++++++++++++--------
>> test/py/ganeti.backend_unittest.py |  88 ++++++++++++++++++++++++++++
>> test/py/ganeti.ssh_unittest.py     |   5 ++
>> 8 files changed, 326 insertions(+), 31 deletions(-)
>>
>> diff --git a/lib/backend.py b/lib/backend.py
>> index d9526c4..717298c 100644
>> --- a/lib/backend.py
>> +++ b/lib/backend.py
>> @@ -948,6 +948,100 @@ def _VerifyClientCertificate(cert_
>> file=pathutils.NODED_CLIENT_CERT_FILE):
>>     return (None, utils.GetCertificateDigest(cert_filename=cert_file))
>>
>>
>> +def _VerifySshSetup(node_status_list, my_name):
>> +  """Verifies the state of the SSH key files.
>> +
>> +  @type node_status_list: list of tuples
>> +  @param node_status_list: list of nodes of the cluster associated with a
>> +    couple of flags: (uuid, name, is_master_candidate,
>> +    is_potential_master_candidate, online)
>> +  @type my_name: str
>> +  @param my_name: name of this node
>> +
>> +  """
>> +  if node_status_list is None:
>> +    return ["No node list to check against the pub_key_file received."]
>> +
>> +  my_status_list = [(my_uuid, name, mc, pot_mc) for (my_uuid, name, mc,
>> pot_mc)
>> +                    in node_status_list if name == my_name]
>> +  if len(my_status_list) < 0:
>>
>
> This should be ... `== 0:`
>
>
>  +    return ["Cannot find node information for node '%s'." % my_name]
>> +  (my_uuid, _, _, potential_master_candidate) = \
>> +     my_status_list[0]
>> +
>> +  result = []
>> +  pot_mc_uuids = [uuid for (uuid, _, _, _) in node_status_list]
>> +  pub_keys = ssh.QueryPubKeyFile(None)
>> +
>> +  if potential_master_candidate:
>> +    # Check that the set of potential master candidates matches the
>> +    # public key file
>> +    pub_uuids_set = set(pub_keys.keys())
>> +    pot_mc_uuids_set = set(pot_mc_uuids)
>> +    missing_uuids = set([])
>> +    if pub_uuids_set != pot_mc_uuids_set:
>> +      unknown_uuids = pub_uuids_set - pot_mc_uuids_set
>> +      if unknown_uuids:
>> +        result.append("The following node UUIDs are listed in the public
>> key"
>> +                      " file on node '%s', but are not potential master"
>> +                      " candidates: %s."
>> +                      % (my_name, ", ".join(list(unknown_uuids))))
>> +      missing_uuids = pot_mc_uuids_set - pub_uuids_set
>> +      if missing_uuids:
>> +        result.append("The following node UUIDs of potential master
>> candidates"
>> +                      " are missing in the public key file on node %s:
>> %s."
>> +                      % (my_name, ", ".join(list(missing_uuids))))
>> +
>> +    (_, key_files) = \
>> +      ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=False,
>> dircheck=False)
>> +    my_keys = pub_keys[my_uuid]
>> +    num_keys = 0
>> +    for (key_type, (_, pub_key_file)) in key_files.items():
>> +      try:
>> +        pub_key = utils.ReadFile(pub_key_file)
>> +        if pub_key.strip() not in my_keys:
>> +          result.append("The %s key of node %s does not match this
>> node's keys"
>> +                        " in the pub key file." % (key_type, my_name))
>> +        num_keys += 1
>> +      except IOError:
>> +        # There might not be keys of every type.
>> +        pass
>> +    if num_keys != len(my_keys):
>> +      result.append("The number of keys for node %s in the public key
>> file"
>> +                    " (%s) does not match the number of keys on the node"
>> +                    " (%s)." % (my_name, len(my_keys), len(key_files)))
>> +  else:
>> +    if len(pub_keys.keys()) > 0:
>> +      result.append("The public key file of node '%s' is not empty,
>> although"
>> +                    " the node is not a potential master candidate."
>> +                    % my_name)
>> +
>> +  # Check that all master candidate keys are in the authorized_keys file
>> +  (auth_key_file, _) = \
>> +    ssh.GetAllUserFiles(constants.SSH_LOGIN_USER, mkdir=False,
>> dircheck=False)
>> +  for (uuid, name, mc, _) in node_status_list:
>> +    if uuid in missing_uuids:
>> +      continue
>> +    if mc:
>> +      for key in pub_keys[uuid]:
>> +        if not ssh.HasAuthorizedKey(auth_key_file, key):
>> +          result.append("A SSH key of master candidate '%s' (UUID: '%s')
>> is"
>> +                        " not in the 'authorized_keys' file of node
>> '%s'."
>> +                        % (name, uuid, my_name))
>> +    else:
>> +      for key in pub_keys[uuid]:
>> +        if name != my_name and ssh.HasAuthorizedKey(auth_key_file, key):
>> +          result.append("A SSH key of normal node '%s' (UUID: '%s') is
>> in the"
>> +                        " 'authorized_keys' file of node '%s'."
>> +                        % (name, uuid, my_name))
>> +        if name == my_name and not ssh.HasAuthorizedKey(auth_key_file,
>> key):
>> +          result.append("A SSH key of normal node '%s' (UUID: '%s') is
>> not"
>> +                        " in the 'authorized_keys' file of itself."
>> +                        % (my_name, uuid))
>> +
>> +  return result
>> +
>> +
>> def VerifyNode(what, cluster_name, all_hvparams, node_groups, groups_cfg):
>>   """Verify the status of the local node.
>>
>> @@ -1004,8 +1098,12 @@ def VerifyNode(what, cluster_name, all_hvparams,
>> node_groups, groups_cfg):
>>   if constants.NV_CLIENT_CERT in what:
>>     result[constants.NV_CLIENT_CERT] = _VerifyClientCertificate()
>>
>> +  if constants.NV_SSH_SETUP in what:
>> +    result[constants.NV_SSH_SETUP] = \
>> +      _VerifySshSetup(what[constants.NV_SSH_SETUP], my_name)
>> +
>>   if constants.NV_NODELIST in what:
>> -    (nodes, bynode) = what[constants.NV_NODELIST]
>> +    (nodes, bynode, mcs) = what[constants.NV_NODELIST]
>>
>>     # Add nodes from other groups (different for each node)
>>     try:
>> @@ -1023,10 +1121,16 @@ def VerifyNode(what, cluster_name, all_hvparams,
>> node_groups, groups_cfg):
>>       ssh_port = params["ndparams"].get(constants.ND_SSH_PORT)
>>       logging.debug("Ssh port %s (None = default) for node %s",
>>                     str(ssh_port), node)
>> -      success, message = _GetSshRunner(cluster_name). \
>> -                            VerifyNodeHostname(node, ssh_port)
>> -      if not success:
>> -        val[node] = message
>> +
>> +      # We only test if master candidates can communicate to other nodes.
>> +      # We cannot test if normal nodes cannot communicate with other
>> nodes,
>> +      # because the administrator might have installed additional SSH
>> keys,
>> +      # over which Ganeti has no power.
>> +      if my_name in mcs:
>> +        success, message = _GetSshRunner(cluster_name). \
>> +                              VerifyNodeHostname(node, ssh_port)
>> +        if not success:
>> +          val[node] = message
>>
>>     result[constants.NV_NODELIST] = val
>>
>> diff --git a/lib/cmdlib/cluster.py b/lib/cmdlib/cluster.py
>> index 915c418..d0c54de 100644
>> --- a/lib/cmdlib/cluster.py
>> +++ b/lib/cmdlib/cluster.py
>> @@ -2682,6 +2682,27 @@ class LUClusterVerifyGroup(LogicalUnit,
>> _VerifyErrors):
>>             " certificate of another node which is master candidate.",
>>             node.uuid)
>>
>> +  def _VerifySshSetup(self, nodes, all_nvinfo):
>> +    """Evaluates the verification results of the SSH setup.
>> +
>> +    @param nodes: List of L{objects.Node} objects
>> +    @param all_nvinfo: RPC results
>> +
>> +    """
>> +    for node in nodes:
>> +      if not node.offline:
>> +        nresult = all_nvinfo[node.uuid]
>> +        if nresult.fail_msg or not nresult.payload:
>> +          self._ErrorIf(True, constants.CV_ENODESSH, node.name,
>> +                        "Could not verify the SSH setup of this node.")
>> +          return
>> +        result = nresult.payload.get(constants.NV_SSH_SETUP, None)
>> +        error_msg = ""
>> +        if isinstance(result, list):
>> +          error_msg = " ".join(result)
>> +        self._ErrorIf(result,
>> +                      constants.CV_ENODESSH, None, error_msg)
>> +
>>   def _VerifyFiles(self, nodes, master_node_uuid, all_nvinfo,
>>                    (files_all, files_opt, files_mc, files_vm)):
>>     """Verifies file checksums collected from all nodes.
>> @@ -3286,17 +3307,38 @@ class LUClusterVerifyGroup(LogicalUnit,
>> _VerifyErrors):
>>     We will make nodes contact all nodes in their group, and one node from
>>     every other group.
>>
>> +    @rtype: tuple of (string, dict of strings to list of strings, string)
>> +    @return: a tuple containing the list of all online nodes, a
>> dictionary
>> +      mapping node names to additional nodes of other node groups to
>> which
>> +      connectivity should be tested, and a list of all online master
>> +      candidates
>> +
>>     @warning: This algorithm has a known issue if one node group is much
>>       smaller than others (e.g. just one node). In such a case all other
>>       nodes will talk to the single node.
>>
>>     """
>>     online_nodes = sorted(node.name for node in group_nodes if not
>> node.offline)
>> +    online_mcs = sorted(node.name for node in group_nodes
>> +                        if (node.master_candidate and not node.offline))
>>     sel = cls._SshNodeSelector(group_uuid, all_nodes)
>>
>>     return (online_nodes,
>>             dict((name, sorted([i.next() for i in sel]))
>> -                 for name in online_nodes))
>> +                 for name in online_nodes),
>> +            online_mcs)
>> +
>> +  def _PrepareSshSetupCheck(self):
>> +    """Prepare the input data for the SSH setup verification.
>> +
>> +    """
>> +    all_nodes_info = self.cfg.GetAllNodesInfo()
>> +    potential_master_candidates = self.cfg.
>> GetPotentialMasterCandidates()
>> +    node_status = [
>> +      (uuid, node_info.name, node_info.master_candidate,
>> +       node_info.name in potential_master_candidates)
>> +      for (uuid, node_info) in all_nodes_info.items()]
>> +    return node_status
>>
>>   def BuildHooksEnv(self):
>>     """Build hooks env.
>> @@ -3391,6 +3433,9 @@ class LUClusterVerifyGroup(LogicalUnit,
>> _VerifyErrors):
>>       constants.NV_CLIENT_CERT: None,
>>       }
>>
>> +    if self.cfg.GetClusterInfo().modify_ssh_setup:
>> +      node_verify_param[constants.NV_SSH_SETUP] =
>> self._PrepareSshSetupCheck()
>> +
>>     if vg_name is not None:
>>       node_verify_param[constants.NV_VGLIST] = None
>>       node_verify_param[constants.NV_LVLIST] = vg_name
>> @@ -3573,7 +3618,8 @@ class LUClusterVerifyGroup(LogicalUnit,
>> _VerifyErrors):
>>     feedback_fn("* Verifying configuration file consistency")
>>
>>     self._VerifyClientCertificates(self.my_node_info.values(),
>> all_nvinfo)
>> -
>> +    if self.cfg.GetClusterInfo().modify_ssh_setup:
>> +      self._VerifySshSetup(self.my_node_info.values(), all_nvinfo)
>>     self._VerifyFiles(vf_node_info, master_node_uuid, vf_nvinfo, filemap)
>>
>>     feedback_fn("* Verifying node status")
>> diff --git a/lib/cmdlib/node.py b/lib/cmdlib/node.py
>> index 9badcba..823ebb6 100644
>> --- a/lib/cmdlib/node.py
>> +++ b/lib/cmdlib/node.py
>> @@ -420,7 +420,7 @@ class LUNodeAdd(LogicalUnit):
>>
>>     node_verifier_uuids = [self.cfg.GetMasterNode()]
>>     node_verify_param = {
>> -      constants.NV_NODELIST: ([self.new_node.name], {}),
>> +      constants.NV_NODELIST: ([self.new_node.name], {}, []),
>>       # TODO: do a node-net-test as well?
>>     }
>>
>> diff --git a/lib/ssh.py b/lib/ssh.py
>> index 78adce6..7e2f64c 100644
>> --- a/lib/ssh.py
>> +++ b/lib/ssh.py
>> @@ -173,6 +173,34 @@ def AddAuthorizedKeys(file_obj, keys):
>>     f.close()
>>
>>
>> +def HasAuthorizedKey(file_obj, key):
>> +  """Check if a particular key is in the 'authorized_keys' file.
>> +
>> +  @type file_obj: str or file handle
>> +  @param file_obj: path to authorized_keys file
>> +  @type key: str
>> +  @param key: string containing key
>> +
>> +  """
>> +  key_fields = _SplitSshKey(key)
>> +
>> +  if isinstance(file_obj, basestring):
>> +    f = open(file_obj, "r")
>> +  else:
>> +    f = file_obj
>> +
>> +  try:
>> +    for line in f:
>> +      # Ignore whitespace changes
>> +      line_key = _SplitSshKey(line)
>> +      if line_key == key_fields:
>> +        return True
>> +  finally:
>> +    f.close()
>> +
>> +  return False
>> +
>> +
>> def AddAuthorizedKey(file_obj, key):
>>   """Adds an SSH public key to an authorized_keys file.
>>
>> diff --git a/src/Ganeti/Constants.hs b/src/Ganeti/Constants.hs
>> index 36227e6..4a2db43 100644
>> --- a/src/Ganeti/Constants.hs
>> +++ b/src/Ganeti/Constants.hs
>> @@ -3209,6 +3209,9 @@ nvVglist = "vglist"
>> nvVmnodes :: String
>> nvVmnodes = "vmnodes"
>>
>> +nvSshSetup :: String
>> +nvSshSetup = "ssh-setup"
>> +
>> -- * Instance status
>>
>> inststAdmindown :: String
>> diff --git a/test/py/cmdlib/cluster_unittest.py b/test/py/cmdlib/cluster_
>> unittest.py
>> index d0a6542..fa0e655 100644
>> --- a/test/py/cmdlib/cluster_unittest.py
>> +++ b/test/py/cmdlib/cluster_unittest.py
>> @@ -50,29 +50,46 @@ class TestClusterVerifySsh(unittest.TestCase):
>>   def testMultipleGroups(self):
>>     fn = cluster.LUClusterVerifyGroup._SelectSshCheckNodes
>>     mygroupnodes = [
>> -      objects.Node(name="node20", group="my", offline=False),
>> -      objects.Node(name="node21", group="my", offline=False),
>> -      objects.Node(name="node22", group="my", offline=False),
>> -      objects.Node(name="node23", group="my", offline=False),
>> -      objects.Node(name="node24", group="my", offline=False),
>> -      objects.Node(name="node25", group="my", offline=False),
>> -      objects.Node(name="node26", group="my", offline=True),
>> +      objects.Node(name="node20", group="my", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node21", group="my", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node22", group="my", offline=False,
>> +                   master_candidate=False),
>> +      objects.Node(name="node23", group="my", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node24", group="my", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node25
>
> diff --git a/lib/backend.py b/lib/backend.py
>
> index 509e99b..8f48f9f 100644
>
> --- a/lib/backend.py
>
> +++ b/lib/backend.py
>
> @@ -973,7 +973,7 @@ def _VerifySshSetup(node_status_list, my_name):
>
>
>
>    my_status_list = [(my_uuid, name, mc, pot_mc) for (my_uuid, name, mc,
>> pot_mc)
>
>                      in node_status_list if name == my_name]
>
> -  if len(my_status_list) < 0:
>
> +  if len(my_status_list) == 0:
>
>      return ["Cannot find node information for node '%s'." % my_name]
>
>    (my_uuid, _, _, potential_master_candidate) = \
>
>       my_status_list[0]
>
> ", group="my", offline=False,
>> +                   master_candidate=False),
>> +      objects.Node(name="node26", group="my", offline=True,
>> +                   master_candidate=True),
>>       ]
>>     nodes = [
>> -      objects.Node(name="node1", group="g1", offline=True),
>> -      objects.Node(name="node2", group="g1", offline=False),
>> -      objects.Node(name="node3", group="g1", offline=False),
>> -      objects.Node(name="node4", group="g1", offline=True),
>> -      objects.Node(name="node5", group="g1", offline=False),
>> -      objects.Node(name="node10", group="xyz", offline=False),
>> -      objects.Node(name="node11", group="xyz", offline=False),
>> -      objects.Node(name="node40", group="alloff", offline=True),
>> -      objects.Node(name="node41", group="alloff", offline=True),
>> -      objects.Node(name="node50", group="aaa", offline=False),
>> +      objects.Node(name="node1", group="g1", offline=True,
>> +                   master_candidate=True),
>> +      objects.Node(name="node2", group="g1", offline=False,
>> +                   master_candidate=False),
>> +      objects.Node(name="node3", group="g1", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node4", group="g1", offline=True,
>> +                   master_candidate=True),
>> +      objects.Node(name="node5", group="g1", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node10", group="xyz", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node11", group="xyz", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node40", group="alloff", offline=True,
>> +                   master_candidate=True),
>> +      objects.Node(name="node41", group="alloff", offline=True,
>> +                   master_candidate=True),
>> +      objects.Node(name="node50", group="aaa", offline=False,
>> +                   master_candidate=True),
>>       ] + mygroupnodes
>>     assert not utils.FindDuplicates(map(operator.attrgetter("name"),
>> nodes))
>>
>> -    (online, perhost) = fn(mygroupnodes, "my", nodes)
>> +    (online, perhost, _) = fn(mygroupnodes, "my", nodes)
>>     self.assertEqual(online, ["node%s" % i for i in range(20, 26)])
>>     self.assertEqual(set(perhost.keys()), set(online))
>>
>> @@ -88,14 +105,18 @@ class TestClusterVerifySsh(unittest.TestCase):
>>   def testSingleGroup(self):
>>     fn = cluster.LUClusterVerifyGroup._SelectSshCheckNodes
>>     nodes = [
>> -      objects.Node(name="node1", group="default", offline=True),
>> -      objects.Node(name="node2", group="default", offline=False),
>> -      objects.Node(name="node3", group="default", offline=False),
>> -      objects.Node(name="node4", group="default", offline=True),
>> +      objects.Node(name="node1", group="default", offline=True,
>> +                   master_candidate=True),
>> +      objects.Node(name="node2", group="default", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node3", group="default", offline=False,
>> +                   master_candidate=True),
>> +      objects.Node(name="node4", group="default", offline=True,
>> +                   master_candidate=True),
>>       ]
>>     assert not utils.FindDuplicates(map(operator.attrgetter("name"),
>> nodes))
>>
>> -    (online, perhost) = fn(nodes, "default", nodes)
>> +    (online, perhost, _) = fn(nodes, "default", nodes)
>>     self.assertEqual(online, ["node2", "node3"])
>>     self.assertEqual(set(perhost.keys()), set(online))
>>
>> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/ganeti.backend_
>> unittest.py
>> index 6e54273..9b18763 100755
>> --- a/test/py/ganeti.backend_unittest.py
>> +++ b/test/py/ganeti.backend_unittest.py
>> @@ -21,6 +21,7 @@
>>
>> """Script for testing ganeti.backend"""
>>
>> +import copy
>> import mock
>> import os
>> import shutil
>> @@ -1275,7 +1276,94 @@ class TestVerifySshSetup(testutils.
>> GanetiTestCase):
>>   _NODE2_NAME = "name2"
>>   _NODE3_NAME = "name3"
>>   _NODE1_KEYS = ["key11", "key12"]
>> +  _NODE2_KEYS = ["key21"]
>> +  _NODE3_KEYS = ["key31"]
>> +
>> +  _NODE_STATUS_LIST = [
>> +    (_NODE1_UUID, _NODE1_NAME, True, True),
>> +    (_NODE2_UUID, _NODE2_NAME, False, True),
>> +    (_NODE3_UUID, _NODE3_NAME, False, False),
>> +    ]
>> +
>> +  _PUB_KEY_RESULT = {
>> +    _NODE1_UUID: _NODE1_KEYS,
>> +    _NODE2_UUID: _NODE2_KEYS,
>> +    _NODE3_UUID: _NODE3_KEYS,
>> +    }
>> +
>> +  _AUTH_RESULT = {
>> +    _NODE1_KEYS[0]: True,
>> +    _NODE1_KEYS[1]: True,
>> +    _NODE2_KEYS[0]: False,
>> +    _NODE3_KEYS[0]: False,
>> +  }
>>
>> +  def setUp(self):
>> +    testutils.GanetiTestCase.setUp(self)
>> +    self._has_authorized_patcher = testutils \
>> +      .patch_object(ssh, "HasAuthorizedKey")
>> +    self._has_authorized_mock = self._has_authorized_patcher.start()
>> +    self._query_patcher = testutils \
>> +      .patch_object(ssh, "QueryPubKeyFile")
>> +    self._query_mock = self._query_patcher.start()
>> +    self._read_file_patcher = testutils \
>> +      .patch_object(utils, "ReadFile")
>> +    self._read_file_mock = self._read_file_patcher.start()
>> +    self._read_file_mock.return_value = self._NODE1_KEYS[0]
>> +
>> +  def tearDown(self):
>> +    super(testutils.GanetiTestCase, self).tearDown()
>> +    self._has_authorized_patcher.stop()
>> +    self._query_patcher.stop()
>> +    self._read_file_patcher.stop()
>> +
>> +  def testValidData(self):
>> +    self._has_authorized_mock.side_effect = \
>> +      lambda _, key : self._AUTH_RESULT[key]
>> +    self._query_mock.return_value = self._PUB_KEY_RESULT
>> +    result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
>> +                                     self._NODE1_NAME)
>> +    self.assertEqual(result, [])
>> +
>> +  def testMissingKey(self):
>> +    self._has_authorized_mock.side_effect = \
>> +      lambda _, key : self._AUTH_RESULT[key]
>> +    pub_key_missing = copy.deepcopy(self._PUB_KEY_RESULT)
>> +    del pub_key_missing[self._NODE2_UUID]
>> +    self._query_mock.return_value = pub_key_missing
>> +    result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
>> +                                     self._NODE1_NAME)
>> +    self.assertTrue(self._NODE2_UUID in result[0])
>> +
>> +  def testUnknownKey(self):
>> +    self._has_authorized_mock.side_effect = \
>> +      lambda _, key : self._AUTH_RESULT[key]
>> +    pub_key_missing = copy.deepcopy(self._PUB_KEY_RESULT)
>> +    pub_key_missing["unkownnodeuuid"] = "pinkbunny"
>> +    self._query_mock.return_value = pub_key_missing
>> +    result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
>> +                                     self._NODE1_NAME)
>> +    self.assertTrue("unkownnodeuuid" in result[0])
>> +
>> +  def testMissingMasterCandidate(self):
>> +    auth_result = copy.deepcopy(self._AUTH_RESULT)
>> +    auth_result["key12"] = False
>> +    self._has_authorized_mock.side_effect = \
>> +      lambda _, key : auth_result[key]
>> +    self._query_mock.return_value = self._PUB_KEY_RESULT
>> +    result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
>> +                                     self._NODE1_NAME)
>> +    self.assertTrue(self._NODE1_UUID in result[0])
>> +
>> +  def testSuperfluousNormalNode(self):
>> +    auth_result = copy.deepcopy(self._AUTH_RESULT)
>> +    auth_result["key31"] = True
>> +    self._has_authorized_mock.side_effect = \
>> +      lambda _, key : auth_result[key]
>> +    self._query_mock.return_value = self._PUB_KEY_RESULT
>> +    result = backend._VerifySshSetup(self._NODE_STATUS_LIST,
>> +                                     self._NODE1_NAME)
>> +    self.assertTrue(self._NODE3_UUID in result[0])
>>
>>
>> if __name__ == "__main__":
>> diff --git a/test/py/ganeti.ssh_unittest.py b/test/py/
>> ganeti.ssh_unittest.py
>> index 9abc629..b0ba9dd 100755
>> --- a/test/py/ganeti.ssh_unittest.py
>> +++ b/test/py/ganeti.ssh_unittest.py
>> @@ -165,6 +165,11 @@ class TestSshKeys(testutils.GanetiTestCase):
>>     finally:
>>       handle.close()
>>
>> +  def testHasAuthorizedKey(self):
>> +    self.assertTrue(ssh.HasAuthorizedKey(self.tmpname, self.KEY_A))
>> +    self.assertFalse(ssh.HasAuthorizedKey(
>> +      self.tmpname, "I am the key of the pink bunny!"))
>> +
>>   def testAddingNewKey(self):
>>     ssh.AddAuthorizedKey(self.tmpname,
>>                          "ssh-dss AAAAB3NzaC1kc3MAAACB root@test")
>> --
>> 2.1.0.rc2.206.gedb03e5
>>
>>
> Rest LGTM
>

Thanks, fyi, interdiff:

diff --git a/lib/backend.py b/lib/backend.py
index 509e99b..8f48f9f 100644
--- a/lib/backend.py
+++ b/lib/backend.py
@@ -973,7 +973,7 @@ def _VerifySshSetup(node_status_list, my_name):

   my_status_list = [(my_uuid, name, mc, pot_mc) for (my_uuid, name, mc,
pot_mc)
                     in node_status_list if name == my_name]
-  if len(my_status_list) < 0:
+  if len(my_status_list) == 0:
     return ["Cannot find node information for node '%s'." % my_name]
   (my_uuid, _, _, potential_master_candidate) = \
      my_status_list[0]




-- 
Helga Velroyen | Software Engineer | [email protected] |

Google Germany GmbH
Dienerstr. 12
80331 München

Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg
Geschäftsführer: Graham Law, Christine Elizabeth Flores

Reply via email to