(RHEL-137251) Add new RestartMode= option#419
Open
dtardon wants to merge 3 commits into
Open
Conversation
github-actions
Bot
changed the title
RestartMode= optionRestartMode= option
github-actions
Bot
added
tracker/invalid-product
tracker/unapproved
Commit validationTracker - RHEL-137251 The following commits meet all requirements
Tracker validationSuccess🟢 Tracker RHEL-137251 has set desired product: Pull Request validationFailed🔴 Review - Missing review from a member (1 required) Success🟡 CI - Waived Triggered by Workflow Run |
github-actions
Bot
removed
tracker/invalid-product
tracker/unapproved
… substates
When a service deactivates and is then automatically restarted via
Restart= we currently quickly transition through
SERVICE_DEAD/SERVICE_FAILED. Which is weird given it's not the
normal ("permanent") dead/failed state, but a transitory one we
immediately leave from again. We do this so that software that looks for
failures/successes can take notice, even if we restart as a consequence
of the deactivation.
Let's clean this up a bit: let's introduce two new states:
SERVICE_DEAD_BEFORE_AUTO_RESTART and SERVICE_FAILED_BEFORE_AUTO_RESTART
that are used for the transitory states. Both the SERVICE_DEAD and
SERVICE_DEAD_BEFORE_AUTO_RESTART will map to the high-level
UNIT_INACTIVE state though. (and similar for the respective failed
states). This means the high-level state machine won't change by this,
only the low-level one.
This clearly seperates the substates, which makes the state engine
cleaner, and allows clients to follow precisely whether we are in a
transitory dead/failed state, or a permanent one, by looking at the
service substate. Moreover it allows us to remove the 'n_keep_fd_store'
which so far we used to ensure the fdstore was not released during this
transitory dead/failed state but only during the permanent one. Since we
can now distinguish these states properly we can just use that.
This has been bugging me for a while. Let's clean this up.
Note that the unit restart logic is already nicely covered in the
testsiute, hence this adds no new tests for that.
And yes, this could be considered a compat break, but sofar we took the
liberty to make changes to the low-level state machine (i.e. SERVICE_xyz
states, sometimes called "substates") without considering this a bad
breakage – the high-level state machine (i.e. UNIT_xyz states) should
be considered API that cannot be changed.
(cherry picked from commit a1d3157)
Related: RHEL-137251
When this option is set to direct, the service restarts without entering a failed state. Dependent units are not notified of transitory failure. This is useful for the following use case: We have a target with Requires=my-service, After=my-service. my-service.service is a oneshot service and has Restart=on-failure in its definition. my-service.service can get stuck for various reasons and time out, in which case it is restarted. Currently, when it fails the first time, the target fails, even though my-service is restarted. The behavior we're looking for is that until my-service is not restarted anymore, the target stays pending waiting for my-service.service to start successfully or fail without being restarted anymore. (cherry picked from commit e568fea) Resolves: RHEL-137251
Follow-up for a1d3157 and 6ac62d6 With the aforementioned commits, unit_release_resources() is dispatched in a dedicated queue, and Service.n_keep_fd_store has been dropped, hence the comment is outdated. Moreover, the unit is added to GC queue in unit_notify() already. No other unit types do this in corresponding _enter_dead() functions, nor does Service need it anymore. (cherry picked from commit 818315a) Related: RHEL-137251
dtardon
force-pushed
the
RHEL-137251-RestartMode=
branch
from
662c5c8 to
2dae6d1
Compare
Member
Author
|
|
github-actions
Bot
added
tracker/missing
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
Resolves: RHEL-137251