Skip to content

Add what's needed to make probes work with normal cron #482

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

Add what's needed to make probes work with normal cron #482

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
51 changes: 51 additions & 0 deletions probes/DeterministicDelay
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
#! /usr/bin/env python3

import argparse
import hashlib
import subprocess
import time

parser = argparse.ArgumentParser(
prog='DeterministicDelay',
description=('Delays running a command by a consistent amount of time. '
'This may be useful in spreading the load out from frequently running cronjobs.'),
epilog=None)

parser.add_argument('-d', required=False, dest='delay',
help=("The maximum amount of time to delay. "
"60 seconds is the default, suffix "
"with s (the default), m, h, or d for seconds, minutes, hours, or days.")
)
parser.add_argument('commands', nargs='*', help="The commands to run")
args = parser.parse_args()

command_hash = hashlib.sha256(bytes(str(args.commands), 'utf-8'))
checksum = int(command_hash.hexdigest(), 16)

if not args.delay:
max_delay = 60
else:
if args.delay[-1] in ['s', 'm', 'h', 'd']:
delay_value, delay_unit = args.delay[:-1], args.delay[-1]
if delay_unit == 'm':
max_delay = int(delay_value) * 60
elif delay_unit == 'h':
max_delay = int(delay_value) * 60 * 60
elif delay_unit == 'd':
max_delay = int(delay_value) * 24 * 60 * 60
else:
max_delay = int(delay_value)
else:
max_delay = int(args.delay)
Comment on lines +28 to +39
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The delay argument parsing does not handle invalid input. If a non-numeric value is provided (e.g., -d abc or -d 5x), the int() conversion will raise a ValueError. Consider adding error handling with a try-except block to provide a meaningful error message instead of crashing with an unhandled exception.

Suggested change
if args.delay[-1] in ['s', 'm', 'h', 'd']:
delay_value, delay_unit = args.delay[:-1], args.delay[-1]
if delay_unit == 'm':
max_delay = int(delay_value) * 60
elif delay_unit == 'h':
max_delay = int(delay_value) * 60 * 60
elif delay_unit == 'd':
max_delay = int(delay_value) * 24 * 60 * 60
else:
max_delay = int(delay_value)
else:
max_delay = int(args.delay)
try:
if args.delay[-1] in ['s', 'm', 'h', 'd']:
delay_value, delay_unit = args.delay[:-1], args.delay[-1]
if delay_unit == 'm':
max_delay = int(delay_value) * 60
elif delay_unit == 'h':
max_delay = int(delay_value) * 60 * 60
elif delay_unit == 'd':
max_delay = int(delay_value) * 24 * 60 * 60
else:
max_delay = int(delay_value)
else:
max_delay = int(args.delay)
except ValueError:
parser.error(
f"Invalid delay value '{args.delay}'. Delay must be a non-negative integer "
"optionally suffixed with s, m, h, or d."
)

Copilot uses AI. Check for mistakes.

delay = checksum % max_delay
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

If the delay argument is '0' or '0s', max_delay will be 0, causing a ZeroDivisionError when computing checksum % max_delay on line 41. The script should validate that max_delay is greater than 0, or handle the zero case by not delaying at all.

Suggested change
delay = checksum % max_delay
if max_delay <= 0:
delay = 0
else:
delay = checksum % max_delay

Copilot uses AI. Check for mistakes.

print(f"Sleeping {delay}s")
time.sleep(delay)

executed = subprocess.run(args.commands, capture_output=True)
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

When no commands are provided (empty args.commands list), subprocess.run will fail with an error. The script should validate that at least one command is provided and print a meaningful error message. Consider adding validation like if not args.commands: parser.error('No commands provided') after parsing arguments.

Copilot uses AI. Check for mistakes.

print("STDOUT")
print(executed.stdout)
print("STDERR")
print(executed.stderr)
Comment on lines +46 to +51
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The subprocess.run call does not check the return code or propagate the exit status of the executed command. This means that cron jobs will always appear to succeed even if the actual command fails. Consider adding sys.exit(executed.returncode) after printing the output to properly propagate the exit status.

Copilot uses AI. Check for mistakes.
Comment on lines +48 to +51
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The output from subprocess.run is captured as bytes but printed directly. This will result in output like b'...' being printed instead of the actual text. The bytes should be decoded before printing, for example: print(executed.stdout.decode('utf-8')) and print(executed.stderr.decode('utf-8')). Alternatively, you can add text=True to the subprocess.run call to get string output directly.

Copilot uses AI. Check for mistakes.
2 changes: 2 additions & 0 deletions probes/Dockerfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ ADD oic.rpm /tmp
RUN dnf install -y epel-release.noarch && \
dnf upgrade -y && \
dnf install -y \
cronie-noanacron \
git \
gcc \
libnsl \
Expand Down Expand Up @@ -49,5 +50,6 @@ RUN git clone https://github.com/rucio/probes.git
ADD rucio.config.default.cfg /tmp/

ADD run-probes.sh /
ADD DeterministicDelay /
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The DeterministicDelay script is added to the container but is not made executable. The Dockerfile should include a RUN chmod +x /DeterministicDelay command after the ADD instruction, or the file should be made executable before being added to the image. Without execute permissions, the script cannot be run directly from cron jobs.

Suggested change
ADD DeterministicDelay /
ADD DeterministicDelay /
RUN chmod +x /DeterministicDelay

Copilot uses AI. Check for mistakes.

ENTRYPOINT ["/run-probes.sh"]
20 changes: 13 additions & 7 deletions probes/run-probes.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,20 @@ if [ ! -z "$RUCIO_PRINT_CFG" ]; then
echo ""
fi

cp /etc/jobber-config/dot-jobber.yaml /root/.jobber
if [ ! -z "$RUCIO_USING_CRON" ]; then
echo "Setting up and starting cron"
cp /etc/cron.rucio/probes-crontab /etc/cron.d/
Copy link

Copilot AI Feb 4, 2026

Choose a reason for hiding this comment

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

The script references /etc/cron.rucio/probes-crontab but this file is not added to the Docker image in the Dockerfile. The cron configuration will fail to copy when RUCIO_USING_CRON is set, causing the container to fail. You need to either add the crontab file to the Dockerfile (e.g., ADD probes-crontab /etc/cron.rucio/) or create it during the build process.

Suggested change
cp /etc/cron.rucio/probes-crontab /etc/cron.d/
if [ -f /etc/cron.rucio/probes-crontab ]; then
cp /etc/cron.rucio/probes-crontab /etc/cron.d/
else
echo "Warning: /etc/cron.rucio/probes-crontab not found; skipping probes cron configuration."
fi

Copilot uses AI. Check for mistakes.
crond -n -s
else
cp /etc/jobber-config/dot-jobber.yaml /root/.jobber

echo "Starting Jobber"
/usr/local/libexec/jobbermaster &
echo "Not using cron. Starting Jobber"
/usr/local/libexec/jobbermaster &

sleep 5
sleep 5

echo
echo "============= Jobber log file ============="
echo
echo "============= Jobber log file ============="

tail -f /var/log/jobber-runs
tail -f /var/log/jobber-runs
fi