Skip to content
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

(MODULES-10955) More robust handling of reboot-task output #280

Merged
merged 1 commit into from
Mar 4, 2021
Merged

(MODULES-10955) More robust handling of reboot-task output #280

merged 1 commit into from
Mar 4, 2021

Conversation

fetzerms
Copy link
Contributor

@fetzerms fetzerms commented Feb 24, 2021

The reboot task (that gets run in line 30) always returns the status and timeout. The timeout is (per definition) the max between 3 and $reboot_delay. No real dynamic output that is generated by the reboot task is used in the plan.

Hence: We can default to a $wait for the maximum of 3 or $reboot_delay. This way it does not matter to the plan what the reboot task runs. So broadcast-messages (and possibly other things) do not interfere with the parsing of the timeout.

The reboot task (that gets run in line 30) always returns the status and timeout. The timeout is (per definition) the max between 3 and $reboot_delay.
No real dynamic output that is generated by the reboot task is used in the plan.

Hence: We can default to a $wait for the maximum of 3 or $reboot_delay. This way it does not matter to the plan what the reboot task runs.
So broadcast-messages (and possibly other things) do not interfere with the parsing of the timeout.
@fetzerms fetzerms requested a review from a team as a code owner February 24, 2021 17:45
@CLAassistant
Copy link

CLAassistant commented Feb 24, 2021

CLA assistant check
All committers have signed the CLA.

@codecov-io
Copy link

Codecov Report

Merging #280 (926ee5e) into main (1141637) will decrease coverage by 31.84%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main     #280       +/-   ##
===========================================
- Coverage   81.52%   49.68%   -31.85%     
===========================================
  Files           6        6               
  Lines         314      314               
===========================================
- Hits          256      156      -100     
- Misses         58      158      +100     
Impacted Files Coverage Δ
lib/puppet/provider/reboot/windows.rb 23.18% <0.00%> (-72.47%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1141637...926ee5e. Read the comment docs.

@fetzerms
Copy link
Contributor Author

The decrease in code coverage seems to be from windows.rb, which I did not touch. Anything else that needs to be done, in order to get this PR merged?

@david22swan david22swan merged commit 494eff6 into puppetlabs:main Mar 4, 2021
@david22swan
Copy link
Member

@fetzerms Thank you for your change and sorry it took a while to get around to merging it.
Looking forward to seeing more from you in the future :)

@pmcmaw pmcmaw added the bugfix label Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants