dracut: add dependency network to ignition-mount.service#1940
Merged
jlebon merged 1 commit intocoreos:mainfrom Sep 13, 2024
Merged
dracut: add dependency network to ignition-mount.service#1940jlebon merged 1 commit intocoreos:mainfrom
jlebon merged 1 commit intocoreos:mainfrom
Conversation
dustymabe
reviewed
Sep 12, 2024
Comment on lines
16
to
17
| # and that network is enabled for provider with a network dependency | ||
| # like Equinix Metal (Packet) |
Member
There was a problem hiding this comment.
I'm a fan of more verbosity to try to explain the nuance if you don't mind. Something like:
Suggested change
| # and that network is enabled for provider with a network dependency | |
| # like Equinix Metal (Packet) | |
| # and that we order ourselves after network such that | |
| # if networking is brought up it will still be available | |
| # for our ExecStop= command. On some providers like Equinix | |
| # Metal (Packet) there is a network callback sent out | |
| # for each Ignition stage (including umount). |
Contributor
Author
There was a problem hiding this comment.
Perfect, thanks for the suggestion.
tormath1
force-pushed
the
tormath1/ignition
branch
from
2d278e9 to
2909beb
Compare
Member
jlebon
approved these changes
Sep 13, 2024
Member
jlebon
left a comment
jlebon
left a comment
There was a problem hiding this comment.
Thanks for the patch! Commits could be combined but LGTM as is.
docs/release-notes.md
Outdated
| ### Bug fixes | ||
|
|
||
| - Fix Akamai Ignition base64 decoding on padded payloads | ||
| - Add network dependency to `ignition-mount.service` |
Member
There was a problem hiding this comment.
Minor: I guess technically we're not adding a dependency, only a strong ordering. Also these notes are typically written to be more concrete to the end-user. E.g. something like
Suggested change
| - Add network dependency to `ignition-mount.service` | |
| - Fix network race when phoning home on Equinix |
?
Contributor
Author
There was a problem hiding this comment.
Good to me. Done and I squashed the two commits.
On some providers (like Equinix Metal), there is a network dependency for the umount stage, network must be still around when ExecStop is executed. Signed-off-by: Mathieu Tortuyaux <mtortuyaux@microsoft.com> Co-authored-by: Dusty Mabe <dusty@dustymabe.com>
tormath1
force-pushed
the
tormath1/ignition
branch
from
2909beb to
6eb35ed
Compare
jlebon
approved these changes
Sep 13, 2024
3 tasks
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
3 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.
On some providers (like Equinix Metal), there is a network dependency for the umount stage, network must be still around when ExecStop is executed.
Thanks @dustymabe for the suggestion on the fix. 💪
Tested on Flatcar CI with Equinix Metal tests: