Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache API tests & unifying behavior #61229

Merged
merged 32 commits into from
Apr 8, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
e4944f0
Add tests for cache backends
waynew Jun 29, 2021
747ef80
Tests run against everything!
waynew Jul 27, 2021
4385936
Get consul passing cache tests
waynew Oct 12, 2021
fbebafa
now mysql tests also pass
waynew Oct 12, 2021
9230a62
comments
waynew Oct 12, 2021
721b66f
add mysql cache tests
waynew Oct 19, 2021
6dd660d
Ensure consul cache is flushing timestamp keys
waynew Oct 21, 2021
453a77e
Ensure etcd cache is flushing timestamp keys
waynew Oct 21, 2021
e6db022
Update redis cache api to conform to localfs
waynew Oct 28, 2021
4cb1f76
Fix etcd path inconsistency
waynew Oct 28, 2021
ac2078e
Remove comments and cleanup test setup
waynew Oct 28, 2021
764169e
Add localfs backed memcache to the tests
waynew Oct 28, 2021
8af593b
Bad serial
waynew Nov 2, 2021
166e68f
No this is wrong
waynew Nov 9, 2021
a0bd87c
Probably the right lazy loader fix
waynew Nov 9, 2021
fa98569
Restore tested mysql cache functionality
waynew Nov 11, 2021
fbf96b8
remove silly files
waynew Nov 11, 2021
b8e7e61
Fix redis_cache cluster mode import
waynew Nov 18, 2021
260f4e0
payload should string-ify all the data
waynew Nov 29, 2021
b09f0eb
Fix unit tests for parametrization
waynew Nov 29, 2021
55cf928
Fix missing docstring complaints
waynew Nov 29, 2021
320690d
Merge remote-tracking branch 'salt/master' into tc/cache_tests
waynew Feb 3, 2022
0c51cb9
incorporate PR feedback
waynew Feb 16, 2022
7606a50
Merge branch 'master' into tc/cache_tests
waynew Mar 4, 2022
213b124
Skip docker if not found and add the rest of mysql
waynew Mar 18, 2022
01aca0b
Tests all passing, woo!
waynew Apr 5, 2022
faf365d
Merge remote-tracking branch 'salt/master' into tc/cache_tests
waynew Apr 5, 2022
286c82f
run black
waynew Apr 5, 2022
f64d918
Merge branch 'master' into tc/cache_tests
waynew Apr 6, 2022
53afc63
Disable pylint here
waynew Apr 6, 2022
75e93d3
Merge remote-tracking branch 'wayne/tc/cache_tests' into tc/cache_tests
waynew Apr 6, 2022
9fbae75
Skip when mysql fails during CI
waynew Apr 6, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog/60272.fixed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Corrected import statement for redis_cache in cluster mode.
1 change: 1 addition & 0 deletions changelog/61081.changed
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Updated mysql cache module to also store updated timestamp, making it consistent with default cache module. Users of mysql cache should ensure database size before updating, as ALTER TABLE will add the timestamp column.
9 changes: 7 additions & 2 deletions salt/cache/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Cache:
The name of the cache driver to use. This is the name of the python
module of the `salt.cache` package. Default is `localfs`.

Terminology.
Terminology:

Salt cache subsystem is organized as a tree with nodes and leafs like a
filesystem. Cache consists of banks. Each bank can contain a number of
Expand Down Expand Up @@ -345,5 +345,10 @@ def store(self, bank, key, data):
self.storage[(bank, key)] = [time.time(), data]

def flush(self, bank, key=None):
self.storage.pop((bank, key), None)
if key is None:
for bank_, key_ in tuple(self.storage):
if bank == bank_:
self.storage.pop((bank_, key_))
else:
self.storage.pop((bank, key), None)
super().flush(bank, key)
60 changes: 47 additions & 13 deletions salt/cache/consul.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

.. versionadded:: 2016.11.2

.. versionchanged:: 3005.0

Timestamp/cache updated support added.

:depends: python-consul >= 0.2.0

It is up to the system administrator to set up and configure the Consul
Expand Down Expand Up @@ -30,6 +34,12 @@
consul.consistency: default
consul.dc: dc1
consul.verify: True
consul.timestamp_suffix: .tstamp # Added in 3005.0

In order to bring the cache APIs into conformity, in 3005.0 timestamp
information gets stored as a separate ``{key}.tstamp`` key/value. If your
existing functionality depends on being able to store normal keys with the
``.tstamp`` suffix, override the ``consul.timestamp_suffix`` default config.

Related docs could be found in the `python-consul documentation`_.

Expand All @@ -47,6 +57,7 @@
"""

import logging
import time

import salt.payload
from salt.exceptions import SaltCacheError
Expand All @@ -61,6 +72,7 @@

log = logging.getLogger(__name__)
api = None
_tstamp_suffix = ".tstamp"


# Define the module's virtual name
Expand Down Expand Up @@ -90,7 +102,8 @@ def __virtual__():
}

try:
global api
global api, _tstamp_suffix
_tstamp_suffix = __opts__.get("consul.timestamp_suffix", _tstamp_suffix)
api = consul.Consul(**consul_kwargs)
except AttributeError:
return (
Expand All @@ -107,9 +120,12 @@ def store(bank, key, data):
Store a key value.
"""
c_key = "{}/{}".format(bank, key)
tstamp_key = "{}/{}{}".format(bank, key, _tstamp_suffix)

try:
c_data = salt.payload.dumps(data)
api.kv.put(c_key, c_data)
api.kv.put(tstamp_key, salt.payload.dumps(int(time.time())))
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
"There was an error writing the key, {}: {}".format(c_key, exc)
Expand Down Expand Up @@ -138,9 +154,13 @@ def flush(bank, key=None):
"""
if key is None:
c_key = bank
tstamp_key = None
else:
c_key = "{}/{}".format(bank, key)
tstamp_key = "{}/{}{}".format(bank, key, _tstamp_suffix)
try:
if tstamp_key:
api.kv.delete(tstamp_key)
return api.kv.delete(c_key, recurse=key is None)
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
Expand All @@ -166,22 +186,36 @@ def list_(bank):
out = set()
for key in keys:
out.add(key[len(bank) + 1 :].rstrip("/"))
keys = list(out)
keys = [o for o in out if not o.endswith(_tstamp_suffix)]
return keys


def contains(bank, key):
"""
Checks if the specified bank contains the specified key.
"""
if key is None:
return True # any key could be a branch and a leaf at the same time in Consul
else:
try:
c_key = "{}/{}".format(bank, key)
_, value = api.kv.get(c_key)
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
"There was an error getting the key, {}: {}".format(c_key, exc)
)
return value is not None
try:
c_key = "{}/{}".format(bank, key or "")
_, value = api.kv.get(c_key, keys=True)
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
"There was an error getting the key, {}: {}".format(c_key, exc)
)
return value is not None


def updated(bank, key):
"""
Return the Unix Epoch timestamp of when the key was last updated. Return
None if key is not found.
"""
c_key = "{}/{}{}".format(bank, key, _tstamp_suffix)
try:
_, value = api.kv.get(c_key)
if value is None:
return None
return salt.payload.loads(value["Value"])
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
"There was an error reading the key, {}: {}".format(c_key, exc)
)
47 changes: 41 additions & 6 deletions salt/cache/etcd_cache.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
Minion data cache plugin for Etcd key/value data store.

.. versionadded:: 2018.3.0
.. versionchanged:: 3005

It is up to the system administrator to set up and configure the Etcd
infrastructure. All is needed for this plugin is a working Etcd agent
Expand Down Expand Up @@ -42,6 +43,9 @@

cache: etcd

In Phosphorus, ls/list was changed to always return the final name in the path.
This should only make a difference if you were directly using ``ls`` on paths
that were more or less nested than, for example: ``1/2/3/4``.

.. _`Etcd documentation`: https://github.com/coreos/etcd
.. _`python-etcd documentation`: http://python-etcd.readthedocs.io/en/latest/
Expand All @@ -50,6 +54,7 @@

import base64
import logging
import time

import salt.payload
from salt.exceptions import SaltCacheError
Expand All @@ -72,6 +77,7 @@
log = logging.getLogger(__name__)
client = None
path_prefix = None
_tstamp_suffix = ".tstamp"

# Module properties

Expand All @@ -94,7 +100,7 @@ def __virtual__():

def _init_client():
"""Setup client and init datastore."""
global client, path_prefix
global client, path_prefix, _tstamp_suffix
if client is not None:
return

Expand All @@ -111,6 +117,7 @@ def _init_client():
"cert": __opts__.get("etcd.cert", None),
"ca_cert": __opts__.get("etcd.ca_cert", None),
}
_tstamp_suffix = __opts__.get("etcd.timestamp_suffix", _tstamp_suffix)
path_prefix = __opts__.get("etcd.path_prefix", _DEFAULT_PATH_PREFIX)
if path_prefix != "":
path_prefix = "/{}".format(path_prefix.strip("/"))
Expand All @@ -129,9 +136,11 @@ def store(bank, key, data):
"""
_init_client()
etcd_key = "{}/{}/{}".format(path_prefix, bank, key)
etcd_tstamp_key = "{}/{}/{}".format(path_prefix, bank, key + _tstamp_suffix)
try:
value = salt.payload.dumps(data)
client.write(etcd_key, base64.b64encode(value))
client.write(etcd_tstamp_key, int(time.time()))
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
"There was an error writing the key, {}: {}".format(etcd_key, exc)
Expand Down Expand Up @@ -162,13 +171,17 @@ def flush(bank, key=None):
_init_client()
if key is None:
etcd_key = "{}/{}".format(path_prefix, bank)
tstamp_key = None
else:
etcd_key = "{}/{}/{}".format(path_prefix, bank, key)
tstamp_key = "{}/{}/{}".format(path_prefix, bank, key + _tstamp_suffix)
try:
client.read(etcd_key)
except etcd.EtcdKeyNotFound:
return # nothing to flush
try:
if tstamp_key:
client.delete(tstamp_key)
client.delete(etcd_key, recursive=True)
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
Expand All @@ -182,7 +195,10 @@ def _walk(r):
r: etcd.EtcdResult
"""
if not r.dir:
return [r.key.split("/", 3)[3]]
if r.key.endswith(_tstamp_suffix):
return []
else:
return [r.key.rsplit("/", 1)[-1]]
Ch3LL marked this conversation as resolved.
Show resolved Hide resolved

keys = []
for c in client.read(r.key).children:
Expand All @@ -199,25 +215,44 @@ def ls(bank):
path = "{}/{}".format(path_prefix, bank)
try:
return _walk(client.read(path))
except etcd.EtcdKeyNotFound:
return []
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
'There was an error getting the key "{}": {}'.format(bank, exc)
)
) from exc


def contains(bank, key):
"""
Checks if the specified bank contains the specified key.
"""
_init_client()
etcd_key = "{}/{}/{}".format(path_prefix, bank, key)
etcd_key = "{}/{}/{}".format(path_prefix, bank, key or "")
try:
r = client.read(etcd_key)
# return True for keys, not dirs
return r.dir is False
# return True for keys, not dirs, unless key is None
return r.dir if key is None else r.dir is False
except etcd.EtcdKeyNotFound:
return False
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
"There was an error getting the key, {}: {}".format(etcd_key, exc)
)


def updated(bank, key):
"""
Return Unix Epoch based timestamp of when the bank/key was updated.
"""
_init_client()
tstamp_key = "{}/{}/{}".format(path_prefix, bank, key + _tstamp_suffix)
try:
value = client.read(tstamp_key).value
return int(value)
except etcd.EtcdKeyNotFound:
return None
except Exception as exc: # pylint: disable=broad-except
raise SaltCacheError(
"There was an error reading the key, {}: {}".format(tstamp_key, exc)
)
Loading