LGTM

On Fri, Nov 6, 2015 at 12:07 PM 'Helga Velroyen' via ganeti-devel <
[email protected]> wrote:

> 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 <[email protected]>
> ---
>  lib/backend.py                     |  3 +++
>  test/py/ganeti.backend_unittest.py | 18 +++++++++++-------
>  2 files changed, 14 insertions(+), 7 deletions(-)
>
> diff --git a/lib/backend.py b/lib/backend.py
> index cd94d92..f891ef6 100644
> --- a/lib/backend.py
> +++ b/lib/backend.py
> @@ -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"
> diff --git a/test/py/ganeti.backend_unittest.py b/test/py/
> ganeti.backend_unittest.py
> index 68b2eee..393239a 100755
> --- a/test/py/ganeti.backend_unittest.py
> +++ b/test/py/ganeti.backend_unittest.py
> @@ -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.
> --
> 2.6.0.rc2.230.g3dd15c0
>
> --
Lisa Velden
Software Engineer
[email protected]

Google Germany GmbH
Dienerstraße 12
80331 München

Geschäftsführer: Matthew Scott Sucherman, Paul Terence Manicle
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Reply via email to