Skip to content

DEBUG: temporary ARG_MAX tracing for #70085 investigation (do not merge) - #35

Closed
twangboy wants to merge 204 commits into
fix/61338/3008.xfrom
debug/61338-argmax-trace
Closed

twangboy wants to merge 204 commits into
fix/61338/3008.xfrom
debug/61338-argmax-trace

Conversation

@twangboy

Copy link
Copy Markdown
Owner

Scratch PR to capture the real argv size of the failing salt-ssh state.apply command on Photon OS / Arm64 runners. Adds a temporary stderr print in Shell._run_cmd(). Not for merging; will be closed once we have the log.

ggiesen and others added 30 commits July 26, 2026 05:00
…ltstack#69758)

zypper returns exit code 104 when a search matches nothing. The
search-style calls in zypperpkg (search, _get_visible_patterns,
_get_patches, and Wildcard._get_available_versions) run through
cmd.run_all, whose default success_retcodes is [0], so the 104 was
logged as a command failure even though the search itself completed
normally and correctly reported that nothing was found.

Add an ignore_not_found option to the _Zypper wrapper that whitelists
104 via success_retcodes for those search calls, so the exit code is
treated as success and no ERROR is emitted. The found case (0) and all
genuine error codes are left untouched.

Fixes saltstack#58551
…saltstack#67975) (saltstack#69813)

* Support dnf5 group list/info in pkg.group_list and pkg.group_info (saltstack#67975)

Backport of the dnf5 group support from saltstack#67975 to 3006.x, grafted onto the
current group functions so the saltstack#60276 multi-word member-group @-fallback is
preserved.

dnf5 changed the "group list" and "group info" output formats, which the
yum/dnf parser did not understand, so the group functions (and
pkg.group_installed) returned empty or wrong data on Fedora 41+ and
RHEL/AlmaLinux 10.

- group_list: parse the dnf5 "group list --hidden" table by tokenizing each row
  (ID plus the trailing yes/no Installed column) instead of a regex, so a group
  name that contains or ends in the word "yes"/"no" is not mistaken for the
  status column, and a row with trailing whitespace is not dropped.
- group_info: parse the dnf5 "group info" format, where each package section
  carries its first member inline after the colon. That inline member is pulled
  into a dedicated variable rather than mutating the loop's line, blank members
  are skipped, and section lines are fully stripped on both ends.

dnf5 environment and language groups live under a separate "environment"
subcommand and are out of scope here, so those keys stay empty on dnf5.

Validated end to end against live dnf5 5.4.2.1 on Fedora 44: group_list parses
all 157 groups and group_info parses every package with none dropped.

Co-authored-by: Greg Oster <oster@fween.ca>

* Drop the spurious empty optional package from the group_info test

The empty-member guard added in this PR correctly stops a blank line in the
dnf/yum "groupinfo" output from being recorded as an empty-string package, so
the existing test_group_info expectation no longer includes the leading "" in
the gnome-desktop optional list.

---------

Co-authored-by: Greg Oster <oster@fween.ca>
…ooks

Two coupled fixes that keep grouped pip-updates PRs from failing on 3006.x:

1. Python 3.9 caps. Dependabot raises the shared floor of several packages
   to a release that no longer supports Python 3.9 (jaraco.functools 4.5.0,
   jaraco.context 6.1.2, msgpack 1.2.1, more-itertools 11.0.0, pycparser
   3.0, pythonnet 3.1.0, virtualenv 21.5.1, xmldiff 3.0, zipp 4.1.0). With a
   single unmarked floor the py3.9 lock targets become unresolvable. Split
   each into a python_version < '3.10' branch capped at the last
   3.9-compatible release plus an open py>=3.10 branch, mirroring the
   existing cryptography/aiohttp/urllib3 splits. (cryptography and pyopenssl
   already carry all-Python caps here, so they need no split.)

2. Malformed pip-compile hooks. The Py3.13 ZeroMQ hooks (linux/freebsd/
   darwin/windows) and the docs hook were missing their `- id: pip-compile`
   line, so YAML folded them into the preceding Py3.14 blocks as duplicate
   keys and the Py3.14 CI locks never regenerated. Restore the missing id
   lines, correct the py3.14->py3.13 file globs, and regenerate the affected
   Py3.14 locks.
The zenoss.monitored state set ret["changes"] to None on the
already-monitored and failed-add code paths. The state output
validator (OutputUnifier.content_check) requires changes to be a
dictionary and raises "'Changes' should be a dictionary.", so both
paths failed with result=False and that exception comment. Return an
empty dict, which correctly means "no changes" and passes validation.

Fixes saltstack#53966
_gen_keep_files filtered requisites with `"file" in comp`, which for a
bare-string requisite ID degraded to a substring match. Any ID containing
the substring "file" then hit `comp["file"]` and raised
"TypeError: string indices must be integers". Guard the membership test
with an isinstance check so only dict requisites are considered; bare
strings are ignored instead of crashing.

Fixes saltstack#53692 and Fixes saltstack#61042
The '^' list-override marker was only stripped when the target dict
already contained a matching list to override. When a class defined an
override list with no prior list present, dict_merge plain-assigned the
value (including the nested "pillars" dict) by reference and never
descended to the list, leaving a literal '^' element in the merged
pillar. Descend into dicts and honour a leading '^' on a list in the
key-absent branch as well.

Fixes saltstack#50755
salt.utils.network.is_reachable_host only caught socket.gaierror, but
socket.getaddrinfo raises UnicodeError (an idna "label too long" error,
which is not a subclass of gaierror) when a name contains a DNS label
longer than 63 characters. A long salt-ssh -E/--pcre target triggers
this, so _expand_target crashed instead of treating the target as not a
reachable host. Also catch UnicodeError and return False.

Fixes saltstack#57207
Minion.pillar_refresh compiles a fresh pillar and rebinds
self.opts["pillar"] to a new dict. A proxy minion packs the pillar into
its proxy module loader once, at init, by reference
(self.proxy.pack["__pillar__"] = self.opts["pillar"]). The rebind orphans
the dict that pack still aliases, so already-loaded proxy modules keep
serving the pillar they were first packed with until the proxy restarts.

Re-pack the loader with the freshly compiled pillar after the rebind,
guarded so a regular (non-proxy) minion is unaffected. This mirrors the
existing deltaproxy __grains__ re-pack and avoids reload_modules(), so a
module's connection state and __context__ are preserved. Deltaproxy is
covered by the same change: handle_event dispatches each sub-proxy's
pillar_refresh to that sub-proxy instance, so the re-pack runs per
sub-proxy against its own loader and opts.

Out of scope: __opts__, __opts__["pillar"] and values a module copied out
of pillar during init() are load-time snapshots and stay stale under any
non-reload fix.

Fixes saltstack#58197
Minion.pillar_refresh calls module_refresh() at the top, rebuilding the
execution-module loaders (functions/returners/executors/utils) from
self.opts, and only afterwards compiles the new pillar and rebinds
self.opts["pillar"]. Each exec loader snapshots opts["pillar"] by value
when it is built, so those freshly rebuilt loaders capture the OLD pillar.

A regular minion masks this: every job rebuilds the loaders through
gen_modules() before running. A proxy minion's metaproxy job path never
does, so an exec module keeps serving the previous refresh's __pillar__
until the next refresh_pillar -- the "run the same command twice, get two
answers" symptom.

Re-run module_refresh() after the rebind so the exec loaders are rebuilt
against the freshly compiled pillar. It sits next to the saltstack#58197 proxy
re-pack in the success branch and is gated on opts["proxy"], so a regular
minion pays no extra loader rebuild and a failed compile triggers none.

Fixes saltstack#59393
pillar_refresh is a coroutine method, so io_loop.run_sync can call it
directly; the no-arg wrapper lambda tripped pylint's unnecessary-lambda
(W0108) in the lint-tests job.
Pillar compilation loads execution modules (for salt[...] calls in pillar
templates) with the incoming opts when file_client is "local". On the
master that branch is always taken, and the incoming opts carry the
master's id, so salt["match.compound"] and friends matched against the
master rather than the minion the pillar was being compiled for (saltstack#58407).
Load them with self.opts, which __gen_opts stamps with the target
minion_id, matching the non-local branch. Masterless is unaffected
(opts["id"] already equals the minion_id there).

Fixes saltstack#58407
…ted example

The saltstack#58407 fix makes execution modules called during pillar rendering
resolve against the target minion, so the documented match.filter_by
example (which omits the minion_id argument) now correctly matches the
minion and gets the db role. The parametrized expectation still asserted
the pre-fix web* roles, which contradicted the fix and failed in CI.
Adds functional coverage for salt.sdb.env (set/get via environment
variables, including the setdefault no-overwrite behavior) and
salt.sdb.yaml (get with nested/colon traversal, multi-file merge, missing
keys, and the read-only set raising NotImplemented). These are the two sdb
backends that require no external service.

Refs saltstack#61260
salt.utils.data.filter_by ran every lookup_dict key through
fnmatch.fnmatchcase, so a literal key containing glob metacharacters (the
"[" and "]" in GPU/PCI model strings such as "GP104GL [Quadro P4000]")
was parsed as a character class and never matched its own value, falling
through to the default (saltstack#60976). Try an exact string comparison before the
fnmatch fallback. The change is additive: literal keys now match and
existing glob patterns are unaffected. This also covers grains.filter_by,
pillar.filter_by, and match.filter_by, which all delegate here.

Fixes saltstack#60976
wheel.key.gen passed keysize straight to salt.crypt.gen_keys, which feeds
it to rsa.generate_private_key (key_size must be an int). The salt-api
rest_cherrypy POST /keys endpoint passes keysize as a string (cherrypy
form values are always strings), so a request specifying a keysize raised
TypeError and returned a 500 (saltstack#56425). Coerce keysize to an int and
enforce the 2048-bit minimum that gen's docstring already documents.

Fixes saltstack#56425
The file.comment, file.append and file.prepend states read the target
file as bytes and decode it with the system encoding purely to build a
diff. A strict decode raised UnicodeDecodeError and aborted the state
when the file contained bytes invalid in that encoding. Decode with
errors="replace" since the result is only used for the diff.

Fixes saltstack#50903
dwoz and others added 12 commits August 27, 2026 11:54
The `SaltVersionsInfo` table in `salt/version.py` had CHLORINE (3007)
marked `released=True` but ARGON (3008) not, even though 3008.0,
3008.1, and 3008.2 have all been tagged and shipped.

`SaltVersionsInfo.current_release()` iterates the version list and
returns the last codename with `released=True`. With ARGON missing
the flag it returns CHLORINE, which then gets used as the baseline
in `__discover_version()`. On the 3008.x branch that produces version
strings like `3007.14+2643.gca4940b8f0` for nightly builds -- packages
end up named `salt-3007.14+N-0.x86_64.rpm` instead of the expected
`salt-3008.2+N-0.x86_64.rpm`.

Regression window on 3008.x: introduced by the 2026-08-25 merge from
3007.x (commit 17c1f44 "Merge remote-tracking branch
'origin/3007.x' into merge/3007.x/3008.x-08-25-26"). Prior nightlies
were correctly labeled -- e.g. build 210 from 2026-08-19 shipped as
`salt-3008.2+210.g9a41326f33`. Master has the same bug and was
similarly affected (though master nightlies have been failing for
other reasons and nobody noticed).

Fix is a one-line flip:

    ARGON = SaltVersion("Argon", info=3008, released=True)

Needs to be backported to 3008.x. Any active supported release
branch whose `current_release()` should return ARGON needs the same.

Discovered while verifying nightly RPM signing on a live 3008.x
build (salt-nightlies run 33103981309): "Prepare Release" step
printed "3007.14+2643.gca4940b8f0" from a 3008.x checkout.
…008.x

[3008.x] version: mark ARGON (3008) as released
The mirror workflow on saltstack/salt-nightlies force-pushes branches
from saltstack/salt daily. Those push events trigger ci.yml here to
re-run the exact same tests that just ran on saltstack/salt before the
mirror — pure duplicate, no additional signal, real runner-minute cost.

Gate the prepare-workflow job with
  if: github.repository != 'saltstack/salt-nightlies'
so CI cascade-skips on the fork (all subsequent jobs `needs:
prepare-workflow`). Set via prepare_workflow_if_check in ci.yml.jinja
so the change survives future template regeneration; the generated
ci.yml is updated in the same commit for immediate effect (would
otherwise wait for the next `tools ci` regen).

Follow-ups if warranted (not in this PR): similar treatment for
scheduled.yml, staging.yml, doc-linkcheck.yml, backport.yml,
dependabot-sync.yml, release.yml.
saltstack#70141 added RPM signing for nightly builds via the reusable
build-packages.yml workflow. DEBs were left unsigned -- build-packages.yml
had sign-rpm-packages / sign-macos-packages / sign-windows-packages
inputs but no sign-deb-packages, and the build-deb-packages job had
no environment: declaration (so it couldn't read secrets), no gnupg
setup, no debsigs invocation. Verified today: run 33107756715 on
saltstack/salt-nightlies produced 16 signed RPMs (RSA/SHA256, Key ID
52f404520060a19b) but all 16 DEBs came out unsigned (no _gpgorigin
in the ar archive).

Closes that gap by mirroring the RPM signing plumbing for DEBs:

tools/pkg/build.py:
  Add `key_id` argument to the `deb` command. When set, run
  `debsigs --sign=origin --default-key $KEY_ID <pkg>.deb` on each
  .deb debuild produced in the parent directory. Same style of
  post-build signing loop the `rpm` command uses with `rpmsign`.

.github/workflows/build-packages.yml:
  - Add `sign-deb-packages` boolean input (default false).
  - Add `environment: ${{ inputs.environment }}` to the
    build-deb-packages job so it can read caller-inherited secrets
    (previously only build-rpm-packages had this).
  - Install debsigs in the deb build container's dependency step.
  - Add a Setup GnuPG step gated on `inputs.sign-deb-packages`,
    identical shape to the RPM job's version: imports SIGNING_GPG_KEY
    with SIGNING_PASSPHRASE, then discovers the imported key's
    fingerprint via `gpg --list-secret-keys` and exports SIGN_KEY_ID
    to $GITHUB_ENV.
  - Thread `--key-id=${env.SIGN_KEY_ID}` through to
    `tools pkg build deb` when signing is enabled.

templates/build-packages.yml.jinja:
  Add `sign-deb-packages: true` for nightly gh_environment, following
  the sign-rpm-packages pattern. Regenerates nightly.yml with the
  flag set on both build-pkgs-onedir and build-pkgs-src callers.

templates/staging.yml.jinja:
  Add matching `sign-deb-packages` input so stable releases can opt
  in explicitly. Default false to preserve current behavior.

Local verification: pre-commit run passes on all modified files
(Generate GitHub Workflow Templates, Lint GitHub Actions Workflows,
isort, black, mypy). End-to-end debsigs signing was independently
tested earlier in a debian:13 container against the same nightly
signing key material -- signature verifies cleanly with
`gpg --verify _gpgorigin <payload>` after extracting the ar archive.

DEB consumer caveat: `apt install` does not check debsigs-style
file signatures by default (it verifies Release-file signatures
from apt repos). Since salt-nightlies live in an Artifactory
generic repo without apt metadata by design, the signed DEBs are
verifiable via `debsigs --verify <file.deb>` for anyone who wants
to before installing. Same posture as the signed nightly RPMs
verified via `rpm --checksig`.

Needs backport to 3008.x (and any other supported release branch
where nightly RPM signing is enabled).
…008.x

[3008.x] nightly: sign DEB packages with debsigs, mirroring RPM signing path
…name

saltstack#70162 marked ARGON released=True on 3008.x so
SaltVersionsInfo.current_release() now returns ARGON. The regression
test test_current_release_matches_maintenance_branch_67061 was
asserting the pre-flip codename by name (CHLORINE), which was
correct for 3007.x but wrong once 3008.x updated the released
flag. Every 3008.x PR started failing here (e.g. saltstack#70157 CI).

Rather than bump the hardcoded codename with every release cycle
(and reintroduce this same failure mode next time POTASSIUM or its
successors flip to released=True), assert the *contract* that
current_release() is documented to satisfy: return the last codename
in SaltVersionsInfo.versions() with released=True.

    released = [v for v in SaltVersionsInfo.versions() if v.released]
    assert released, "SaltVersionsInfo table has no released codenames"
    expected = released[-1]
    current = SaltVersionsInfo.current_release()
    assert current == expected, "..."

Same regression-protection value against the original saltstack#67061 bug
(current_release() walking to the first *un*-released codename would
still fail this assertion), zero per-release-cycle churn. Failure
message names both the expected and actual codenames.

Verified locally: contract-based assertion passes with the current
SaltVersionsInfo table state on this branch (expected=Argon/3008,
current=Argon/3008).
…008.x

[3008.x] ci: skip CI on saltstack/salt-nightlies (duplicate of upstream)
…lease-argon-3008.x

[3008.x] test_version: assert current_release() contract, not a hardcoded codename
Commit f3ffc8f (originally landed on 3007.x) added
`--match v3007.*` to salt.version's git-describe call to keep 3008.x
tags from hijacking version detection on 3007.x when a reverted
mis-merge left 3008.x commits reachable.

That commit got merged forward into 3008.x on 2026-08-25 (via
17c1f44 "Merge remote-tracking branch 'origin/3007.x' into
merge/3007.x/3008.x-08-25-26") without updating the constraint to
this branch's major. Result on 3008.x: git describe skips every
v3008.* tag and reports "v3007.14-N-gSHA". Downstream:

- __discover_version() sees parsed.major=3007 < saltstack_version.major=3008
  and takes the "lift baseline" branch, replacing the parsed base
  with SaltVersionsInfo.current_release() (=ARGON, info=3008 only,
  no minor).
- Minor defaults to 0.
- Every nightly RPM/DEB on 3008.x since the merge has been labeled
  salt-3008.0+N-... instead of salt-3008.2+N-...

Fix is one-line: change v3007.* -> v3008.*. Same intent as the
original 3007.x commit (constrain to the branch's own major),
adapted to 3008.x's calver line. git describe now returns
v3008.2-<ahead>-gSHA, __discover_version() parses it directly
(same major, no lift-baseline needed), and RPM/DEB packages get
the correct salt-3008.2+N-... version prefix.

Verified in the salt-nightlies 3008.x run 33122447777 (post-saltstack#70162
merged): produces salt-3008.0+2651.gfd8241adb4 instead of the
expected salt-3008.2+2651.gfd8241adb4. This one-line fix restores
the correct minor.

Master is unaffected -- it uses `--match v[0-9]*` (no branch-major
constraint).
…tch-3008-on-3008.x

[3008.x] version: constrain git-describe --match to v3008.* on 3008.x
Shell.send() built the scp command as an unquoted f-string, so any
space in the local or remote path (e.g. a master cachedir/root_dir
containing a space) caused shlex.split() to break it into bogus
extra arguments and fail the transfer. This affected every send()
caller: thin/relenv tarball deploy, ext_mods tarball, pre-flight
script, shim transfer, and the relenv minion-config transfer.

Quote local and host:remote with shlex.quote() in Shell.send(), and
apply the same treatment to the related unquoted "test -d"/"test -f"
remote checks in Single.check_thin_dir() and Single.cmd_block().

Add a unit test covering the command string built by Shell.send(),
and an integration test that overrides a roster's thin_dir with a
space-containing path and drives a real salt-ssh deploy against it.

Fixes saltstack#61338
@twangboy

Copy link
Copy Markdown
Owner Author

Closing — rerouting this debug investigation through a real upstream draft PR instead, since basing a scratch PR against a fork feature-branch broke the release-prep step before it could reach the relevant Test Salt jobs.

@twangboy twangboy closed this Aug 28, 2026
@twangboy
twangboy deleted the debug/61338-argmax-trace branch August 29, 2026 02:58

This branch had an error being deployed

1 failed deployment
ci — e99900f1 Deployed Aug 28, 2026 by twangboy via Build Onedir Packages / RPM (arm64) #985
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

10 participants