Added keep_jobs_seconds config option, deprecated keep_jobs

changelog/55295.changed

@@ -0,0 +1 @@
+renamed `keep_jobs`, specifying job cache TTL in hours, to `keep_jobs_seconds`, specifying TTL in seconds

conf/master

@@ -74,9 +74,13 @@
 # Verify and set permissions on configuration directories at startup:
 #verify_env: True
 
-# Set the number of hours to keep old job information in the job cache:
+# Set the number of hours to keep old job information in the job cache.
+# This option is deprecated by the keep_jobs_seconds option.
 #keep_jobs: 24
 
+# Set the number of seconds to keep old job information in the job cache:
+#keep_jobs_seconds: 86400
+
 # The number of seconds to wait when the client is requesting information
 # about running jobs.
 #gather_job_timeout: 10

conf/suse/master

@@ -74,8 +74,12 @@ syndic_user: salt
 #verify_env: True
 
 # Set the number of hours to keep old job information in the job cache:
+# This option is deprecated by the keep_jobs_seconds option.
 #keep_jobs: 24
 
+# Set the number of seconds to keep old job information in the job cache:
+#keep_jobs_seconds: 86400
+
 # The number of seconds to wait when the client is requesting information
 # about running jobs.
 #gather_job_timeout: 10

doc/ref/configuration/master.rst

@@ -315,10 +315,27 @@ Default: ``24``
 Set the number of hours to keep old job information. Note that setting this option
 to ``0`` disables the cache cleaner.
 
+.. deprecated:: 3006
+    Replaced by :conf_master:`keep_jobs_seconds`
+
 .. code-block:: yaml
 
     keep_jobs: 24
 
+.. conf_master:: keep_jobs_seconds
+
+``keep_jobs_seconds``
+---------------------
+
+Default: ``86400``
+
+Set the number of seconds to keep old job information. Note that setting this option
+to ``0`` disables the cache cleaner.
+
+.. code-block:: yaml
+
+    keep_jobs_seconds: 86400
+
 .. conf_master:: gather_job_timeout
 
 ``gather_job_timeout``
@@ -532,9 +549,9 @@ jobs dir.
     directory, which is ``/var/cache/salt/master/jobs/`` by default, will be
     smaller, but the JID directories will still be present.
 
-    Note that the :conf_master:`keep_jobs` option can be set to a lower value,
+    Note that the :conf_master:`keep_jobs_seconds` option can be set to a lower
-    such as ``1``, to limit the number of hours jobs are stored in the job
+    value, such as ``3600``, to limit the number of seconds jobs are stored in
-    cache. (The default is 24 hours.)
+    the job cache. (The default is 86400 seconds.)
 
     Please see the :ref:`Managing the Job Cache <managing_the_job_cache>`
     documentation for more information.

doc/topics/jobs/index.rst

@@ -70,8 +70,8 @@ lookup_jid
 ----------
 
 When jobs are executed the return data is sent back to the master and cached.
-By default it is cached for 24 hours, but this can be configured via the
+By default it is cached for 86400 seconds, but this can be configured via the
-``keep_jobs`` option in the master configuration.
+``keep_jobs_seconds`` option in the master configuration.
 Using the lookup_jid runner will display the same return data that the initial
 job invocation with the salt command would display.
 

doc/topics/jobs/job_cache.rst

@@ -19,15 +19,15 @@ job cache directory (on Linux systems this is typically
 deployments as it does not typically require any further configuration or
 management.
 
-The default job cache is a temporary cache and jobs will be stored for 24
+The default job cache is a temporary cache and jobs will be stored for 86400
-hours. If the default cache needs to store jobs for a different period the
+seconds. If the default cache needs to store jobs for a different period the
-time can be easily adjusted by changing the ``keep_jobs`` parameter in the
+time can be easily adjusted by changing the ``keep_jobs_seconds`` parameter
-Salt Master configuration file. The value passed in is measured via hours:
+in the Salt Master configuration file. The value passed in is measured in seconds:
 
 
 .. code-block:: yaml
 
-    keep_jobs: 24
+    keep_jobs_seconds: 86400
 
 Reducing the Size of the Default Job Cache
 ------------------------------------------
@@ -48,16 +48,16 @@ The default location for the job cache is in the ``/var/cache/salt/master/jobs/`
 directory.
 
 Setting the :conf_master:`job_cache` to ``False`` in addition to setting
-the :conf_master:`keep_jobs` option to a smaller value, such as ``1``, in the Salt
+the :conf_master:`keep_jobs_seconds` option to a smaller value, such as ``3600``,
-Master configuration file will reduce the size of the Default Job Cache, and thus
+in the Salt Master configuration file will reduce the size of the Default Job Cache,
-the burden on the Salt Master.
+and thus the burden on the Salt Master.
 
 .. note::
 
-    Changing the ``keep_jobs`` option sets the number of hours to keep old job
+    Changing the ``keep_jobs_seconds`` option sets the number of seconds to keep
-    information and defaults to ``24`` hours. Do not set this value to ``0`` when
+    old job information and defaults to ``86400`` seconds. Do not set this value
-    trying to make the cache cleaner run more frequently, as this means the cache
+    to ``0`` when trying to make the cache cleaner run more frequently, as this
-    cleaner will never run.
+    means the cache cleaner will never run.
 
 
 Additional Job Cache Options

doc/topics/tutorials/intro_scale.rst

@@ -241,7 +241,7 @@ the retention time defined by
 
 .. code-block:: yaml
 
-    keep_jobs: 24
+    keep_jobs_seconds: 86400
 
 .. code-block:: text
 

salt/config/__init__.py

@@ -25,6 +25,7 @@ import salt.utils.platform
 import salt.utils.stringutils
 import salt.utils.user
 import salt.utils.validate.path
+import salt.utils.versions
 import salt.utils.xdg
 import salt.utils.yaml
 from salt._logging import (
@@ -490,7 +491,10 @@ VALID_OPTS = immutabletypes.freeze(
         # to send returns.
         "ret_port": int,
         # The number of hours to keep jobs around in the job cache on the master
+        # This option is deprecated by keep_jobs_seconds
         "keep_jobs": int,
+        # The number of seconds to keep jobs around in the job cache on the master
+        "keep_jobs_seconds": int,
         # If the returner supports `clean_old_jobs`, then at cleanup time,
         # archive the job data before deleting it.
         "archive_jobs": bool,
@@ -1296,6 +1300,7 @@ DEFAULT_MASTER_OPTS = immutabletypes.freeze(
         "ret_port": 4506,
         "timeout": 5,
         "keep_jobs": 24,
+        "keep_jobs_seconds": 86400,
         "archive_jobs": False,
         "root_dir": salt.syspaths.ROOT_DIR,
         "pki_dir": os.path.join(salt.syspaths.LIB_STATE_DIR, "pki", "master"),

salt/daemons/masterapi.py

@@ -31,6 +31,7 @@ import salt.utils.files
 import salt.utils.gitfs
 import salt.utils.gzip_util
 import salt.utils.jid
+import salt.utils.job
 import salt.utils.mine
 import salt.utils.minions
 import salt.utils.path
@@ -137,7 +138,7 @@ def clean_pub_auth(opts):
                     if not os.path.isfile(auth_file_path):
                         continue
                     if time.time() - os.path.getmtime(auth_file_path) > (
-                        opts["keep_jobs"] * 3600
+                        salt.utils.job.get_keep_jobs_seconds(opts)
                     ):
                         os.remove(auth_file_path)
     except OSError:

salt/returners/couchbase_return.py

@@ -7,7 +7,7 @@ settings are listed below, along with sane defaults.
     couchbase.host:   'salt'
     couchbase.port:   8091
     couchbase.bucket: 'salt'
-    couchbase.ttl: 24
+    couchbase.ttl: 86400
     couchbase.password: 'password'
     couchbase.skip_verify_views: False
 
@@ -167,7 +167,7 @@ def _get_ttl():
     """
     Return the TTL that we should store our objects with
     """
-    return __opts__.get("couchbase.ttl", 24) * 60 * 60  # keep_jobs is in hours
+    return __opts__.get("couchbase.ttl", 86400)
 
 
 # TODO: add to returner docs-- this is a new one

salt/returners/local_cache.py

@@ -16,6 +16,7 @@ import salt.payload
 import salt.utils.atomicfile
 import salt.utils.files
 import salt.utils.jid
+import salt.utils.job
 import salt.utils.minions
 import salt.utils.msgpack
 import salt.utils.stringutils
@@ -410,7 +411,8 @@ def clean_old_jobs():
     """
     Clean out the old jobs from the job cache
     """
-    if __opts__["keep_jobs"] != 0:
+    keep_jobs_seconds = salt.utils.job.get_keep_jobs_seconds(__opts__)
+    if keep_jobs_seconds != 0:
         jid_root = _job_dir()
 
         if not os.path.exists(jid_root):
@@ -440,8 +442,8 @@ def clean_old_jobs():
                     _remove_job_dir(f_path)
                 elif os.path.isfile(jid_file):
                     jid_ctime = os.stat(jid_file).st_ctime
-                    hours_difference = (time.time() - jid_ctime) / 3600.0
+                    seconds_difference = time.time() - jid_ctime
-                    if hours_difference > __opts__["keep_jobs"] and os.path.exists(
+                    if seconds_difference > keep_jobs_seconds and os.path.exists(
                         t_path
                     ):
                         # Remove the entire f_path from the original JID dir
@@ -456,8 +458,8 @@ def clean_old_jobs():
                 # Checking the time again prevents a possible race condition where
                 # t_path JID dirs were created, but not yet populated by a jid file.
                 t_path_ctime = os.stat(t_path).st_ctime
-                hours_difference = (time.time() - t_path_ctime) / 3600.0
+                seconds_difference = time.time() - t_path_ctime
-                if hours_difference > __opts__["keep_jobs"]:
+                if seconds_difference > keep_jobs_seconds:
                     _remove_job_dir(t_path)
 
 

salt/returners/mysql.py

@@ -44,9 +44,9 @@ optional. The following ssl options are simply for illustration purposes:
     alternative.mysql.ssl_key: '/etc/pki/mysql/certs/localhost.key'
 
 Should you wish the returner data to be cleaned out every so often, set
-`keep_jobs` to the number of hours for the jobs to live in the tables.
+`keep_jobs_seconds` to the number of hours for the jobs to live in the
-Setting it to `0` will cause the data to stay in the tables. The default
+tables.  Setting it to `0` will cause the data to stay in the tables. The
-setting for `keep_jobs` is set to `24`.
+default setting for `keep_jobs_seconds` is set to `86400`.
 
 Should you wish to archive jobs in a different table for later processing,
 set `archive_jobs` to True.  Salt will create 3 archive tables
@@ -56,7 +56,7 @@ set `archive_jobs` to True.  Salt will create 3 archive tables
 - `salt_events_archive`
 
 and move the contents of `jids`, `salt_returns`, and `salt_events` that are
-more than `keep_jobs` hours old to these tables.
+more than `keep_jobs_seconds` seconds old to these tables.
 
 Use the following mysql database schema:
 
@@ -145,6 +145,7 @@ from contextlib import contextmanager
 import salt.exceptions
 import salt.returners
 import salt.utils.data
+import salt.utils.job
 import salt.utils.json
 
 # Let's not allow PyLint complain about string substitution
@@ -623,11 +624,12 @@ def clean_old_jobs():
     deletes the events and job details from the database.
     :return:
     """
-    if __opts__.get("keep_jobs", False) and int(__opts__.get("keep_jobs", 0)) > 0:
+    keep_jobs_seconds = int(salt.utils.job.get_keep_jobs_seconds(__opts__))
+    if keep_jobs_seconds > 0:
         try:
             with _get_serv() as cur:
-                sql = "select date_sub(now(), interval {} hour) as stamp;".format(
+                sql = "select date_sub(now(), interval {} second) as stamp;".format(
-                    __opts__["keep_jobs"]
+                    keep_jobs_seconds
                 )
                 cur.execute(sql)
                 rows = cur.fetchall()

salt/returners/pgjsonb.py

@@ -59,7 +59,7 @@ optional. The following ssl options are simply for illustration purposes:
     alternative.pgjsonb.ssl_key: '/etc/pki/mysql/certs/localhost.key'
 
 Should you wish the returner data to be cleaned out every so often, set
-``keep_jobs`` to the number of hours for the jobs to live in the tables.
+``keep_jobs_seconds`` to the number of seconds for the jobs to live in the tables.
 Setting it to ``0`` or leaving it unset will cause the data to stay in the tables.
 
 Should you wish to archive jobs in a different table for later processing,
@@ -70,7 +70,7 @@ set ``archive_jobs`` to True.  Salt will create 3 archive tables;
 - ``salt_events_archive``
 
 and move the contents of ``jids``, ``salt_returns``, and ``salt_events`` that are
-more than ``keep_jobs`` hours old to these tables.
+more than ``keep_jobs_seconds`` seconds old to these tables.
 
 .. versionadded:: 2019.2.0
 
@@ -168,6 +168,7 @@ from contextlib import contextmanager
 import salt.exceptions
 import salt.returners
 import salt.utils.data
+import salt.utils.job
 
 # Let's not allow PyLint complain about string substitution
 # pylint: disable=W1321,E1321
@@ -577,11 +578,12 @@ def clean_old_jobs():
     deletes the events and job details from the database.
     :return:
     """
-    if __opts__.get("keep_jobs", False) and int(__opts__.get("keep_jobs", 0)) > 0:
+    keep_jobs_seconds = int(salt.utils.job.get_keep_jobs_seconds(__opts__))
+    if keep_jobs_seconds > 0:
         try:
             with _get_serv() as cur:
-                sql = "select (NOW() -  interval '{}' hour) as stamp;".format(
+                sql = "select (NOW() -  interval '{}' second) as stamp;".format(
-                    __opts__["keep_jobs"]
+                    keep_jobs_seconds
                 )
                 cur.execute(sql)
                 rows = cur.fetchall()

salt/returners/postgres_local_cache.py

@@ -112,6 +112,7 @@ import re
 import sys
 
 import salt.utils.jid
+import salt.utils.job
 import salt.utils.json
 
 try:
@@ -398,12 +399,13 @@ def get_jids():
         """jid, tgt_type, cmd, tgt, kwargs, ret, username, arg, fun """
         """FROM jids"""
     )
-    if __opts__["keep_jobs"] != 0:
+    keep_jobs_seconds = int(salt.utils.job.get_keep_jobs_seconds(__opts__))
+    if keep_jobs_seconds != 0:
         sql = (
             sql
             + " WHERE started > NOW() - INTERVAL '"
-            + str(__opts__["keep_jobs"])
+            + str(keep_jobs_seconds)
-            + "' HOUR"
+            + "' SECOND"
         )
 
     cur.execute(sql)

salt/returners/redis_return.py

@@ -97,6 +97,7 @@ import logging
 
 import salt.returners
 import salt.utils.jid
+import salt.utils.job
 import salt.utils.json
 import salt.utils.platform
 
@@ -201,7 +202,7 @@ def _get_serv(ret=None):
 
 
 def _get_ttl():
-    return __opts__.get("keep_jobs", 24) * 3600
+    return salt.utils.job.get_keep_jobs_seconds(__opts__)
 
 
 def returner(ret):

salt/utils/job.py

@@ -9,6 +9,7 @@ import salt.minion
 import salt.utils.event
 import salt.utils.jid
 import salt.utils.verify
+import salt.utils.versions
 
 log = logging.getLogger(__name__)
 
@@ -179,4 +180,24 @@ def get_retcode(ret):
     return retcode
 
 
+def get_keep_jobs_seconds(opts):
+    """
+    Temporary function until 'keep_jobs' is fully deprecated,
+    this will prefer 'keep_jobs_seconds', and only use
+    'keep_jobs' as the configuration value if 'keep_jobs_seconds'
+    is unmodified, and 'keep_jobs' is modified (in which case it
+    will emit a deprecation warning).
+    """
+    if opts["keep_jobs_seconds"] == 86400 and opts["keep_jobs"] != 24:
+        salt.utils.versions.warn_until(
+            "Chlorine",
+            "The 'keep_jobs' option has been deprecated and replaced with "
+            "'keep_jobs_seconds'.",
+        )
+        keep_jobs_seconds = opts["keep_jobs"] * 3600
+    else:
+        keep_jobs_seconds = opts["keep_jobs_seconds"]
+    return keep_jobs_seconds
+
+
 # vim:set et sts=4 ts=4 tw=80:

tests/pytests/unit/returners/local_cache/test_local_cache.py

@@ -24,7 +24,7 @@ def configure_loader_modules(tmp_cache_dir):
         local_cache: {
             "__opts__": {
                 "cachedir": str(tmp_cache_dir),
-                "keep_jobs": 0.0000000010,
+                "keep_jobs_seconds": 0.0000000010,
             }
         }
     }
@@ -139,7 +139,7 @@ def test_not_clean_new_jobs(add_job, job_cache_dir_files):
     """
     add_job()
 
-    with patch.dict(local_cache.__opts__, {"keep_jobs": 24}):
+    with patch.dict(local_cache.__opts__, {"keep_jobs_seconds": 86400}):
         assert local_cache.clean_old_jobs() is None
 
         _check_dir_files(

tests/pytests/unit/returners/local_cache/test_local_cache_clean_old_jobs.py

@@ -31,7 +31,7 @@ def configure_loader_modules(tmp_cache_dir):
         local_cache: {
             "__opts__": {
                 "cachedir": str(tmp_cache_dir),
-                "keep_jobs": 1,
+                "keep_jobs_seconds": 3600,
             },
         }
     }
@@ -96,9 +96,9 @@ def test_clean_old_jobs_empty_jid_dir_removed(make_tmp_jid_dirs, tmp_jid_dir):
     # Make sure there are no files in the directory before continuing
     assert jid_file is None
 
-    # Call clean_old_jobs function, patching the keep_jobs value with a
+    # Call clean_old_jobs function, patching the keep_jobs_seconds value
-    # very small value to force the call to clean the job.
+    # with a very small value to force the call to clean the job.
-    with patch.dict(local_cache.__opts__, {"keep_jobs": 0.00000001}):
+    with patch.dict(local_cache.__opts__, {"keep_jobs_seconds": 0.00000001}):
         # Sleep on Windows because time.time is only precise to 3 decimal
         # points, and therefore subtracting the jid_ctime from time.time
         # will result in a negative number
@@ -113,7 +113,7 @@ def test_clean_old_jobs_empty_jid_dir_removed(make_tmp_jid_dirs, tmp_jid_dir):
 def test_clean_old_jobs_empty_jid_dir_remains(make_tmp_jid_dirs, tmp_jid_dir):
     """
     Tests that an empty JID dir is NOT removed because it was created within
-    the keep_jobs time frame.
+    the keep_jobs_seconds time frame.
     """
     # Create temp job cache dir without files in it.
     jid_dir, jid_file = make_tmp_jid_dirs(create_files=False)
@@ -130,7 +130,7 @@ def test_clean_old_jobs_empty_jid_dir_remains(make_tmp_jid_dirs, tmp_jid_dir):
     else:
         jid_dir_name = jid_dir.rpartition("/")[2]
 
-    # Assert the JID directory is still present to be cleaned after keep_jobs interval
+    # Assert the JID directory is still present to be cleaned after keep_jobs_seconds interval
     assert [jid_dir_name] == os.listdir(tmp_jid_dir)
 
 
@@ -176,9 +176,9 @@ def test_clean_old_jobs_jid_file_is_cleaned(make_tmp_jid_dirs, tmp_jid_dir):
     jid_dir_name = jid_file.rpartition(os.sep)[2]
     assert jid_dir_name == "jid"
 
-    # Call clean_old_jobs function, patching the keep_jobs value with a
+    # Call clean_old_jobs function, patching the keep_jobs_seconds value with a
     # very small value to force the call to clean the job.
-    with patch.dict(local_cache.__opts__, {"keep_jobs": 0.00000001}):
+    with patch.dict(local_cache.__opts__, {"keep_jobs_seconds": 0.00000001}):
         # Sleep on Windows because time.time is only precise to 3 decimal
         # points, and therefore subtracting the jid_ctime from time.time
         # will result in a negative number

tests/pytests/unit/returners/test_pgjsonb.py

@@ -15,7 +15,7 @@ if pgjsonb.HAS_PG:
 
 @pytest.fixture
 def configure_loader_modules():
-    return {pgjsonb: {"__opts__": {"keep_jobs": 1, "archive_jobs": 0}}}
+    return {pgjsonb: {"__opts__": {"keep_jobs_seconds": 3600, "archive_jobs": 0}}}
 
 
 def test_clean_old_jobs_purge():

tests/pytests/unit/returners/test_postgres_local_cache.py

@@ -14,7 +14,10 @@ from tests.support.mock import MagicMock, patch
 def configure_loader_modules(tmp_path):
     return {
         postgres_local_cache: {
-            "__opts__": {"cachedir": str(tmp_path / "cache_dir"), "keep_jobs": 1}
+            "__opts__": {
+                "cachedir": str(tmp_path / "cache_dir"),
+                "keep_jobs_seconds": 3600,
+            }
         }
     }