Merge "Fix cleaning up console tokens"

nova/compute/manager.py

@@ -8835,15 +8835,14 @@ class ComputeManager(manager.Manager):
 
     @periodic_task.periodic_task(spacing=CONF.instance_delete_interval)
     def _cleanup_expired_console_auth_tokens(self, context):
-        """Remove expired console auth tokens for this host.
+        """Remove all expired console auth tokens.
 
         Console authorization tokens and their connection data are stored
         in the database when a user asks for a console connection to an
         instance. After a time they expire. We periodically remove any expired
         tokens from the database.
         """
-        objects.ConsoleAuthToken.clean_expired_console_auths_for_host(
+        objects.ConsoleAuthToken.clean_expired_console_auths(context)
-            context, self.host)
 
     def _claim_pci_for_instance_vifs(self, ctxt, instance):
         """Claim PCI devices for the instance's VIFs on the compute node

nova/db/api.py

@@ -1864,6 +1864,15 @@ def console_auth_token_destroy_all_by_instance(context, instance_uuid):
                                                            instance_uuid)
 
 
+def console_auth_token_destroy_expired(context):
+    """Delete expired console authorizations.
+
+    The console authorizations expire at the time specified by their
+    'expires' column. This function is used to garbage collect expired tokens.
+    """
+    return IMPL.console_auth_token_destroy_expired(context)
+
+
 def console_auth_token_destroy_expired_by_host(context, host):
     """Delete expired console authorizations belonging to the host.
 

nova/db/sqlalchemy/api.py

@@ -5833,6 +5833,13 @@ def console_auth_token_destroy_all_by_instance(context, instance_uuid):
         filter_by(instance_uuid=instance_uuid).delete()
 
 
+@pick_context_manager_writer
+def console_auth_token_destroy_expired(context):
+    context.session.query(models.ConsoleAuthToken).\
+        filter(models.ConsoleAuthToken.expires <= timeutils.utcnow_ts()).\
+        delete()
+
+
 @pick_context_manager_writer
 def console_auth_token_destroy_expired_by_host(context, host):
     context.session.query(models.ConsoleAuthToken).\

nova/objects/console_auth_token.py

@@ -34,7 +34,10 @@ LOG = logging.getLogger(__name__)
 @base.NovaObjectRegistry.register
 class ConsoleAuthToken(base.NovaTimestampObject, base.NovaObject):
     # Version 1.0: Initial version
-    VERSION = '1.0'
+    # Version 1.1: Add clean_expired_console_auths method.
+    #              The clean_expired_console_auths_for_host method
+    #              was deprecated.
+    VERSION = '1.1'
 
     fields = {
         'id': fields.IntegerField(),
@@ -176,6 +179,19 @@ class ConsoleAuthToken(base.NovaTimestampObject, base.NovaObject):
         """
         db.console_auth_token_destroy_all_by_instance(context, instance_uuid)
 
+    @base.remotable_classmethod
+    def clean_expired_console_auths(cls, context):
+        """Remove all expired console authorizations.
+
+        :param context: the context
+
+        All expired authorizations will be removed.
+        Tokens that have not expired will remain.
+        """
+        db.console_auth_token_destroy_expired(context)
+
+    # TODO(takashin): This method was deprecated and will be removed
+    # in a next major version bump.
     @base.remotable_classmethod
     def clean_expired_console_auths_for_host(cls, context, host):
         """Remove all expired console authorizations for the host.

nova/tests/unit/compute/test_compute_mgr.py

@@ -4728,11 +4728,10 @@ class ComputeManagerUnitTestCase(test.NoDBTestCase,
         self.compute._clean_instance_console_tokens(self.context, instance)
         mock_clean.assert_not_called()
 
-    @mock.patch('nova.objects.ConsoleAuthToken.'
+    @mock.patch('nova.objects.ConsoleAuthToken.clean_expired_console_auths')
-                'clean_expired_console_auths_for_host')
     def test_cleanup_expired_console_auth_tokens(self, mock_clean):
         self.compute._cleanup_expired_console_auth_tokens(self.context)
-        mock_clean.assert_called_once_with(self.context, self.compute.host)
+        mock_clean.assert_called_once_with(self.context)
 
     @mock.patch.object(nova.context.RequestContext, 'elevated')
     @mock.patch.object(nova.objects.InstanceList, 'get_by_host')

nova/tests/unit/db/test_db_api.py

@@ -9818,6 +9818,35 @@ class ConsoleAuthTokenTestCase(test.TestCase):
         self.assertEqual(hash1, db_obj1['token_hash'])
         self.assertIsNone(db_obj2, "the token uuid should not match")
 
+    def test_console_auth_token_destroy_expired(self):
+        uuid1 = uuidsentinel.uuid1
+        uuid2 = uuidsentinel.uuid2
+        uuid3 = uuidsentinel.uuid3
+        hash1 = utils.get_sha256_str(uuidsentinel.token1)
+        hash2 = utils.get_sha256_str(uuidsentinel.token2)
+        hash3 = utils.get_sha256_str(uuidsentinel.token3)
+        self.addCleanup(timeutils.clear_time_override)
+        timeutils.set_time_override(timeutils.utcnow())
+        self._create_instances([uuid1, uuid2, uuid3])
+
+        self._create(hash1, uuid1, 10)
+        self._create(hash2, uuid2, 10, host='other-host')
+        timeutils.advance_time_seconds(100)
+        self._create(hash3, uuid3, 10)
+
+        db.console_auth_token_destroy_expired(self.context)
+
+        # the api only supports getting unexpired tokens
+        # but by rolling back time we can see if a token that
+        # should be deleted is still there
+        timeutils.advance_time_seconds(-100)
+        db_obj1 = db.console_auth_token_get_valid(self.context, hash1, uuid1)
+        db_obj2 = db.console_auth_token_get_valid(self.context, hash2, uuid2)
+        db_obj3 = db.console_auth_token_get_valid(self.context, hash3, uuid3)
+        self.assertIsNone(db_obj1, "the token should have been deleted")
+        self.assertIsNone(db_obj2, "the token should have been deleted")
+        self.assertIsNotNone(db_obj3, "a valid token should be found here")
+
     def test_console_auth_token_destroy_expired_by_host(self):
         uuid1 = uuidsentinel.uuid1
         uuid2 = uuidsentinel.uuid2

nova/tests/unit/objects/test_console_auth_token.py

@@ -148,6 +148,11 @@ class _TestConsoleAuthToken(object):
         mock_destroy.assert_called_once_with(
             self.context, uuidsentinel.instance)
 
+    @mock.patch('nova.db.api.console_auth_token_destroy_expired')
+    def test_clean_expired_console_auths(self, mock_destroy):
+        token_obj.ConsoleAuthToken.clean_expired_console_auths(self.context)
+        mock_destroy.assert_called_once_with(self.context)
+
     @mock.patch('nova.db.api.console_auth_token_destroy_expired_by_host')
     def test_clean_expired_console_auths_for_host(self, mock_destroy):
         token_obj.ConsoleAuthToken.clean_expired_console_auths_for_host(

nova/tests/unit/objects/test_objects.py

@@ -1045,7 +1045,7 @@ object_data = {
     'CellMappingList': '1.1-496ef79bb2ab41041fff8bcb57996352',
     'ComputeNode': '1.19-af6bd29a6c3b225da436a0d8487096f2',
     'ComputeNodeList': '1.17-52f3b0962b1c86b98590144463ebb192',
-    'ConsoleAuthToken': '1.0-a61bf7b54517c4013a12289c5a5268ea',
+    'ConsoleAuthToken': '1.1-8da320fb065080eb4d3c2e5c59f8bf52',
     'CpuDiagnostics': '1.0-d256f2e442d1b837735fd17dfe8e3d47',
     'DNSDomain': '1.0-7b0b2dab778454b6a7b6c66afe163a1a',
     'DNSDomainList': '1.0-4ee0d9efdfd681fed822da88376e04d2',