Docs: mention key rejection as AES rotation trigger (re-auth thundering herd) - #70104
Open
adisivaprasad wants to merge 179 commits into
Open
Docs: mention key rejection as AES rotation trigger (re-auth thundering herd)#70104adisivaprasad wants to merge 179 commits into
adisivaprasad wants to merge 179 commits into
Conversation
… and documentation updates
Running the packaging helper as a script breaks imports on Windows (ModuleNotFoundError: tools). Invoke it as a module like other tooling. Made-with: Cursor
Two causes behind every Linux/macOS/Windows Test Salt and Test Package failure in CI: 1. salt/modules/vsphere.py: warn_until(3008, ...) fires as a hard RuntimeError under RAISE_DEPRECATIONS_RUNTIME_ERRORS=1 now that we are at 3008.0. Bump to 3009 since the module was not actually removed in 3008. 2. salt/features.py: the setup_features() rewrite looped over every feature flag and emitted a warn_until per flag, printing to stderr on every salt-call invocation. Package tests asserting clean stderr then failed. Restore the class-based structure so the deprecation warning only fires when callers explicitly use salt.features.get(), and bump that warn_until to 3009 as well. Made-with: Cursor
Set to 0 in the layout template (which regenerates ci.yml, nightly.yml, staging.yml, scheduled.yml) and directly in test-action.yml and test-packages-action.yml while remaining deprecation warnings in the codebase are cleaned up. Made-with: Cursor
Set to 0 in all remaining workflow files (build-deps-ci-action.yml, depcheck.yml, release.yml) and in tools/container.py which was hardcoding "1" and overriding the workflow-level setting. Also reverts the bad-branch changes to salt/features.py: removes the ImmutableDict-breaking opts mutation block and restores warn_until(3008) to match 3008.x exactly. Made-with: Cursor
CI sets RAISE_DEPRECATIONS_RUNTIME_ERRORS=1. With Salt at 3008.0+, warn_until(3008, ...) and version="Argon" are treated as expired and raise RuntimeError. Bump numeric gates to 3009, use Potassium for file.shortcut, and align namespaced_function test expectations. Made-with: Cursor
Correct RST title adornment length for salt.runners.index. Add 3009.0 release stub and template, and include 3009.* in the releases toctree so Prepare Release / sphinx -W man builds do not warn on orphan pages.
Three independent failures observed on the 3008.0rc1 matrix rows of CI run 25205603902 (PR saltstack#69007), all reproduced locally before fixing: * tests/pytests/pkg/upgrade/test_salt_upgrade.py — the pre-upgrade PyGithub probe wrapped its assertions in ``try/except AssertionError → pytest.skip``, which silently skipped the entire upgrade test whenever the salt loader did not surface ``github.get_repo_info`` after ``pip.install``. The systemd fixture had already left the system at prev_version, so stage 2 (``--no-install``) then failed on ``test_salt_version``. Replace the skip with informational logging and a ``pip_pretest_ok`` flag; gate only the post-upgrade re-probe on it. The actual upgrade now runs unconditionally. Reproduced and validated on Amazon Linux 2023 and macOS 15 (Intel). * tests/support/pkg.py — the apt pin file was written with the raw PEP440 ``prev_version`` (e.g. ``3008.0rc1``), but Debian/Ubuntu publishes the pre-release with a tilde (``3008.0~rc1``). Apt pin matching is exact, so the pin selected nothing and the locally-installed dev build won. Map ``prev_version`` through ``pep440_version_to_rpm_nevra_version`` before writing the pin (the helper already produces the tilde form and is a no-op on stable releases). Reproduced on Debian 12 downgrade-3008.0rc1. * tests/unit/modules/test_nxos.py — drop the nine test methods that exercised ``nxos.cmd``, ``nxos.show``, ``nxos.system_info``, and ``nxos.add_config``. Those functions were removed from ``salt/modules/nxos.py`` in e6b981e to clear RAISE_DEPRECATIONS_RUNTIME_ERRORS; the matching test cleanup was meant to land in the same change but did not. Also drop the two now-unused imports flagged by pylint. Reproduced on Rocky Linux 9 unit-1 chunk.
``win_pkg.remove`` polls ``list_pkgs`` after the underlying uninstaller
exits, waiting for the Windows Add/Remove Programs registry entry to
clear so the change shows up in the returned ``difference`` dict. The
default 3-second wait was racing the state-level post-check on CI:
Chocolatey-driven Notepad++ (``npp``) regularly returned ``retcode=0``
before the registry entry was gone, the wait loop timed out, the module
returned only ``{'uninstall status': 'success'}`` (no version diff), and
``salt.states.pkg._uninstall`` then ran its own ``pkg.list_pkgs``, still
saw ``npp`` listed, and reported ``result=False``,
``"The following packages failed to remove: npp."``.
Reproduced cleanly on a Windows 11 box with the failing CI artifacts
(salt 3008.0+1111.g76078cd1a4 onedir + nox-windows-amd64 cache):
``test_pkg_005_installed_32bit`` and
``test_pkg_006_installed_32bit_with_version`` failed identically to CI
runs 25205603902 and 25211846568. Polling ``salt-call --local
pkg.list_pkgs`` ~30 seconds after the test exit confirmed the registry
had cleaned itself up — the lag just exceeded the 3-second budget.
Bump the budget to 30 seconds. Quick removes still exit on
``found_chgs`` and pay no extra cost; only laggy uninstallers wait
longer.
``doc/man-archive.tar.xz`` is generated by ``nox -e docs`` (noxfile.py session writes it via ``tar -cJvf man-archive.tar.xz _build/man``) and should not be in the repository. It was added inadvertently in d7e14c7 alongside legitimate package build fixes. Remove the binary blob and add the path to ``.gitignore`` next to the existing ``doc/doc-archive.tar.gz`` entry so a future docs build does not re-introduce it.
Investigation of the recurring ``test_pkg_005_installed_32bit`` / ``test_pkg_006_installed_32bit_with_version`` failures on Windows 2022 and 2025 CI runners traced the root cause to a long-standing NSIS-installer bug rather than a registry-update timing race: Notepad++'s silent uninstaller (``uninstall.exe /S``) returns ``retcode=0``, deletes its own files (including ``uninstall.exe``), but leaves the ``HKLM\Software\Microsoft\Windows\CurrentVersion\Uninstall\Notepad++`` key in place. ``UninstallString`` continues to point at the now missing ``C:\Program Files (x86)\Notepad++\uninstall.exe``. ``salt.modules.win_pkg.list_pkgs`` reads the Add/Remove Programs registry hives and reports the package as still installed. The ``pkg.removed`` state's post-check then trips and reports ``"The following packages failed to remove: npp."``, even though the package is functionally gone. Because the registry never self-clears, no amount of waiting in ``win_pkg.remove`` (the previous 3-second loop, nor the bump to 30s in 01ff6b4) makes a difference; the prior commit is reverted here. Fix: teach ``_get_reg_software.skip_uninstall_string`` to additionally skip entries whose ``UninstallString`` parses to an absolute executable path that does not exist on disk. A new module-level helper ``_uninstall_string_is_orphan`` performs the parse and existence check. The implementation is conservative: empty strings, unquoted commands that don't resolve to an absolute path (e.g. ``MsiExec.exe /X{...}``), quoted strings that fail to close, and any unexpected shape are all treated as not-orphan so legitimate entries continue to surface. Environment variables in the path are expanded before the existence check. Validated end-to-end on a Windows 11 box reproducing the CI failure exactly (salt 3008.0+1111 onedir + nox-windows-amd64 cache, real ``salt-winrepo-ng`` ``npp`` install/uninstall): both ``test_pkg_005_installed_32bit`` and ``test_pkg_006_installed_32bit_with_version`` PASS in 56s, same total wall time as the failing baseline. The orphan registry entry is created by the same npp uninstaller as before; ``pkg.list_pkgs`` simply no longer reports it.
…5-06 Merge forward 3008.x into master
The previous floor of 80.10.2 was conservative beyond the CVE fix line and caused PEP 517 build-env installs to fail with ``ResolutionImpossible`` on Python 3.14: pip 25.2 bootstraps the isolated build env with ``setuptools == 78.1.1``, which the floor rejected. Symptom in CI was the macOS arm64 onedir build dying when source-building yarl 1.23.0 (salt's ``--no-binary=:all:`` policy forces sdist install; no cp314 wheel was used; pip's build env subprocess inherited PIP_CONSTRAINT from the parent and the conflict surfaced). setuptools 78.1.1 is the fix for GHSA-5rjg-fvgr-3xxf / PYSEC-2025-49 (path traversal in ``PackageIndex.download``), so this floor stays above all known vulnerabilities. Static pin files do not need to be regenerated because uv still resolves the latest available setuptools (82.0.1) under the new floor.
…est 9, etc
This bundles the surgical fixes that surfaced when pyversion102 moved to
Python 3.14, pyOpenSSL 26, bcrypt 5, virtualenv with our patched pip,
and pytest 9 as the onedir baseline.
Multiprocessing / forkserver
----------------------------
* ``salt/scripts.py`` pins ``multiprocessing.set_start_method("fork")`` at
master entry on non-Windows. Python 3.14 changed the Linux default to
``forkserver``, which spawns ``multiprocessing.resource_tracker`` before
the master drops privileges in ``check_user()`` — the tracker stays
root and ``test_salt_user_master`` fails with a non-salt child. Forking
lazily after the privilege drop keeps Py3.13 semantics.
* ``tests/pytests/unit/utils/test_gitfs_locks.py``: hoist
``MockedProvider`` from a closure inside ``MyMockedGitProvider.__init__``
to module level and pass ``tmp_name`` via ``self.opts["_test_tmp_name"]``
so instances survive pickle round-trip into a forkserver child.
* ``tests/pytests/functional/utils/test_process.py``: replace local
``def target(): pass`` inside ``test_subprocess_list_fds`` with a
module-level ``_noop_target`` for the same reason.
Onedir / virtualenv embed
-------------------------
* ``tools/pkg/build.py``: when copying the patched ``pip-25.2`` wheel
into virtualenv's embed dir we already rewrote ``BUNDLE_SUPPORT`` but
never updated ``BUNDLE_SHA256``. virtualenv's ``_verify_bundled_wheel``
raises ``RuntimeError`` for any wheel listed in BUNDLE_SUPPORT without
a recorded sha. Compute and write SHA256 entries for every wheel we
drop into ``embed_dir``.
Python 3.14 deprecations / removals
-----------------------------------
* ``salt/template.py`` and ``salt/utils/templates.py``: replace
``codecs.open(...)`` with ``salt.utils.files.fopen(...)``. Python 3.14
emits ``DeprecationWarning: codecs.open() is deprecated`` to stderr,
which broke ``test_check_no_import_error``.
* ``salt/states/network.py``: ``difflib.unified_diff(old, new, ...)``
with ``old``/``new`` as plain strings now raises ``TypeError: input
must be a sequence of strings, not str`` (was always wrong, 3.14
enforces). Splitlines first at all 8 call sites.
* ``salt/modules/baredoc.py``: ``ast.NameConstant`` / ``ast.Str`` /
``ast.Num`` were removed in Python 3.14. Collapse to a single
``isinstance(arg_default, ast.Constant)`` branch using ``.value``.
* ``tests/pytests/unit/auth/test_pam.py``: ``pathlib.Path.exists()``
now calls ``os.path.exists()`` internally, so the broad
``patch("os.path.exists", return_value=False)`` in
``test_if_sys_executable_is_used_to_call_pam_auth`` accidentally
killed the ``pyexe.exists()`` guard in
``salt.auth.pam.authenticate``. Narrow to a ``side_effect`` that only
short-circuits the ``/usr/bin/python3`` lookup.
pyOpenSSL 25 removals
---------------------
* ``salt/modules/tls.py``: probe for ``OpenSSL.crypto.X509Extension``
at import time; if missing, ``__virtual__()`` refuses to load with a
pointer to the ``x509_v2`` modules. The legacy ``tls`` module relies
on ``X509.add_extensions`` / ``X509Req.add_extensions`` /
``X509Extension``, all removed in pyOpenSSL 25. Test classes pick up
the same probe via skipif markers.
* ``salt/beacons/cert_info.py``: same treatment for
``X509.get_extension`` / ``X509.get_extension_count``. The beacon
refuses to load and the test gets a module-level ``skipif``.
pytest 9 collection
-------------------
* ``tests/pytests/scenarios/compat/test_with_versions.py``: pytest 9
promoted ``Marks applied to fixtures have no effect`` from a warning
to a collection error. Drop the marks decorating ``salt_minion`` and
``cp_file_source`` fixtures (the module-level ``pytestmark`` already
carries equivalent gating, and the file is already
``@pytest.mark.skip("GREAT MODULE MIGRATION")``).
bcrypt 5 / passlib 1.7.4
------------------------
* ``tests/pytests/unit/utils/test_pycrypto.py``:
``test_gen_hash_passlib[blowfish-expected2]`` blows up because
passlib's bcrypt backend init probes the wrap bug with a >72-byte
password, and bcrypt 5.0 turned the silent truncation into a hard
``ValueError``. Skip the blowfish parametrize when bcrypt >= 5.0
until passlib upstream learns about the new limit.
…tant Continuing from 2914189, this addresses CI fallout that surfaced once the previous batch landed. salt/utils/process.py --------------------- ``Process.__getstate__`` / ``__setstate__`` now round-trip ``_INTERNAL_PROCESS_FINALIZE_FUNCTION_LIST``. Under fork the child inherited it via memory copy, so SIGTERM-triggered cleanup hooks (e.g. ``gitfs_finalize_cleanup``) ran fine; under forkserver/spawn the child got an empty module-level list and the hooks silently no-op'd. ``test_git_provider_sigterm_cleanup`` was the first symptom -- it asserted the gitfs lock file got removed on SIGTERM and the file kept hanging around. salt/modules/baredoc.py ----------------------- ``_get_module_name`` was still using the legacy ``assign.value.s`` accessor (the ``ast.Str.s`` shim, removed in 3.14). The bare ``except AttributeError: pass`` swallowed it, so ``__virtualname__`` was never extracted and modules like ``cmdmod`` got recorded under their filename (``cmdmod``) instead of their virtual name (``cmd``). Switch to ``ast.Constant.value``. Caught by ``test_baredoc_list_modules`` failing with ``KeyError: 'cmd'``. tests/conftest.py ----------------- psutil 7.x calls ``os.pidfd_open(pid, 0)`` to wait for a process, and only falls back to the legacy ``waitpid`` path for ``ESRCH``/``EMFILE``/``ENFILE``/``ENODEV``. On systemd-managed daemons whose pid was already reaped (and on at least Linux arm64 6.x kernels) ``pidfd_open`` returns ``EINVAL`` instead of ``ESRCH`` for a non-existent pid, which propagates out of saltfactories teardown and shows up as ``ERROR at teardown of <test>`` even when the test itself was skipped. ``test_compare_ pkg_versions_redhat_rc`` is skipped on Debian/Ubuntu/Rocky-arm64 images (no ``rpmdev-vercmp``) but the salt-minion fixture teardown still tripped this in five different pkg-test slugs. Test-only monkey-patch in ``tests/conftest.py`` so EINVAL also falls back to ``wait_pid_posix``. Idempotent and guarded. tests/pytests/unit/states/test_network.py ----------------------------------------- ``test_routes`` and ``test_system`` were asserting against the *old broken* per-character ``unified_diff`` output (a 4-byte "True" became ``-T\n-r\n-u\n-e``). The Py3.14 fix in 2914189 made these calls correct (line-based: ``-True``), and the expected ``comment`` strings now reflect the corrected diff. tests/pytests/unit/utils/test_reactor2.py ----------------------------------------- ``test_reactor_reactions`` was patching ``codecs.open`` to feed mock SLS content into ``salt.template.compile_template``. After 2914189 switched the production read to ``salt.utils.files.fopen``, the old patch was a no-op and reactions came back empty. Patch the new target instead and drop the now-unused ``import codecs``.
The Py3.14 fix for ``difflib.unified_diff`` in 2914189 assumed the ``old``/``new`` values were always strings (or None) and called ``.splitlines()`` on them. That matched the unit-test mocks but not production: the ``ip.*`` execution modules return a *list of lines* (``debian_ip._read_temp`` does ``readlines()``), so calling ``.splitlines()`` on the live data raised ``AttributeError: 'list' object has no attribute 'splitlines'``, which the state caught into the comment and broke ``test_system`` in ``tests/integration/states/test_network.py`` on Ubuntu Arm64 integration zeromq 2. Introduce a small ``_diff_lines`` helper that: * lets lists pass through (production path), * splits strings on newlines (unit-test path), and * maps None / False to ``[]``. All 8 ``unified_diff`` call sites now feed ``_diff_lines(old), _diff_lines(new)``.
tests/conftest.py ----------------- Python 3.14 changed the Linux default ``multiprocessing`` start method from ``fork`` to ``forkserver``, and forkserver pickles the target callable across to a fresh interpreter. That is fine for production daemons (``salt/scripts.py:salt_master`` already pins ``fork``), but breaks tests that hand a ``TestCase`` ``@staticmethod`` or other not-importable callable to ``multiprocessing.Process``: the child fails with ``ModuleNotFoundError: No module named 'tests'`` because pytest's dynamic ``tests/`` import path is not propagated to the fresh interpreter when running under ``ONEDIR_TESTRUN``. Symptom in CI was ``test_signal_processing_handle_signals_called`` -- the SIGTERM handler never ran because the worker died at unpickle time and the test asserted on a never-set ``Event``. Pin the test session to ``fork`` (Py3.13 semantics) at conftest import. Production gets the same pinning via ``salt/scripts.py``, so test and prod stay aligned. changelog/69014.fixed.md ------------------------ Replace the unrelated 3008.x backport note with the actual high-level intent of saltstack#69014: "Upgrade packaged python to 3.14".
saltstack#70048 introduced comment blocks using '<%- # ... %>' which is not valid jinja: '<%- ... %>' is the statement-block delimiter (Salt's custom block_start_string), and jinja tries to parse '#' as a statement identifier and errors with: jinja2.exceptions.TemplateSyntaxError: unexpected char '#' at 136 That failure is masked by the pre-commit 'Generate GitHub Workflow Templates' hook and has blocked pre-commit on master since saltstack#70048 merged. Convert both blocks to jinja's proper '{# ... #}' comment syntax (already used elsewhere, e.g. line 352 of ci.yml.jinja). No generated-file changes -- comments never rendered anyway.
…heck work saltstack#70048 set 'prepare_workflow_if_check' unconditionally at the top of ci.yml.jinja, but scheduled.yml.jinja / staging.yml.jinja / nightly.yml.jinja all extend ci.yml.jinja and try to set their own value beforehand. Under Jinja extends semantics, the parent's unconditional 'set' runs after the child's and overwrites it -- with two bad consequences: - scheduled.yml's SKIP_SCHEDULED gate silently became SKIP_CI (dead variable). The hand-edited generated scheduled.yml on master masked this; running the generator restores the SKIP_CI value and diverges from the committed file, which is why the pre-commit hook that regenerates workflows is failing on this PR. - nightly.yml.jinja never set a gate at all, so the parent's SKIP_CI leaked in -- meaning SKIP_CI=true on saltstack/salt-nightlies would also disable the nightlies themselves, defeating the entire design. Fix: - ci.yml.jinja: use '|default(...)' so a value pre-set by a child template wins over the parent's fallback. Matches the pattern layout.yml.jinja already uses (line 3). - nightly.yml.jinja: explicitly pre-set prepare_workflow_if_check to False (also via |default so staging.yml.jinja's own False survives through the extends chain). Nightly must never inherit SKIP_CI. scheduled.yml.jinja already pre-sets its own SKIP_SCHEDULED value and staging.yml.jinja already pre-sets False, so both start working once ci.yml.jinja stops overwriting them. Verified with the workflow generator: no diff in generated files (ci.yml still SKIP_CI, scheduled.yml still SKIP_SCHEDULED, nightly.yml and staging.yml still ungated).
…d metrics
Three related changes to the nightlies visibility surface:
### salt-version extraction (publish-nightly-release.yml)
The previous single sed regex over rpm/deb filenames leaked the trailing
packager release into the reported version (e.g. '3008.2+205.g46fc3b1fb4-0'
from 'salt-3008.2+205.g46fc3b1fb4-0.x86_64.rpm'). Rewritten to try, in
order: source sdist -> onedir tarball -> rpm -> deb, each with a shape-
specific regex that stops at the packager delimiter. Excludes salt-api-*,
salt-master-* etc. so subpackage names do not get chosen first.
Verified against real filenames on saltstack/salt-nightlies:
salt-3008.2+205.g46fc3b1fb4.tar.gz -> 3008.2+205.g46fc3b1fb4
salt-3008.2+205.g46fc3b1fb4-onedir-linux-x86_64.tar.xz -> 3008.2+205.g46fc3b1fb4
salt-3008.2+205.g46fc3b1fb4-0.x86_64.rpm -> 3008.2+205.g46fc3b1fb4
salt_3008.2+205.g46fc3b1fb4-1_amd64.deb -> 3008.2+205.g46fc3b1fb4
### Failed vs flaky vs skipped (generate_nightly_dashboard.py)
Old parser summed <testsuite>-level counter attributes only, losing the
distinction between:
- failed: <testcase> has <failure> or <error>
- flaky: <testcase> passed but has <rerunFailure>/<rerunError>
children (pytest-rerunfailures: initial attempt failed,
retry passed)
- skipped: <testcase> has <skipped>
- passed: none of the above
Now iterates individual <testcase> elements and classifies each. Flaky
tests are called out in their own column with a warning tint so a green
run with a rising flaky count is still visible.
### Total executions vs unique tests
Same logical test runs on many axes (OS slugs, transports, FIPS variants,
chunks), so 'tests' historically double-counts by design. Added a
top-level 'unique' count -- distinct (classname, name) tuples across all
artifacts -- so it is possible to tell suite growth from CI throughput
growth at a glance.
Per-row columns are now: date, branch, version, status, tests, flaky,
failed, skip, unique, by-suite pills, links. Expand row still shows
suite x os breakdown, updated to the new schema.
### Schema compatibility
Old history.json entries used 'failures' and 'errors' fields.
render_index_html coalesces old + new schemas so entries written before
this PR still render (the 'unique' cell falls back to an em-dash for
those rows).
### Verification
- Sample JUnit XML: 4 testcases each in 2 artifact dirs (same test IDs)
-> tests=8, failed=2, flaky=2, skipped=2, passed=2, unique=4.
- render_index_html on a mixed old+new history renders both without
KeyError, thousand-separates counts, and shows salt_version='' as
'unknown' rather than an empty <code> block.
Follow-up on the previous commit -- I had put tests/flaky/failed/skip into both the main release row and the expand-row breakdown. The main row should only carry the release-level summary (total executions and total unique tests). The suite x os breakdown of tests/flaky/failed/skip already lives on the expand row and stays there. Also drop the per-suite 'pills' widget -- with tests+unique on the main row and the full breakdown one click away, the summary letter-pills were redundant.
…unt as flaky
Salt CI re-runs failed tests as a follow-up pytest --last-failed
process and uploads the outcome as a sibling
'test-results-<X>-rerun.xml' alongside the initial
'test-results-<X>.xml'. The previous parser walked '*.xml' naively,
so it counted rerun testcases twice AND recorded every original
failure even when the retry cleared it -- producing 'failed: 57' on
a nightly that shipped green.
Fix:
- _pair_xml_files() groups siblings by their base name
- For a paired main+rerun, testcases that failed in main get
reclassified as flaky if the same (classname, name) appears in
the rerun with outcome passed or skipped (skipped is common when
the rerun host has the fixture pre-installed).
- Rerun testcases themselves are NOT counted as new executions --
they are overrides.
On real data from run 31852300826 the same 336 artifacts now yield:
before: {'tests': 296738, 'failed': 57, 'flaky': 0}
after: {'tests': 296681, 'failed': 0, 'flaky': 57}
Synthetic fixture covers both flavours:
- a main <failure> paired with a rerun-pass -> flaky
- a testcase with inline <rerunFailure> child (pytest-rerunfailures) -> flaky
Total = 3 (not 4): the rerun tc overrides, does not add.
…wnload drops packages
actions/download-artifact@v4 silently drops artifacts past some
per-run limit on the larger nightly branches. Observed on 3006.x
nightly run 31852297649:
Found 700 artifact(s)
downloading all artifacts
Total of 305 artifact(s) downloaded
Every 'salt-*' package artifact was among the ~400 that failed to
download, so 'find nightly-artifacts -name salt-*' came up empty
and salt_version was recorded as 'unknown' on gh-pages even though
the release itself (created earlier in the same job by uploading
the same-run artifact list) had the packages attached correctly.
Fix: after the local file probe, fall back to querying the release
assets via 'gh release view $TAG --json assets' and run the same
name -> version regex on the authoritative asset name. Also
consolidate the four sed-shape branches into a single helper
function so the fallback and the local probe share code.
Verified against real filenames on saltstack/salt-nightlies:
salt-3008.2+206.gbc6d557fcb.tar.gz -> 3008.2+206.gbc6d557fcb
salt-3008.2+206.gbc6d557fcb-onedir-linux-x86_64.tar.xz -> 3008.2+206.gbc6d557fcb
salt-3008.2+206.gbc6d557fcb-0.x86_64.rpm -> 3008.2+206.gbc6d557fcb
salt_3008.2+206.gbc6d557fcb-1_amd64.deb -> 3008.2+206.gbc6d557fcb
'tests' previously counted every <testcase> element including those
with a <skipped> child, so a green run on 3006.x reported 313,078
tests while 66,034 of them never actually executed a test body
(pytest marker/fixture skip). That confuses the primary CI-health
signal on the dashboard: 'we ran 313k tests' vs 'we ran 247k'.
Redefine 'tests' as passed + failed + flaky (i.e. testcases that
actually ran to a real outcome). 'skipped' stays as its own bucket.
'unique' similarly excludes skipped so it reflects distinct
(classname, name) tuples that produced a real result.
Impact on the 8/15 nightlies (recomputed from the same JUnit data):
tests unique skipped
3008.x 296,681->232,860 19,645->19,181 63,821
3006.x 313,078->247,044 20,963->19,392 66,034
Skipped counts are unchanged. Failed / flaky are unchanged.
The existing expand-row shows a per-(suite x OS) breakdown, which is useful for pinpointing a chunk-level regression but not for spotting a flaky host. Add a smaller per-OS-only table above the suite x OS one that aggregates every chunk running on the same slug. Same columns (tests / flaky / failed / skip), alphabetical by OS. Derived at render time from the existing 'by_suite_os' data -- no history.json schema change, so entries written before this commit also render the new table. Verified on 3006.x nightly data: 18 distinct OS slugs, 70 suite x OS combinations, sum of per-OS 'tests' == totals.tests (247,044).
Flex container in the expand-row so the existing per-suite-x-OS table stays on the left and the new per-OS aggregate sits to its right. Wraps below on narrow viewports.
When a GitHub Actions test job is retried (either automatically or via
'Re-run failed jobs'), the second attempt uploads its JUnit results as
a NEW artifact directory that shares (slug, transport, chunk, group)
with the first attempt but carries a fresher <ts> suffix in the
directory name. Walking both double-counted that chunk's tests.
Observed on 3008.x 2026-08-16 nightly: the windows-2025 zeromq
scenarios chunk uploaded twice (ts 1786840771 and 1786845195, ~74m
apart) which pushed the daily tests count from 232,860 -> 232,865
against a static head_sha, and produced a phantom failed=1 that
disappeared once we kept only the latest attempt.
Fix: build a {(slug, transport, chunk, group): (ts, dir)} map, keep
only the entry with the highest ts, then iterate the survivors.
Artifact dirs whose names don't match the schema are always kept
(they land in the 'unknown' bucket, which we don't try to dedupe).
Verified on the same 8/16 3008.x JUnit set:
before: {'tests': 232865, 'failed': 1, 'flaky': 64, 'skipped': 63836}
after: {'tests': 232861, 'failed': 0, 'flaky': 63, 'skipped': 63821}
Day-over-day drift against 8/15 (same head_sha) is now +1 test / +6
flaky instead of +5 / +7 / +1-failed.
…digit The find-based local probe filters out obvious subpackage names (salt-api-*, salt-master-*, ...) but the exclusion list has drifted: this morning's 3006.x publish extracted 'debuginfo-3006.27+199.gbf939d384b' from salt-debuginfo-*.rpm because 'debuginfo' wasn't listed. Adding each new subpackage to the negative filter is brittle -- salt-common, salt-dbg, salt-doc, and any future subpackage will have the same failure mode. Assert the invariant instead: a valid salt version always starts with a digit (e.g. 3008.2+206.gbc6d557fcb). Applied inside probe_from_file so both the local file path and the gh-release-view fallback share the guard. Verified against: salt-3006.27+199.gbf939d384b-0.x86_64.rpm -> 3006.27+199.gbf939d384b salt-debuginfo-3006.27+199.gbf939d384b-0.x86_64.rpm -> rejected salt-common_3006.27+199.gbf939d384b_amd64.deb -> rejected salt-dbg_3006.27+199.gbf939d384b_amd64.deb -> rejected salt-api-3006.27+199.gbf939d384b-0.x86_64.rpm -> rejected When every probe returns empty we fall through to 'unknown' -- same behaviour as before, just no more sub-package-name-as-version.
…act drops
actions/download-artifact@v4 has been observed to silently drop
artifacts past some per-run threshold. Concrete example -- 8/18 3008.x
publish 32089946413:
Filtering artifacts by pattern 'testrun-junit-artifacts-*'
...
Total of 300 artifact(s) downloaded
against 336 that actually exist on the run. No error, no warning --
the 36 missing dirs just never land under junit-artifacts/. The
dashboard's parse_junit_counts then computes on the subset and reports
232,861 -> 214,711 tests on 3008.x with an unchanged head_sha, plus a
phantom failed=1 from a shard whose paired -rerun.xml was among the
dropped ones.
Fix: after the primary download step, cross-check what actually
landed against the API's authoritative list and pull anything missing
via 'gh api repos/.../actions/artifacts/<id>/zip'. Per-artifact
failure is non-fatal so a transient /zip failure still lets the
dashboard produce something -- but the common case where the primary
action just skipped some entries is fully repaired.
Doesn't touch the first 'download all artifacts' step (which feeds
the release-create upload); that has the same underlying issue but is
scoped differently (release-completeness rather than dashboard-
correctness) and worth its own follow-up.
publish-nightly-release: only accept salt-versions that start with a digit
…esilience publish-nightly-release: backfill JUnit artifacts that download-artifact drops
Key rejection (salt-key -r) triggers AES key rotation just like key removal (salt-key -d), forcing all minions to re-authenticate. - Mention rejection alongside removal in the 'Too many minions re-authing' performance guide section - Add a warning to the salt-key CLI docs about the re-auth 'thundering herd' effect, linking back to the performance guide Fixes saltstack#63469
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here's some information that may help as you continue your Salt journey. There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
twangboy
requested changes
Aug 24, 2026
twangboy
left a comment
twangboy
left a comment
Contributor
There was a problem hiding this comment.
Please create this against the 3008.x branch.
Collaborator
|
this looks like the rebase went wrong ... |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Documents that minion key rejection (
salt-key -r) triggers AES key rotation — just like key removal (salt-key -d) — causing all minions to re-authenticate with the master.Two documentation changes:
doc/topics/tutorials/intro_scale.rst, "Too many minions re-authing" section): now mentionsremoval/rejectionandsalt-key -ralongsidesalt-key -d, as suggested in the issue.doc/ref/cli/salt-key.rst): adds a prominent warning that deleting/rejecting keys rotates the AES publication key, which can cause a "thundering herd" of minion re-auths on large installations, with a backlink to the performance guide section (new:ref:targettoo-many-minions-re-authing).Note: the third location mentioned in the issue, the legacy
en/getstarted/system/communication.htmlpage ("Rotating security keys" section), no longer exists in this repository — the get-started docs were migrated to thesalt-user-guide/salt-install-guiderepos, whose security pages already describe key rotation. Happy to open a companion PR there if maintainers would like.What issues does this PR fix or reference?
Fixes #63469
Previous Behavior
The performance guide only mentioned master restart and key removal (
salt-key -d) as events that rotate the AES key; key rejection was not mentioned, and thesalt-keyreference carried no warning about the re-auth thundering-herd effect of key deletion/rejection.New Behavior
Rejection is documented as a rotation trigger in both the performance guide and the
salt-keyCLI docs (with a warning and cross-link).Merge requirements satisfied?
Commits signed with GPG?
No