Improve signal handling - Make the state machine context aware #231
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #231 +/- ##
==========================================
+ Coverage 93.99% 94.00% +0.01%
==========================================
Files 18 18
Lines 3412 3418 +6
==========================================
+ Hits 3207 3213 +6
Misses 132 132
Partials 73 73
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
upils
force-pushed
the
improve-signal-handling
branch
from
23a82a4 to
abab7f3
Compare
upils
force-pushed
the
improve-signal-handling
branch
from
abab7f3 to
99f2fb3
Compare
Signed-off-by: Paul Mars <[email protected]>
Signed-off-by: Paul Mars <[email protected]>
For now we are not doing anything with the context but this is the first step to enable proper cancellation. Also adding a cancelFuncs in stateMachine is to prepare calling these functions to properly clean things when a stateFuncs is stopped. Signed-off-by: Paul Mars <[email protected]>
upils
force-pushed
the
improve-signal-handling
branch
from
99f2fb3 to
e58f999
Compare
| ch := make(chan os.Signal, 2) | ||
| signal.Notify(ch, syscall.SIGINT, syscall.SIGTERM) | ||
|
|
||
| go func() { |
There was a problem hiding this comment.
Do you already have plans for the signal handler? Like, if the cleanup process blocks or takes too long for some reason (like trying to call umount on a problematic mounting point (I don't know, imagine a bad disk or NFS mount that got stuck)) it should be possible to still interrupt the tool (you could kill -9 it anyway).
If you ctrl+c again while the signal handler is running nothing will happen because the signal is being caught. You could handle subsequent ctrl+c's and inform the user that the tool is shutting down but if they want to stop it anyway they can hit ctrl+c again.
There was a problem hiding this comment.
Yes! That is the plan in the following PR #235 (still very much WIP)
Hitting Ctrl-C will currently immediately kill ubuntu-image and every underlying subcommand. This is problematic because several states are mounting some important directories and thus cancelling the execution could leave the system in a broken state. Unaware users could then try to clean the work directory and thus remove important directories from their system.
To properly cancel the execution, we need to:
This PR is dealing with the first 2 steps.