WIP: replace cleanup scriptlets with shutdown script #5860
Conversation
This file contains 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
garlick
force-pushed
the
shutdown
branch
2 times, most recently
from
69adb5e
to
9883eec
Compare
|
Fixed some stuff
|
garlick
force-pushed
the
shutdown
branch
3 times, most recently
from
82bc90c
to
579104e
Compare
garlick
changed the title
|
I dropped the actual changes to the shutdown process from this PR (including I think the script is going to be far more maintainable than the "cleanup scriptlets" pushed by rc1, and it opens the possibility of letting A downside is that this impacts the tests that set rc1,rc3, including some in other projects. (Thus cleanup-push remains as an alias until the other projects can transition) |
Problem: the shutdown script will coexist with rc1 and rc3 as an alternative to the "cleanup" commands currently pushed into broker memory by rc1, but it has no path configuration. Add builtin 'shutdown_path' config key, like to 'rc1_path' and 'rc3_path'. Add 'broker.shutdown_path' broker attribute, like 'broker.rc1_path' and 'broker.rc3_path'. Update flux-config(1) and flux-broker-attributes(7).
Problem: the cleanup commands pushed into broker memory in rc1 are not easily maintained or extended. Create a new "shutdown" script that lives next to rc1 and rc3. Run this script instead of the "cleanup" commands when the broker CLEANUP state is entered. Upon completion of the shutdown script, the broker transitions to the SHUTDOWN state.
Problem: test "personalites" like "job" and "kvs" select alternate rc1 and rc3 scripts, but not shutdown. Require that a personality define a shutdown script even if empty, and add them for "job" and "kvs".
Problem: some tests assume that clearing the rc1/rc3 paths is sufficient to bypass all default startup and shutdown activities, but now the shutdown script path must also be cleared. Modify test scripts to take the shutdown script into consideration when modifying rc1/rc3 paths.
Problem: a function call with a long parameter list is not broken to one per line, as per project norms. Fix line breaks.
Problem: rc1, rc3 scripts are run with shell -c <script> but they could be executed directly. Run scripts directly. Adjust one test.
Problem: some framework projects are using "test personalities" that call flux admin cleanup-push. Accept "cleanup" as an alias for "shutdown" to allow those projects to continue to work while transitioning to the shutdown script. Note: although these commands are no longer interpreted by the shell, none of those projects appear to be relying on that.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #5860 +/- ##
==========================================
- Coverage 83.31% 83.27% -0.05%
==========================================
Files 514 514
Lines 82801 82827 +26
==========================================
- Hits 68986 68974 -12
- Misses 13815 13853 +38
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This makes some improvement to the shutdown process:
flux admin cleanup-pushmethod of registering cleanup tasks in rc1, and instead have ashutdownscript similar to other rc scriptsflux shutdown --forcewhich causes the shutdown script to be called with the-foptionflux queue idlewhen-fis presentTodo:
flux shutdown --forceinsystemctl stop fluxflux adminis removed so deal with thatI thought maybe a pause at this point to get feedback would be good. This had a bigger impact on tests than I was expecting, so maybe I took a wrong turn somewhere.