From e5884a8fd51b735cf870f818d2a828163006e18a Mon Sep 17 00:00:00 2001 From: Christoph Manns Date: Thu, 27 Sep 2018 16:18:54 +0200 Subject: [PATCH] Fix stacktraces with redis caching backend If you use redis as a caching backend and you delete a server with no consoleauth tokens you'll get a stacktrace as an empty list is passed down to the redis client and ultimately the redis server which responds with an error, complaining about a wrong number of arguments for the del command. The code now checks if the list of tokens is empty and only calls the caching backend if there are tokens available to delete. This also may improve performance, as it no longer hands down an empty list. Closes-Bug: #1794812 Change-Id: Iffdd4e251bfa2bac1bfd49498e32b738843709de --- nova/consoleauth/manager.py | 3 ++- nova/tests/unit/consoleauth/test_consoleauth.py | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/nova/consoleauth/manager.py b/nova/consoleauth/manager.py index 52d4529e5453..1a0ade144f17 100644 --- a/nova/consoleauth/manager.py +++ b/nova/consoleauth/manager.py @@ -139,6 +139,7 @@ class ConsoleAuthManager(manager.Manager): def delete_tokens_for_instance(self, context, instance_uuid): tokens = self._get_tokens_for_instance(instance_uuid) - self.mc.delete_multi( + if tokens: + self.mc.delete_multi( [tok.encode('UTF-8') for tok in tokens]) self.mc_instance.delete(instance_uuid.encode('UTF-8')) diff --git a/nova/tests/unit/consoleauth/test_consoleauth.py b/nova/tests/unit/consoleauth/test_consoleauth.py index 1c4f17f78465..7d103aeacbc3 100644 --- a/nova/tests/unit/consoleauth/test_consoleauth.py +++ b/nova/tests/unit/consoleauth/test_consoleauth.py @@ -122,6 +122,23 @@ class ConsoleauthTestCase(test.NoDBTestCase): self.assertIsNone( self.manager_api.check_token(self.context, token)) + def test_delete_tokens_for_instance_no_tokens(self): + with test.nested( + mock.patch.object(self.manager, '_get_tokens_for_instance', + return_value=[]), + mock.patch.object(self.manager.mc, 'delete_multi'), + mock.patch.object(self.manager.mc_instance, 'delete') + ) as ( + mock_get_tokens, mock_delete_multi, mock_delete + ): + self.manager.delete_tokens_for_instance( + self.context, self.instance_uuid) + # Since here were no tokens, we didn't try to clear anything + # from the cache. + mock_delete_multi.assert_not_called() + mock_delete.assert_called_once_with( + self.instance_uuid.encode('UTF-8')) + @mock.patch('nova.objects.instance.Instance.get_by_uuid') def test_wrong_token_has_port(self, mock_get): mock_get.return_value = None