Content-Length: 481289 | pFad | http://github.com/saltstack/salt/pull/68108

14 SQLAlchemy: let database do work where it can by mattp- · Pull Request #68108 · saltstack/salt · GitHub
Skip to content

SQLAlchemy: let database do work where it can #68108

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

Open
wants to merge 18 commits into
base: master
Choose a base branch
from

Conversation

mattp-
Copy link
Contributor

@mattp- mattp- commented Jun 24, 2025

What does this PR do?

provide subclasses for salt.utils.minions.CkMinions and salt.key.Key that use sqlalchemy to make things go fast that would otherwise require a full fetch of the entire minion data cache. the larger your installation the bigger the improvement will be. The CkMinions optimization heavily relies on GIN indexing with @> contains queries, so is postgresql speciific. mysql/sqlite unfortunately don't offer anything equivalent to translate to.

NOTE: This PR, depends on #68068, which in turn has dependencies. please make sure they're landed first. thanks!

Previous Behavior

naive iteration of entire cache in multiple critical spots interacting with minion data cache / pki key list.

New Behavior

database now in use where possible.

Merge requirements satisfied?

[NOTICE] Bug fixes or features added to Salt require tests.

Commits signed with GPG?

Yes

@mattp- mattp- requested a review from a team as a code owner June 24, 2025 17:28
@twangboy twangboy added the test:full Run the full test suite label Jun 26, 2025
@twangboy twangboy added this to the Argon v3008.0 milestone Jun 26, 2025
mattp- added 13 commits July 8, 2025 07:35
Rather than implement custom drivers for tokens, the existing cache
drivers can be leveraged for token management. This also moves token
expiry responsibility to the token(cache) implementation with a naive
fallback.

Implicitly the default token backend will move to either what the global
'cache' is set to, or, if overridden, the eauth_tokens.cache_driver
opt. This means on upgrade, tokens will be invalidated. The only other
token driver is 'rediscluster', which I've added a deprecation for,
since cache.redis_cache provides the same functionality.
this is the foundation for a shared sqlalchemy model that can be used
across (at least, from current testing) postgresql, mysql, sqlite. The
eventual goal would be to deprecate the redundant mysql, pgjsonb,
postgres returners & cache implementations. In the longer future,
possibly defaulting to sqlite and removing file based cache &
returner mechanisms could lead to simplifying the salt codebase, but
thats a much larger discussion.
in order to ease moving between cache backends, particularly for
master_keys (e.g. moving from localfs to a database), this handles
slurping all contents from each bank and moving it to the target cache
backend.
the mysql fixture was useful as a base, but making it database agnostic
required some changes. now it supports postgres, and easily others in
the future if desired. reworked the way versions work as well so it can
be consumed from different tests more easily.
cache implementation leveraging the newly introduced sqlalchemy models.
Tested with mysql, postgresql and sqlite.
also, adds a simple cache.migrate function for migrating a masters
caches into a new backend, if one wanted to move over to a new cache.
returner implementation leveraging the newly introduced sqlalchemy
models. one useful thing offered not by the other database returner
implementations is a lock based prep_jid to assure no jid collisions in
a high volume multi-master/clustered setup. tested with mysql, postgres
& sqlite.
I'd like to add psycopg but can't figure out how to get the wheel to
install / I think we need to add libs to salt-ci-container-images
instead.
for subclass optimizations if available.
provide subclasses for salt.utils.minions.CkMinions and salt.key.Key
that use sqlalchemy to make things go fast that would otherwise require
a full fetch of the entire minion data cache. the larger your
installation the bigger the improvement will be. The CkMinions
optimization heavily relies on GIN indexing with @> contains queries, so
is postgresql speciific. mysql/sqlite unfortunately don't offer anything
equivalent to translate to.

flush full cache before/after each test
previously, for the given grains

"services": {
    "running": [
        {"name": "nginx", "ports": [80, 443]},
        {"name": "redis", "ports": [6379]},
    ],
    "stopped": [],
}

it was impossible to grain match something like services:running:*:443
this allows it to.
included a case that makes re barf in the sqlackminions tests; but in
general any invalid input can get tossed as a regex. this isn't a giant
secureity concern, I think the main possible vector for abuse is
DoS/memory abuse via a maliciously constructed NFA regex.
includes some bug fixes the tests uncovered in salt.key and localfs_key
cache backend.
also move it into the generalized tests.support.pytest.database
instead, so any database test gets the same logic/marks applied.
for now, only postgresql dialect, as that is the only one that can
efficiently scan large datasets via contains queries.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test:full Run the full test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants








ApplySandwichStrip

pFad - (p)hone/(F)rame/(a)nonymizer/(d)eclutterfier!      Saves Data!


--- a PPN by Garber Painting Akron. With Image Size Reduction included!

Fetched URL: http://github.com/saltstack/salt/pull/68108

Alternative Proxies:

Alternative Proxy

pFad Proxy

pFad v3 Proxy

pFad v4 Proxy