Syncing from upstream saltstack/salt (feature/nightly-stress-3008x-dispatch) - #143
Open
bt-admin wants to merge 535 commits into
Open
Conversation
The pkg-test downgrade matrix installs current salt, then rolls back to the previous release before running pytest. That leaves the pre-#69950 onedir on disk, which correctly lacks libyaml-linked PyYAML — asserting its presence there produces a false positive. Gate both libyaml tests on install_salt.downgrade so the downgrade matrix skips them; install/upgrade flavors continue to exercise the fix.
…owngrade Post-downgrade pytest re-enters the pkg/integration suite with the previous salt onedir on disk and no --downgrade flag, so the earlier install_salt.downgrade guard didn't trigger and the test still failed. Rewritten to key on install_salt.version: - >= 3006.28 -> assert libyaml present (guards the fix) - < 3006.28 -> assert libyaml absent (documents the pre-fix baseline so a silent regression on the old branch is also caught) Works uniformly across the three pkg-test flavors without skips.
The prior design compared install_salt.version >= Version("3006.28") to
predict whether libyaml should be present, but dev builds report
'3006.27+NNN.gSHA' which packaging.version orders BEFORE '3006.27' let
alone '3006.28' (PEP 440 local-version segment). That flipped the
expected-libyaml boolean to False on install jobs and turned every
Linux install matrix row red.
install_salt.use_prev_version is True iff the pytest run is executing
against the downgraded-to previous release (set by --use-prev-version in
the post-downgrade validation stage). That's the only flavor where the
onedir predates PR #69950 and libyaml is legitimately absent. Key the
expectation off that flag and drop the version-comparison plumbing.
…69920) Give each daemon AsyncReqMessageClient a per-instance uuid.uuid4().hex as its ZMQ IDENTITY, replacing the earlier process-wide _REQ_IDENTITY_SLOT counter and SALT_REQ_IDENTITY_SLOT_MAX cap. Each RequestClient is opened and closed by Salt itself, so a per-instance UUID matches the object's lifetime and gives the master ROUTER's routing-id table a 1:1 mapping to a client we control. Fork inheritance of the earlier counter -- root cause of #69753 -- is impossible by construction, since each child draws a fresh UUID.
path= stripped PEM headers while key= hashed the raw string, so master_finger from a key string did not match salt-key -F. Fixes #69970
`salt-pip` shells out to `python -m pip` against a packager-pinned onedir pip; pip's periodic "A new release of pip is available" HTTPS check is pure noise (the user can't do anything about it) and a proxy-config gotcha (cf. #69910). `salt/modules/pip.py` already suppresses it on 6 install/list/upgrade paths; salt-pip should too. Set `PIP_DISABLE_PIP_VERSION_CHECK=1` in `_pip_environment` via `setdefault` so operators can opt back in by exporting `PIP_DISABLE_PIP_VERSION_CHECK=0` before invoking salt-pip. Fixes #70024
The unit tests on `_pip_environment` prove the helper injects `PIP_DISABLE_PIP_VERSION_CHECK=1` (and respects an operator override) but would still pass if a future refactor stopped routing `salt_pip` through `_pip_environment`. Add two end-to-end tests that drive `salt.scripts.salt_pip` with a stubbed `subprocess.run` and assert on the `env` dict actually handed to the child `python -m pip` process. Refs #70024
Adds opt-in minion_memory_headroom (accepts "5%" / "5G" / int bytes) and minion_memory_max (bytes / size string) config options with silent cgroup v1 / v2 detection. When either opt is set, the reference "total memory available" is resolved from config > cgroup-v2 > cgroup-v1 > psutil.virtual_memory().total, and used bytes come from the matching tier. When neither opt is set the check preserves the legacy psutil.virtual_memory().percent > 95 behavior byte-for-byte, so no minion changes behavior on upgrade. Refs #69884
resource_modules() built its LazyLoader over _module_dirs(), which returns
the full stock module set (~100+ salt/modules/*) with per-type overlay
directories merely prepended. That left every stock function reachable
in a resource-scoped loader. Targeting a resource with a name like
cmd.run / grains.setval / file.remove / state.apply resolved through the
resource loader, ran the stock function in the managing minion process,
and returned the result attributed to the resource id — contradicting
the documented Resources safety contract (unsupported functions must
fail loudly; managing-minion access is explicit via __minion__).
Add a new private helper _resource_type_module_dirs() that walks the
same layer stack _module_dirs() walks (cli module_dirs, extension_modules,
entry-point packages, SALT_BASE_PATH) but only accepts each layer's
resources/<rtype>/<ext_type>/ overlay subdir — never the layer's plain
<ext_type>/ dir. resource_modules() now uses this helper.
Result:
* salt <resource-id> <stock-fun> — "Function 'X' is not supported for
resource type 'Y'." (the _thread_return guard fires because the name
is absent from the loader).
* salt <resource-id> <per-type-fun> — reachable as before via the
resources/<rtype>/modules/ overlay.
* Resource-context modules that intentionally want managing-minion
behavior still call __minion__["x.y"] explicitly.
Tests:
* tests/pytests/unit/loader/test_per_resource_overrides.py
- Flip test_no_override_falls_through_to_standard_state_module (which
asserted stock state.sls was present) → test_no_override_hides_stock_modules
asserting no stock cmd/state/grains/file/… leak through. The prior
assertion documented the buggy contract.
- Add test_per_type_override_present_and_callable for the positive path.
* tests/pytests/unit/cli/test_caller_resources.py
- salt-call -r grains.items / state.apply against a dummy resource
now returns "not supported for resource type 'dummy'" rejections
instead of stock function results.
* tests/pytests/integration/resources/test_resource_loader_strict.py (new)
- End-to-end master+minion coverage: salt dummy-01 cmd.run /
grains.setval / file.remove / sys.list_functions all return the
per-type rejection; test.ping (a real per-type override) still runs;
grains.setval leaves the managing minion's grains untouched.
Fixes #69881
Complements the unit-level matrix in tests/pytests/unit/test_minion_memory_headroom.py by booting a real minion daemon with each opt combination and driving the runtime check via a subprocess that loads the minion's on-disk config through salt.config.minion_config and calls Minion._has_memory_headroom. Four scenarios: * config round-trip via salt-call --local config.get * default preserved (no opts -> legacy True on any healthy host) * config override -> deterministic True (huge max, tiny reserve) * config override -> deterministic False (tiny max, full reserve) All scenarios use deterministic config values so no system-memory pressure is required. Refs #69884
napalm.junos_cli defaults dev_timeout to None and forwards it, so the Junos _timeout_decorator / _timeout_decorator_cleankwargs wrappers hit max(None, 0) -- which raises TypeError -- and would otherwise try to set the junos-eznc connection timeout to None, which it rejects. Coalesce None to 0 when computing the effective timeout, and only override the connection timeout when a real (>0) dev_timeout/timeout is given; otherwise run the command with the connection's default timeout. This makes junos_cli and the other timeout-decorated calls work when no timeout is passed, while still honouring an explicit dev_timeout/timeout.
…65867) junos.rpc (used by napalm.junos_rpc) builds the RPC ``op`` dict from __pub_arg, which carries the reserved ``__kwarg__`` marker that the Salt CLI appends to keyword arguments. On a ``get-config`` call with a ``filter`` the marker leaked into the RPC options, and junos-eznc's ElementMaker raised ``KeyError: <class 'bool'>`` while rendering the ``True`` value as an XML attribute, so the filter option stopped working after upgrading from 3004. Strip dunder keys from ``op`` with salt.utils.args.clean_kwargs (already used by junos.diff) right after it is assembled, so reserved markers are dropped on every RPC path. Adds regression tests for the get-config and non get-config paths.
napalm_ntp (set_peers/set_servers/delete_peers/delete_servers), napalm_snmp (update_config/remove_config) and napalm_probes (set_probes/delete_probes/ schedule_probes) all call net.load_template with a bare template name (e.g. "set_ntp_peers"). net.load_template used to route bare names into NAPALM's own renderer, but that path was removed in the Sodium release (#57370) -- whose own deprecation warning explicitly told netntp/netsnmp/netusers users to ignore it. The bare name now falls through to the fileserver as "Local file source set_ntp_peers does not exist", so every one of these functions fails. Lift the resolver introduced for users in #62170 into a shared salt.utils.napalm.template_path (walks the driver class MRO + inspect.getfile, catching TypeError/OSError, returns None when the driver ships no such template) plus salt.utils.napalm.template_not_available (standard failure payload). The three modules resolve the driver's NAPALM-shipped template to an absolute path and render it through the Salt pipeline, or return the clear failure message. template_not_available also closes the per-call connection a non-always-alive proxy/minion opened, since it short-circuits net.load_template (which would otherwise close it). napalm_users is intentionally left to #62170; a follow-up dedups its local resolver onto the shared one. Validated on a live Juniper EX3400 (Junos 23.4R2): ntp.set_servers renders, commits, is confirmed in the device config, and is removed again. (NAPALM's junos snmp/probes templates still use py2 dict.iteritems() and need modernizing upstream in NAPALM; the NTP templates are py3-clean.) New tests/pytests/unit/utils/test_napalm.py covers the resolver (MRO concrete-over-base precedence, base fall-through, OSError/TypeError skip, missing -> None) and the close behaviour; test_ntp/test_snmp are rewritten and test_probes added with routing tests asserting the correct template name, the resolved path, forwarded flags, and inherit_napalm_device identity.
napalm_probes.set_probes/delete_probes/schedule_probes now resolve the driver's template and no longer return result=True for the bare-name mock, so the legacy unittest-style tests (which asserted the pre-fix behaviour) fail. They are superseded by tests/pytests/unit/modules/napalm/test_probes.py added here, which covers config/results plus the routing and no-template paths.
An always-alive NAPALM proxy runs with multiprocessing disabled, so jobs executing at the same time are threads that share one cached device object and its single command channel (get_device returns the same device by reference). Two concurrent calls -- e.g. net.cli, or a grains refresh landing during a state run -- can then interleave on that channel, mixing each other's output and, on drivers that share a raw CLI session without their own locking, corrupting the connection. Give each device a reentrant lock, created in get_device(), and hold it in salt.utils.napalm.call() for the duration of the call. A reentrant lock is required because call() re-enters itself (close/open/re-exec) on a reconnect; a plain Lock would deadlock. Devices built without a LOCK (hand-constructed in tests, or inherited via inherit_napalm_device) run unserialized, unchanged. The lock is per-device, not global, so a deltaproxy hosting many sub-proxies in one process does not needlessly serialize calls across unrelated devices.
The EP fan-out hot path did one msgpack.dumps per event (in frame_msg(package)) followed by a full msgpack.loads on the outer IPC frame (in TCPPuller) and again on the inner event body (in SaltEvent.unpack), even though the routing logic only needed to inspect the event tag. Under a 50-minion highstate return burst this produced ~120 MB of transient Python dict/list objects per burst that stressed the glibc arena, driving EP RSS growth. Two related changes: 1. raw_payload passthrough (salt/transport/tcp.py): TCPPuller.handle_stream reads the length-prefixed msgpack frame with raw=True (skips per-key str allocation) and passes the original wire bytes as raw_payload=payload to the handler. PublishServer.publish_payload and PubServer.publish_payload accept and forward raw_payload; when set, PubServer writes raw_payload to subscribers instead of re-packing via frame_msg. This removes one msgpack.dumps per event on the EP hot path. 2. Tag peek (salt/channel/server.py MasterPubServerChannel.publish_payload): Bytes-level load.partition(TAGEND) grabs the tag without deserializing the body. Full salt.payload.loads is called lazily via a _decode_data() closure, only in the five cluster/runner/* branches that actually need the decoded dict. For non-cluster masters (the >99% case), the full unpack is never performed. The cluster-peer fanout path (self.pushers non-empty) still decodes data via _decode_data() where it needs to wrap the event in a cluster/event envelope, so cluster deployments retain the same behavior. Local fanout forwards raw_payload to transport.publish_payload; cluster branches do not, because they mutate load before publishing. Measured under a 50-minion state.apply/highstate stress rig: - Peak Python allocation (memray): -28% (124 -> 89 MB) - Leaked Python bytes (memray): -34% (122 -> 81 MB) - msgpack.unpackb calls: 6084 -> 66 - Return throughput ceiling: +55% with passthrough alone, +111% with tag peek layered on top Non-cluster deployments see the full benefit. Cluster deployments retain identical semantics; the cluster branches still call _decode_data() before publishing.
…ek (#70052) Unit + functional coverage for PR #70052: - salt/transport/tcp.py * PubServer.publish_payload writes raw_payload bytes verbatim when supplied and skips frame_msg; falls back to frame_msg otherwise. * PubServer.publish_payload raw bypass applies with topic_list too. * PublishServer.publish_payload forwards raw_payload= kwarg down to self.pub_server.publish_payload (default None preserves framing). * TCPPuller.handle_stream passes raw_payload=<wire bytes> to the handler, falls back to positional-only handler on TypeError, and unpacks the outer frame with raw=True (dict keys are bytes). - salt/channel/server.py MasterPubServerChannel.publish_payload * Non-cluster tags (salt/job/..., salt/auth) never invoke salt.payload.loads -- verified by patching and asserting .called. * raw_payload is forwarded to self.transport.publish_payload for the non-cluster local-fanout branch (and defaults to None). * All five cluster/runner/* branches (sync_roots, collect_from_peers, shed_unowned_all, delegate_write, ring_create/destroy/route_set/ route_clear/ring_set) invoke _decode_data() and dispatch the decoded body into the appropriate _run_* / _handle_multi_ring_* method. * cluster/peer* tags do not re-broadcast locally. * cluster-peer fanout branch (self.pushers non-empty, tag NOT cluster/peer*) decodes the body to build the cluster/event envelope AND still forwards raw_payload to the local transport. - tests/pytests/functional/transport/tcp/test_pub_server.py * End-to-end regression: real PublishServer + PublishClient + TCPPuller path, verifying raw_payload flows from the pull socket to the subscriber and the received payload still decodes back to the original dict. Fixes two pre-existing tests that broke on the raw=True unpack switch (test_tcp_puller_handle_stream_awaits_payload_handler and test_tcp_puller_handle_stream_survives_handler_exception): body values now arrive as bytes.
- get_device_opts: optional_args explicitly set to null yielded None (the get
default only applies to a missing key), crashing the "config_lock" membership
test; and a present optional_args dict was mutated in place, leaking the
config_lock / keepalive defaults into the caller's opts/pillar. Use
copy.deepcopy(device_dict.get("optional_args") or {}).
- proxy_napalm_wrap: force_reconnect did opts["proxy"].update(**kwargs)
unconditionally, raising KeyError on a straight (non-proxy) minion which has
no 'proxy' key. That merge is only for the always-alive proxy path; a straight
minion picks the override up from clean_kwargs, so guard it with is_proxy().
- proxy.shutdown: a trailing comma made 'port' a 1-tuple, so a failed close()
logged ':(830,)' / ':(None,)'. Remove it.
The NSIS stress tests still hit intermittent installer Abort (exit code 2) after the previous retry fix, on both 3006.x and 3007.x. Timing recovered from the CI logs shows the SCM held the salt-minion service key for 25s+ before the uninstall side's own wait_svc_deleted loop and the test harness's post-uninstall wait both gave up, leaving only ~10s of retry budget on the install side before it aborted -- not enough headroom for the observed delay. Wait for the salt-minion service registry key to disappear before even attempting "ssm install" (CreateService), instead of only reacting after CreateService fails. This is a no-op on a normal install, since the key was never present. Also widen the existing retry budgets: the install-side CreateService retry from 5x2s to 10x2s, and the uninstall-side wait_svc_deleted from 10s to 15s. Add diagnostics so future occurrences don't require a fresh repro: print the tail of the relevant %TEMP%\SaltInstaller\*.log directly into the pytest failure output on any non-zero exit or timeout, and upload the full log directory as a CI artifact from both the Logic Tests and Stress Tests jobs.
[3008.x] nightly: sign DEB packages with debsigs, mirroring RPM signing path
…name #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. #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 #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).
[3008.x] ci: skip CI on saltstack/salt-nightlies (duplicate of upstream)
…name #70161 marked ARGON released=True on master 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 needed bumping to ARGON. Rather than bump the hardcoded value 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 #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. Sibling PR: 3008.x -- SaltProject/salt#70167.
…on-3008.x [3008.x] test_version: assert current_release() contract, not a hardcoded codename
…on-master 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-#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).
…on-3008.x [3008.x] version: constrain git-describe --match to v3008.* on 3008.x
Aligns the salt-nightlies visibility page (https://saltstack.github.io/salt-nightlies) with the visual language used across saltproject.io, docs.saltproject.io, and the install guide (all pydata-sphinx-theme or its Hugo port). Changes to .github/scripts/generate_nightly_dashboard.py: - Embed SaltProject_altlogo_teal.png as a base64 data URI, sourced from the salt-install-guide _static/img/ set (identical to docs sites). - Add a top navbar with logo + links to saltproject.io / install-guide / docs / GitHub, with the current page marked active. - Swap the CSS palette to PST tokens: primary teal #0a7d91, link-hover purple #8045e5, success #28a745, danger #d72d47, warning #9a6700, background #fff, borders #d1d5da. - Switch fonts to the PST system-UI + ui-monospace stacks. - Add a footer with the "Updated ..." timestamp and repo link. Zero external dependencies: no CDN CSS/JS, everything (logo + styles) is self-contained in the generated index.html so the page renders on enterprise GHE / air-gapped mirrors identically. Rendered locally against the live gh-pages history.json — no regression in table structure and the expand-on-click detail rows still work.
dashboard: adopt PyData Sphinx Theme palette + logo, matching docs
…-08-28-26 # Conflicts: # .github/workflows/ci.yml # .github/workflows/dependabot-sync.yml # .github/workflows/nightly-stress-test.yml # .github/workflows/nightly.yml # .github/workflows/scheduled.yml # .github/workflows/staging.yml # .github/workflows/templates/layout.yml.jinja # .pre-commit-config.yaml # pkg/macos/install_salt.sh # requirements/base.txt # tests/pytests/pkg/integration/test_version.py # tests/pytests/unit/cli/test_batch.py # tests/pytests/unit/grains/test_core.py
end-of-file-fixer flagged two of these scratch files for missing final newlines. They are transient outputs from the silent-drop audit sub-agent and shouldn't be tracked in the merge PR. The audit report itself (agents/reports/silent_drop_audit_3008_to_master.md) stays.
[master] Merge forward from 3008.x
schedule/workflow_dispatch triggers only ever run a workflow as it exists on the default branch. 3008.x and 3006.x each already carry their own copy of this file with the same schedule block, but it has never fired for either (confirmed via the Actions run history: every event:schedule run is on master). Matrix the existing job over branch: [master, 3008.x, 3006.x] with an explicit `ref: matrix.branch` checkout, so master's live schedule drives all three -- each running its own checked-out tests/monitoring/ code. Branch-qualify the Docker layer cache key, the uploaded artifact name, and the stress-snapshots publish directory, since matrixed jobs share one github.run_id/github.sha and would otherwise collide. 3007.x is left out: its tests/monitoring/ predates render_panels.py, so the Render Dashboard Panels / Publish panels / Panel Summary steps would fail against it as-is.
Scoped to this branch only -- lets a manual workflow_dispatch actually execute (to verify the checkout-against-fork failure from earlier is fixed by testing directly against this repo's own 3006.x/3008.x) without touching the repo-wide variable, which would also affect master's real schedule and anyone else relying on it. Must be reverted before merge.
VCOPS-100029: today there's no way to run this stress test against a specific PR/branch's diff -- workflow_dispatch only exposes `duration`, and the matrix's checkout ref was hardcoded to [master, 3008.x, 3006.x] regardless of which ref you dispatched against. Compute the matrix branch list from the trigger type instead of a fixed array: schedule (nightly cron) still expands to the same three-branch set unconditionally; workflow_dispatch expands to a single-element array built from the new `branch` input (default "master", but any branch/PR ref the caller types in). event_name=='schedule' short- circuits before `inputs.branch` is evaluated, since schedule events have no `inputs` context.
Drops the matrix/fromJSON trick entirely. nightly-stress-test.yml goes back to a single job with one branch input (default master) -- whoever dispatches it picks exactly one branch, full stop, no schedule trigger of its own anymore. Nightly coverage of master/3008.x/3006.x now comes from a new sibling workflow, nightly-stress-test-dispatch.yml, which holds the only live schedule (registered on master, per GitHub's default-branch-only rule) and fires nightly-stress-test.yml three separate times via workflow_dispatch -- once per branch, each its own independent run rather than a matrix leg sharing one. Always dispatches against --ref master so every run loads the same simple job definition, threading the actual branch through as an input instead. Since each dispatch is now its own run, matrix-driven collisions (shared github.run_id/github.sha across legs) no longer apply -- dropped the RUN_DIR/artifact-name branch-qualification comments that were about avoiding those, kept the qualification itself for readability. Still need inputs.branch (not github.sha alone) in the Docker cache key: every dispatch shares master's github.sha (since dispatch ref is always master), but checks out different code per inputs.branch, so a sha-only key would serve one branch's cached layers to another. 3007.x still deliberately excluded from the nightly set (predates render_panels.py).
The dispatcher (schedule-triggered, fires the nightly set) now owns the nightly-stress-test.yml name -- that's the one people actually mean by "the nightly stress test". The single-run building block (branch + duration inputs, one job) moves to run-stress-test.yml, invoked both by the dispatcher and by anyone manually testing a specific branch/PR. Also trimmed the `branch` input description down to just "Branch", and dropped a comment on the stress-snapshots RUN_DIR that was explaining something not worth explaining.
It gates whether the *nightly* firing happens, not whether a stress test can run at all -- it belongs on nightly-stress-test.yml's dispatch job, not on run-stress-test.yml's, so that manually running run-stress-test.yml against a PR branch (VCOPS-100029) still works regardless of this repo-level variable. Also drops the TEMPORARY if:true bypass from run-stress-test.yml now that the real check has moved off of it entirely.
Mirrors run-nightly.yml's trigger-branch-nightly-builds job: use --ref matrix.branch (via strategy.matrix) instead of always --ref master, so each branch's own copy of run-stress-test.yml governs its run instead of borrowing master's. This means 3008.x and 3006.x each need their own run-stress-test.yml landed (separate PRs against those base branches, coming next) -- master alone isn't enough anymore.
…master" This reverts commit 663eda2.
Per Option A (single source of truth on master): rather than 3008.x keeping its own divergent copy of this workflow just to expose these two inputs, fold them into master's copy as universal, opt-in inputs available to every branch dispatched against. worker_threads defaults to empty (not 3008.x's original '5') -- master's and 3006.x's tests/monitoring/master.conf both currently ship worker_threads: 10, and a hardcoded '5' default would silently nudge their runs into an unrequested config change + restart the first time someone runs this without setting the input. Only a non-empty explicit value touches master.conf now.
Daniel Wozniak (core maintainer) confirmed: workflow_dispatch --ref X runs the copy of the target workflow file that exists ON BRANCH X, not master's copy checked out against X. The previous design (--ref master for every dispatch, reusing master's single-branch-input copy of run-stress-test.yml, with inputs.branch only steering the actions/checkout step inside it) therefore never actually picked up a change made to run-stress-test.yml on a non-master branch -- editing 3006.x's copy of that workflow would silently have zero effect on 3006.x's nightly run. Now each branch in the loop dispatches with --ref matching itself, so that branch's own copy of run-stress-test.yml governs its own nightly run. This file is meant to be byte-identical across master/3006.x/3007.x/ 3008.x once each has it -- porting to a not-yet-covered branch is a straight copy of the whole file, not a re-edit of the branch list. Only master's copy is ever actually fired by the `schedule:` cron (GitHub only honors schedule off the default branch); the branch list still lives here (not only reasoned about on master) so a manual workflow_dispatch of this file on another branch behaves the same as the real nightly run would, and so porting requires no re-derivation. 3008.x is in the branch list already even though it doesn't have its own run-stress-test.yml yet -- that dispatch will fail until it's ported (companion PR), and `|| echo ...` keeps that one failure from blocking master/3006.x in the same run. 3007.x stays out of the list entirely (not commented out) -- its tests/monitoring/ predates render_panels.py, so the Render Dashboard Panels / Publish panels / Panel Summary steps would fail against it even once ported. Also fixes a stale comment on run-stress-test.yml's Docker cache key that described the old always-master dispatch behavior.
The dispatcher passed -f duration=30m, hardcoding a value that duplicates (and could silently drift from) run-stress-test.yml's own default of 0.5h. enable_metrics/worker_threads were never passed here either. Drop the redundant duration override so the dispatcher only sets what it actually needs (branch), leaving every other input to the called workflow's own defaults. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t in run-nightly-stress.yml nightly-stress-test.yml looping over master/3006.x/3008.x internally meant any explicit workflow_dispatch of it (e.g. run-nightly-stress.yml waking up 3006.x's otherwise-inert schedule) re-triggered all three branches again, double-dispatching run-stress-test.yml for branches already covered elsewhere. Match nightly.yml/run-nightly.yml's existing split instead: nightly-stress-test.yml drops its own schedule and only ever dispatches run-stress-test.yml for the branch it's running on (github.ref_name); run-nightly-stress.yml's matrix becomes the sole cron entry point and the sole branch list, now including master. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Match nightly.yml's shape exactly: one file that IS the worker, triggered only by workflow_dispatch, no separate forwarder-plus-worker pair. run-nightly-stress.yml already calls nightly-stress-test.yml directly via --ref with no -f flags (mirroring run-nightly.yml calling nightly.yml the same way), so the extra run-stress-test.yml hop was unnecessary indirection. github.ref_name replaces the removed inputs.branch throughout (cache key, artifact name, snapshot run path). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
run-nightly-stress.yml's trigger job already gates on this var before ever dispatching nightly-stress-test.yml, so checking it again here was redundant for the automated path -- and actively wrong for a manual workflow_dispatch, since the var's whole purpose (per its own comment) is capping automated nightly cost, not blocking a human who deliberately ran this. nightly.yml (the equivalent worker for regular nightly builds) carries no such check either; only its dispatcher (run-nightly.yml) does. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This PR's scope shrinks to just run-nightly-stress.yml's matrix change. nightly-stress-test.yml's rewrite (dispatcher-loop removal, schedule removal, SKIP_NIGHTLY_STRESS_TEST dedup) is being established on 3006.x first (#70197) and will be ported to master and 3008.x as separate, later changes -- not bundled into this PR. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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
9 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.
bt_gitbot