network: start dnsconfd in initramfs #6200
Open
+56
−0
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
Resolves: RHEL-80302
rvykydal
force-pushed
the
start-dnsconfd-in-initramfs-r10
branch
from
0ac32b7 to
e59efe7
Compare
|
/kickstart-test --testtype smoke |
jkonecny12
approved these changes
Feb 26, 2025
| fi | ||
|
|
||
| if [ "${start}" == "yes" ]; then | ||
| echo "Attempting to start dnsconfd. Starting." |
There was a problem hiding this comment.
I think you can just write it starting as this shouldn't be just an attempt.
There was a problem hiding this comment.
I wanted make easier matching the message to the previous related log message Attempting to start dnsconfd. Reason: ${reason} when looking at the log.
| if [ "${start}" == "yes" ]; then | ||
| echo "Attempting to start dnsconfd. Starting." | ||
| systemctl start --no-block unbound.service | ||
| return 0 |
There was a problem hiding this comment.
Don't we want to have return $? instead to catch possible failure?
There was a problem hiding this comment.
I think we'd care more about the information 'should the service be started now?'. The return value is not needed anywhere anyway. I think at this point it is just too early complicate the things by figuring stable expected return values.
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.
Resolves: RHEL-80302
I was trying to add dracut unit test (like here) but the logic is very sparse here and we'd need to figure so much mocking (boot options, existing files) that I gave up. The only option seemed to me to test a function that would take all the possible contitions as parameters (like "can_import_certificates_later", ore "is_required_by_boot_options") and let an (untested) wrapper collect the parameters.