Merge "Add check to limit maximum value of age_in_days"

cinder/cmd/manage.py

@@ -58,6 +58,7 @@ from __future__ import print_function
 import logging as python_logging
 import os
 import sys
+import time
 
 from oslo_config import cfg
 from oslo_db import exception as db_exc
@@ -229,6 +230,9 @@ class DbCommands(object):
         if age_in_days <= 0:
             print(_("Must supply a positive, non-zero value for age"))
             sys.exit(1)
+        if age_in_days >= (int(time.time()) / 86400):
+            print(_("Maximum age is count of days since epoch."))
+            sys.exit(1)
         ctxt = context.get_admin_context()
 
         try:

cinder/db/sqlalchemy/api.py

@@ -4715,10 +4715,6 @@ def purge_deleted_rows(context, age_in_days):
         msg = _('Invalid value for age, %(age)s') % {'age': age_in_days}
         LOG.exception(msg)
         raise exception.InvalidParameterValue(msg)
-    if age_in_days <= 0:
-        msg = _('Must supply a positive value for age')
-        LOG.error(msg)
-        raise exception.InvalidParameterValue(msg)
 
     engine = get_engine()
     session = get_session()

cinder/tests/unit/db/test_purge.py

@@ -234,10 +234,6 @@ class PurgeDeletedTest(test.TestCase):
         self.assertRaises(exception.InvalidParameterValue,
                           db.purge_deleted_rows, self.context,
                           age_in_days='ten')
-        # Test with negative value
-        self.assertRaises(exception.InvalidParameterValue,
-                          db.purge_deleted_rows, self.context,
-                          age_in_days=-1)
 
     def test_purge_deleted_rows_integrity_failure(self):
         dialect = self.engine.url.get_dialect()

cinder/tests/unit/test_cmd.py

@@ -12,6 +12,7 @@
 
 import datetime
 import sys
+import time
 
 import ddt
 import mock
@@ -395,6 +396,11 @@ class TestCinderManageCmd(test.TestCase):
     def tearDown(self):
         super(TestCinderManageCmd, self).tearDown()
 
+    def _test_purge_invalid_age_in_days(self, age_in_days):
+        db_cmds = cinder_manage.DbCommands()
+        ex = self.assertRaises(SystemExit, db_cmds.purge, age_in_days)
+        self.assertEqual(1, ex.code)
+
     @mock.patch('cinder.db.migration.db_sync')
     def test_db_commands_sync(self, db_sync):
         version = mock.MagicMock()
@@ -430,6 +436,36 @@ class TestCinderManageCmd(test.TestCase):
             version_cmds.__call__()
             version_string.assert_called_once_with()
 
+    def test_purge_age_in_days_value_equal_to_zero(self):
+        age_in_days = 0
+        self._test_purge_invalid_age_in_days(age_in_days)
+
+    def test_purge_with_negative_age_in_days(self):
+        age_in_days = -1
+        self._test_purge_invalid_age_in_days(age_in_days)
+
+    def test_purge_exceeded_age_in_days_limit(self):
+        age_in_days = int(time.time() / 86400) + 1
+        self._test_purge_invalid_age_in_days(age_in_days)
+
+    @mock.patch('cinder.db.sqlalchemy.api.purge_deleted_rows')
+    @mock.patch('cinder.context.get_admin_context')
+    def test_purge_less_than_age_in_days_limit(self, get_admin_context,
+                                               purge_deleted_rows):
+        age_in_days = int(time.time() / 86400) - 1
+        ctxt = context.RequestContext(fake.USER_ID, fake.PROJECT_ID,
+                                      is_admin=True)
+        get_admin_context.return_value = ctxt
+
+        purge_deleted_rows.return_value = None
+
+        db_cmds = cinder_manage.DbCommands()
+        db_cmds.purge(age_in_days)
+
+        get_admin_context.assert_called_once_with()
+        purge_deleted_rows.assert_called_once_with(
+            ctxt, age_in_days=age_in_days)
+
     @mock.patch('cinder.db.service_get_all')
     @mock.patch('cinder.context.get_admin_context')
     def test_host_commands_list(self, get_admin_context, service_get_all):