Fix run on start#61965
Conversation
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
bugfood
force-pushed
the
fix-run_on_start
branch
from
e82b9a6 to
975c85d
Compare
|
They should still, however, run at a random interval after startup depending on the Tests like |
|
I do think that the |
|
I think the test failure on this PR is unrelated to the changes I made. |
dwoz
force-pushed
the
fix-run_on_start
branch
from
7c8278c to
21b04d6
Compare
|
Thanks for the interest in this old PR. I have not been active with saltstack work in the interim, but of course you can still use the changes if they are any good. Wouldn't it be better to rebase this branch against current master rather than merge master into this branch? |
|
Please rebase and fix the failing pre-commit |
This internal parameter is written to but never read from. The equivalent internal parameter _next_fire_time was introduced in 391424c; this parameter is actually used.
According to the document: https://docs.saltproject.io/salt/user-guide/en/latest/topics/scheduler.html By default, any job scheduled based on the startup time of the minion will run the scheduled job when the minion starts up. This was broken by 391424c and entrenched into the test suite by various commits afterward. Fix the code and adjust the unit tests to match. Add a new unit test to ensure that disabling job run at startup still works.
This test is unlike the seconds, minutes, and hours tests; dry_run is enabled unconditionally.
1. Ensure that jobs are run at minion startup as per the default `run_on_start: True`. 2. Ensure that jobs are not run before the splayed interval. 3. Update _check_last_run docstring to match behavior.
Rebased and amended. The pre-commit complaints seem to be about:
I think the CI pipeline needs to triggered to run. |
|
Congratulations on your first PR being merged! 🎉 |
|
Is this regression present in |
I think so, from looking at the code--there don't seem to have been any significant changes to the Thank you for the merge. -Corey |
4 participants
What does this PR do?
run_on_startparameter to match the documentation and older salt behavior.What issues does this PR fix or reference?
Fixes: #61964
Initial Behavior
Jobs with seconds/minutes/hours/days parameters would run on startup by default.
Regression from 391424c:
Such jobs no longer run on startup by default and need to be forced by `run_on_start: True'.
New Behavior
Such jobs now run on startup by default again.
Merge requirements satisfied?
[NOTICE] Bug fixes or features added to Salt require tests.
Commits signed with GPG?
No
Please review Salt's Contributing Guide for best practices.
See GitHub's page on GPG signing for more information about signing commits with GPG.