Skip to content

Fix recurring timer tests at top of hour - #888

Open
jakduch wants to merge 2 commits into
theforeman:masterfrom
jakduch:fix/recurring-timer-top-of-hour-race
Open

jakduch wants to merge 2 commits into
theforeman:masterfrom
jakduch:fix/recurring-timer-top-of-hour-race

Conversation

@jakduch

@jakduch jakduch commented Sep 26, 2026 •

Copy link
Copy Markdown
Contributor

Why are you introducing these changes? (Problem description, related links)

The hourly recurring timer can fire while the feature suite is reaching its timer tests. In that state systemd temporarily omits NextElapseUSecRealtime, and systemctl start is coalesced with the already-running service. The tests then fail even though the timer-triggered service started and completed successfully.

This reproduced in two independent matrix jobs:

Fixes #887

What are the changes introduced in this pull request?

  • Accept the timer's firing state only when it has a recorded trigger and its service is starting, running, or stopping successfully.
  • Use an already-active live invocation as execution coverage.
  • Preserve the existing future-elapse and new-InvocationID checks when the service is idle.
  • Reject a service that is deactivating after a failed main process, with deterministic unit coverage.

How to test this pull request

  • ruff check tests src development/scripts inventories
  • python -m pytest tests/unit/recurring_timer_test.py -vv (5 passed)
  • python -m pytest tests/feature/foreman/base_test.py --collect-only -q (39 tests collected)
  • python -m py_compile tests/feature/foreman/base_test.py tests/unit/recurring_timer_test.py
  • git diff --check

The live timer behavior requires the existing deployed-VM CI matrix.

Checklist

  • Tests added/updated (if applicable)
  • Documentation updated (not applicable; test-only change)

@coderabbitai

coderabbitai Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 35 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Advanced

Run ID: 10801ac3-1736-44e4-8ba5-b3c2dc32fc45

📥 Commits

Reviewing files that changed from the base of the PR and between 6c79648 and f9c0326.

📒 Files selected for processing (2)
  • tests/feature/foreman/base_test.py
  • tests/unit/recurring_timer_test.py

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jakduch

jakduch commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Follow-up hardening is in f9c0326:

  • all three recurring-service state checks now use the same predicate;
  • active and activating remain accepted;
  • deactivating is accepted only with Result=success;
  • a deterministic test confirms that deactivating with Result=exit-code is rejected.

Local validation: ruff passed, all 5 focused unit cases passed, the 39 Foreman feature tests collect successfully, Python compilation passed, and git diff --check passed. The updated matrix is running in Actions run 36270656288.

@jakduch

jakduch commented Sep 26, 2026

Copy link
Copy Markdown
Contributor Author

Exact-head CI is green on f9c0326.

Actions run 36270656288 passed all 25 jobs, including Python and Ansible lint, the full deployed-VM matrix, proxy/capsule/satellite deployments, migration, upgrade, and the aggregate test-suite gate. Both Packit RPM builds also passed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Avoid recurring timer test race at the top of the hour

1 participant