memoize sdb module's function for config application

[kiemlicz]

Description of Issue

I've created custom sdb module that reads KDBX
I use this module in Salt Master installations which contain a lot of sdb://kdbx/... schemes.
During:

  File "/usr/lib/python3/dist-packages/salt/config/__init__.py", line 3938, in master_config
    apply_sdb(opts)

Every config's sdb scheme causes separate call to heavy method: _load
I thought that if I add @memoize for _load method that would help the KDBX load once

However during evaluating config the apply_sdb doesn't save the memoize closue, thus adding @memoize doesn't help

1. Could someone explain me why the memoize doesn't work (I'm not a Python dev)? Is it because of https://github.com/saltstack/salt/blob/master/salt/loader.py#L954 sdb loader returning new Loader instance with every sdb entry in config?
2. How can I achieve caching of loaded KDBX instance for all consecutive entries in config?

Setup

1. Add any custom module with heavyweight method, like this custom kdbx
2. Use it in config file:

somekey1: sdb://kdbx/a/b/c
somekey2: sdb://kdbx/a/b/d
somekey3: sdb://kdbx/a/b/e

Steps to Reproduce Issue

With aforementioned setup, run any runner module. It will take ages to run, e.g. salt-run saltutil.sync_all

Versions Report

Salt Version:
           Salt: 2019.2.2
 
Dependency Versions:
           cffi: 1.13.2
       cherrypy: unknown
       dateutil: 2.5.3
      docker-py: 1.10.6
          gitdb: 2.0.0
      gitpython: 2.1.1
          ioflo: Not Installed
         Jinja2: 2.10.3
        libgit2: 0.28.2
        libnacl: Not Installed
       M2Crypto: 0.35.2
           Mako: Not Installed
   msgpack-pure: Not Installed
 msgpack-python: 0.4.8
   mysql-python: Not Installed
      pycparser: 2.19
       pycrypto: 3.9.4
   pycryptodome: Not Installed
         pygit2: 0.28.2
         Python: 3.5.3 (default, Sep 27 2018, 17:25:39)
   python-gnupg: Not Installed
         PyYAML: 5.2
          PyZMQ: 16.0.2
           RAET: Not Installed
          smmap: 2.0.1
        timelib: Not Installed
        Tornado: 4.4.3
            ZMQ: 4.2.1
 
System Versions:
           dist: debian 9.11 
         locale: UTF-8
        machine: x86_64
        release: 4.9.0-8-amd64
         system: Linux
        version: debian 9.11 

[garethgreenaway]

@kiemlicz Thanks for the report. @saltstack/team-core Anyone with familiarity with sdb or the memoize decorator have any insight into this one?

[waynew]

@kiemlicz one quick way to check - edit salt/utils/decorators/__init__.py and change:

cache = {}
log.warning('Cache ID: %s', id(cache))

Then it should appear in your /var/log/salt/master log file. If you see a bunch of different IDs you'll know that it's creating a bunch of different caches.

The other place to look is in that same function:

        if args_ not in cache:

Change that to this:

        log.warning('Cache args: %s', args_)
        if args_ not in cache:
            log.warning('Args were not in cache!')

See if that helps narrow anything down. HTH!

[kiemlicz]

Thank you for your response.

Given master.d/sdb.conf and proposed changes

kdbx:
  driver: kdbx
  db_file: /tmp/dbfile
  password: redacted
  key_file: /tmp/keyfile

a: sdb://kdbx/a
b: sdb://kdbx/b
c: sdb://kdbx/c
d: sdb://kdbx/d
e: sdb://kdbx/e
f: sdb://kdbx/f
g: sdb://kdbx/g

The cache instance is always different, which explains why there is always cache miss

# salt 'minion' -t 180 saltutil.sync_all
[WARNING ] Cache args:
[WARNING ] Cache args:
[WARNING ] Cache ID: 140300234160200
[WARNING ] Cache args: {'key_file': '/tmp/keyfile', 'driver': 'kdbx', 'password': 'redacted', 'db_file': '/tmp/dbfile'}
[WARNING ] args were not in cache
[WARNING ] kdbx load
[WARNING ] Cache ID: 140300302901384
[WARNING ] Cache args: {'key_file': '/tmp/keyfile', 'driver': 'kdbx', 'password': 'redacted', 'db_file': '/tmp/dbfile'}
[WARNING ] args were not in cache
[WARNING ] kdbx load
[WARNING ] Cache ID: 140300299411848
[WARNING ] Cache args: {'key_file': '/tmp/keyfile', 'driver': 'kdbx', 'password': 'redacted', 'db_file': '/tmp/dbfile'}
[WARNING ] args were not in cache
[WARNING ] kdbx load
[WARNING ] Cache ID: 140300234671432
[WARNING ] Cache args: {'key_file': '/tmp/keyfile', 'driver': 'kdbx', 'password': 'redacted', 'db_file': '/tmp/dbfile'}
[WARNING ] args were not in cache
[WARNING ] kdbx load
[WARNING ] Cache ID: 140300419955016
[WARNING ] Cache args: {'key_file': '/tmp/keyfile', 'driver': 'kdbx', 'password': 'redacted', 'db_file': '/tmp/dbfile'}
[WARNING ] args were not in cache
[WARNING ] kdbx load
[WARNING ] Cache ID: 140300234869448
[WARNING ] Cache args: {'key_file': '/tmp/keyfile', 'driver': 'kdbx', 'password': 'redacted', 'db_file': '/tmp/dbfile'}
[WARNING ] args were not in cache
[WARNING ] kdbx load
[WARNING ] Cache ID: 140300234422216
[WARNING ] Cache args: {'key_file': '/tmp/keyfile', 'driver': 'kdbx', 'password': 'redacted', 'db_file': '/tmp/dbfile'}
[WARNING ] args were not in cache
[WARNING ] kdbx load
[WARNING ] Cache args:
...

1. Does it mean that memoize closure is bound to LazyLoader (salt.loader.sdb) and since salt.loader.sdb always creates new loader the cache is lost
2. Is it by design? Do I have any other options to cache something in memory for these consecutive operations?

[mchugh19]

@kiemlicz since each use of sdb creates a new instance, memonize isn't helpful, but you could look to use __context__ to hold information between each lookup. In https://github.com/saltstack/salt/pull/54094/files#diff-e75b02a233b726e8be8f614a7a554fd5R162 @thenoid cached the results of vault lookups, which sounds similar to what you are trying to accomplish.

[kiemlicz]

Thank you
Are you sure that the __context__ is retained between e.g. sdb instances?

I've tried using __context__, and changing https://github.com/kiemlicz/ambassador/blob/master/salt/base/_sdb/kdbx.py#L64 to:

def _load(kdbx_settings):
    log.warning("CTX:\n{}".format(id(__context__)))

prints different id of __context__, moreover the context is always empty.
Putting key/value there is lost.

I though that maybe I should move kdbx loading logic to _utils, however it didn't change a thing.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

[mchugh19]

I'm not a salt developer and not familiar with that area of the code, but my understanding is that __context__ is retained for the running minion session. So it should be the place to retain your cache data.

@waynew can this get a question label to get eyes on it, and calm down stalebot?

[stale]

Thank you for updating this issue. It is no longer marked as stale.

[waynew]

@mchugh19 IIRC it's not around for the whole minion lifetime, just for the entire call - like state.highstate the minion would get a __context__ at the start that stays around until the highstate has completed. It wouldn't be around between two separate calls to minions.

That is, if I understand correctly 😂

[kiemlicz]

Communication with minion is a one thing, surely related.
For me the issue is more about: apply_sdb (salt.config.apply_sdb) evaluated on the master side during config load upon any module execution (which doesn't retain the __context__)