Skip to content

fix(jenkins): bound journal and coredump collection to the current build #15669

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.

Sign up for GitHub

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
fruch wants to merge 1 commit into
base: master
Choose a base branch
from
+43 −9
Open

fix(jenkins): bound journal and coredump collection to the current build #15669

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 16 additions & 5 deletions utils/upload_sct_coredump.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,23 @@ if [[ -n "${RUNNER_IP}" ]] ; then
EXTRA_HYDRA_ARGS="--execute-on-runner ${RUNNER_IP}"
fi

# Check if the coredumps exists in directory
if ./docker/env/hydra.sh $EXTRA_HYDRA_ARGS "bash -c \"[[ -n \\\"\$( ls $COREDUMP_DIR )\\\" ]]\"" ; then
# Only coredumps from this build. On an ephemeral builder or runner the directory is empty at
# start, so mtime filtering changes nothing there - but on a long-lived agent it holds every dump
# the host ever produced (other jobs' included), and unbounded this used to tar and upload all of
# it. collectTestCoredumps passes the build start as SCT_COREDUMPS_SINCE_EPOCH; standalone runs
# fall back to the last 24h.
SINCE_EPOCH="${SCT_COREDUMPS_SINCE_EPOCH:-$(( $(date +%s) - 86400 ))}"

# Compress the coredumps into a tar.gz file
./docker/env/hydra.sh $EXTRA_HYDRA_ARGS "bash -c \"sudo tar --zstd -cf $COREDUMP_TARBALL -C $COREDUMP_DIR .\""
# Collect this build's coredumps, if any (find prints them; empty output means nothing new)
NEW_COREDUMPS=$(./docker/env/hydra.sh $EXTRA_HYDRA_ARGS "bash -c \"find $COREDUMP_DIR -maxdepth 1 -type f -newermt @$SINCE_EPOCH\"" | grep "^$COREDUMP_DIR/" || true)

@dimakr dimakr Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

|| true would drop the exit code of hydra.sh execution. Then on line 36 below the scipts returns/prints no coredumps ... nothing to upload, but the script states this as a fact and doesn't check exactly this is the result of hydra command execution and not some other error.


# Upload the tar.gz file
if [[ -n "${NEW_COREDUMPS}" ]] ; then

# Compress only the new coredumps into a tarball (relative paths, like the old -C invocation)
./docker/env/hydra.sh $EXTRA_HYDRA_ARGS "bash -c \"cd $COREDUMP_DIR && find . -maxdepth 1 -type f -newermt @$SINCE_EPOCH -print0 | sudo tar --zstd -cf $COREDUMP_TARBALL --null -T -\""

# Upload the tarball
./docker/env/hydra.sh $EXTRA_HYDRA_ARGS upload --test-id $SCT_TEST_ID $COREDUMP_TARBALL
else
echo "no coredumps newer than @$SINCE_EPOCH in $COREDUMP_DIR - nothing to upload"
fi
25 changes: 22 additions & 3 deletions vars/collectBuilderLogs.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,35 @@

def call(Map params){
def test_config = groovy.json.JsonOutput.toJson(params.test_config)
// Bound the journal to this build. On an ephemeral builder the journal is minutes old so the
// bound changes nothing, but on a long-lived agent it is weeks of other jobs' history, and
// unbounded this used to tar and upload the entire host journal every build.
def sinceEpoch = (long) (currentBuild.startTimeInMillis / 1000)
sh """#!/bin/bash

set -xe

echo "${params.test_config}"
export SCT_CONFIG_FILES=${test_config}
SHORT_SCT_TEST_ID=\$(echo \$SCT_TEST_ID | cut -c1-8)
sudo journalctl --no-tail --no-pager -o short-precise > builder-\$SHORT_SCT_TEST_ID.log
tar -zcvf builder-\$SHORT_SCT_TEST_ID.log.tar.gz builder-\$SHORT_SCT_TEST_ID.log
# sudo -n so an agent without passwordless sudo fails fast instead of hanging on a password
# prompt. Then fall back to an unprivileged read before giving up: on a static agent that is
# still the journal this build can see (its own units, and everything else when the agent user
# is in systemd-journal), which beats no builder log at all. The if/else keeps a journal we
# cannot read from aborting the stage under set -e before anything got uploaded.
if sudo -n journalctl --since "@${sinceEpoch}" --no-tail --no-pager -o short-precise > builder-\$SHORT_SCT_TEST_ID.log ; then
journal_source="sudo journalctl"
elif journalctl --since "@${sinceEpoch}" --no-tail --no-pager -o short-precise > builder-\$SHORT_SCT_TEST_ID.log ; then
journal_source="unprivileged journalctl"
else
journal_source=""
echo "WARNING: neither sudo -n journalctl nor an unprivileged journalctl could read the journal on \$(hostname) - skipping builder journal upload"
fi

./docker/env/hydra.sh upload --test-id \$SCT_TEST_ID builder-\$SHORT_SCT_TEST_ID.log.tar.gz
if [[ -n "\${journal_source}" ]] ; then
echo "collected builder journal via \${journal_source}"
tar -zcvf builder-\$SHORT_SCT_TEST_ID.log.tar.gz builder-\$SHORT_SCT_TEST_ID.log
./docker/env/hydra.sh upload --test-id \$SCT_TEST_ID builder-\$SHORT_SCT_TEST_ID.log.tar.gz
fi
Comment on lines +21 to +34

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Implement the current-boot fallback.

If sudo -n journalctl fails, this branch only skips collection. It does not collect the readable current boot. Run the non-sudo current-boot fallback before skipping the upload.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vars/collectBuilderLogs.groovy` around lines 19 - 24, Update the journal
collection fallback in collectBuilderLogs so that when the sudo journalctl
command fails, it retries journalctl without sudo while restricting results to
the current boot, and continues with the existing archive and upload flow if
that succeeds; only emit the warning and skip upload when the fallback also
fails.

@fruch fruch Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Implemented - see the reply on the sibling thread for detail: sudo -n first, unprivileged journalctl as the fallback, skip only when both fail, and the archive/upload flow is shared by both success paths. One deviation from the suggestion: the fallback keeps --since @<build start> rather than --boot, because bounding the window is the whole point of this change and --boot on a long-lived agent would restore most of what it removes.

"""
}
6 changes: 5 additions & 1 deletion vars/collectTestCoredumps.groovy
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
#!groovy

def call(){
// Bound the coredump sweep to this build: on a long-lived agent the coredump directory holds
// every dump the host ever produced (other jobs' included), and unbounded the script tars and
// uploads all of it. Epoch seconds, consumed by upload_sct_coredump.sh via -newermt.
def sinceEpoch = (long) (currentBuild.startTimeInMillis / 1000)

@coderabbitai coderabbitai Bot Aug 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- vars/collectTestCoredumps.groovy ---'
cat -n vars/collectTestCoredumps.groovy
printf '%s\n' '--- vars/collectBuilderLogs.groovy ---'
cat -n vars/collectBuilderLogs.groovy
printf '%s\n' '--- references ---'
rg -n -F 'SCT_COREDUMPS_SINCE_EPOCH' . || true
rg -n -F 'collectTestCoredumps' . || true
rg -n -F 'collectBuilderLogs' . || true

Repository: scylladb/scylla-cluster-tests

Length of output: 2931

🏁 Script executed:

#!/bin/bash
set -e
printf '%s\n' '--- utils/upload_sct_coredump.sh ---'
cat -n utils/upload_sct_coredump.sh
printf '%s\n' '--- vars/runCollectLogs.groovy ---'
cat -n vars/runCollectLogs.groovy
printf '%s\n' '--- timestamp consumers ---'
rg -n -C 4 'SINCE_EPOCH|journalctl|newermt|find .*core|coredump' utils vars

Repository: scylladb/scylla-cluster-tests

Length of output: 13823

🏁 Script executed:

#!/bin/bash
set -e
python3 - <<'PY'
from decimal import Decimal

start_ms = 1_700_000_000_750
start_s = Decimal(start_ms) / Decimal(1000)
truncated_s = start_ms // 1000

samples = {
    "prior_same_second": Decimal("1700000000.500"),
    "build_start": start_s,
    "after_start": Decimal("1700000000.900"),
}
print(f"build_start={start_s}")
print(f"truncated_lower_bound={truncated_s}")
for name, record_s in samples.items():
    print(
        f"{name}: record={record_s} "
        f"included_with_truncated={record_s > truncated_s} "
        f"included_with_fractional={record_s > start_s}"
    )
PY

printf '%s\n' '--- GNU find fractional timestamp parsing ---'
find /dev/null -newermt '`@1700000000.750`' -print >/dev/null
echo "find_exit=$?"

printf '%s\n' '--- journalctl fractional timestamp parsing ---'
set +e
journalctl --since '`@1700000000.750`' --until '`@1700000000.751`' --no-pager --no-hostname -o cat >/dev/null 2>&1
status=$?
set -e
echo "journalctl_exit=$status"

Repository: scylladb/scylla-cluster-tests

Length of output: 641

🌐 Web query:

journalctl --since @ UNIX timestamp fractional seconds timestamp grammar

💡 Result:

When using the --since or --until options in journalctl, you can specify a UNIX epoch timestamp by prefixing an integer or floating-point number with the "@" character [1]. The systemd time parsing logic, documented in systemd.time(7), supports fractional seconds for UNIX timestamps [1]. This allows you to achieve sub-second precision by including a decimal component after the integer portion of the timestamp (e.g., --since=@1625097600.500) [2][1]. In practical application, if you are generating this timestamp in a script, it is common to use tools like date to produce the required format: journalctl --since=@$(date +%s.%6N) In this example, +%s.%6N produces the UNIX epoch followed by a decimal point and 6 digits of microsecond precision, which journalctl will parse correctly [2].

Citations:

Sensitive Data Exposure (CWE-200): Exposure of Sensitive Information to an Unauthorized Actor

Reachability: External · Exploitability: Moderate

Preserve millisecond precision in both collection bounds.

Use a fractional epoch value for SCT_COREDUMPS_SINCE_EPOCH and journalctl --since; the current truncation includes prior-tenant records created before the build within the same second.

📍 Affects 2 files
  • vars/collectTestCoredumps.groovy#L7-L7 (this comment)
  • vars/collectBuilderLogs.groovy#L8-L8
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@vars/collectTestCoredumps.groovy` at line 7, Preserve fractional epoch
precision in the since-time calculation used by collectTestCoredumps.groovy at
lines 7-7 and collectBuilderLogs.groovy at lines 8-8. Update the shared
collection-bound logic in each file so SCT_COREDUMPS_SINCE_EPOCH and journalctl
--since receive millisecond-precise values rather than truncated whole seconds.

@fruch fruch Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not changing this one, deliberately - the truncation is in the safe direction and the sub-second overlap is harmless.

currentBuild.startTimeInMillis / 1000 floors to whole seconds, so the window can start up to 999ms before the build did. That can only ever include slightly more than this build; rounding the other way would risk missing a coredump or journal entry written in the same second the build started, which is the failure that actually costs you a diagnosis. The bug being fixed here is weeks of unrelated history, not a millisecond boundary.

Fractional-epoch handling would also have to hold for both consumers (journalctl --since @… and SCT_COREDUMPS_SINCE_EPOCH in utils/upload_sct_coredump.sh), which is more surface than the benefit justifies. Happy to revisit if a real overlap ever shows up in a collected archive.

sh """#!/bin/bash

./utils/upload_sct_coredump.sh
SCT_COREDUMPS_SINCE_EPOCH=${sinceEpoch} ./utils/upload_sct_coredump.sh

@dimakr dimakr Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SCT_* names are for the SCT config namespace. sct_config.py doesn't define coredumps_since_epoch config param, so it would reject this as Unsupported environment variables were used

"""
}