-
Notifications
You must be signed in to change notification settings - Fork 788
Avoid boot loop when there are dnf network issues #4442
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
jandubois
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand which part of the PR is supposed to stop the reboot loop in case there is a network problem.
| {{- if .UpgradePackages}} | ||
| LIMA_CIDATA_UPGRADE_PACKAGES=1 | ||
| {{- else}} | ||
| LIMA_CIDATA_UPGRADE_PACKAGES=0 | ||
| {{- end}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It bothers me that we do these silly translations from booleans to environment variables that have to be set to 1 or the empty string. Or in this case 0.
Why don't we assign the boolean value directly and deal with it that the variable will be set to true or false?
| {{- if .UpgradePackages}} | |
| LIMA_CIDATA_UPGRADE_PACKAGES=1 | |
| {{- else}} | |
| LIMA_CIDATA_UPGRADE_PACKAGES=0 | |
| {{- end}} | |
| LIMA_CIDATA_UPGRADE_PACKAGES={{.UpgradePackages}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was just following the rest of them, either way
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then it would be more on the lines of:
$LIMA_CIDATA_UPGRADE_PACKAGES || exit 0There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And using 0 is a bit unconventional, not "ifdef"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently that was my bad, it was only 1 being used
5133ee7 to
eb9c142
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Only try to reboot when upgradePackages is true, and check the network connectivity before checking if a restart is needed. Also don't hide the stderr output, in case of any errors, and also show the list of packages that are requiring the reboot. Signed-off-by: Anders F Björklund <[email protected]>
eb9c142 to
ad8e9c2
Compare
Only try to reboot when upgradePackages is true, and check the network connectivity before checking if a restart is needed.
Also don't hide the stderr output, in case of any errors, and also show the list of packages that are requiring the reboot.
Closes #4440
first boot:
second boot: