feat: add graceful VM shutdown on exit signals #316

vyasgun · 2025-06-02T11:01:35Z

Replace os.Exit() with channel-based shutdown to allow proper VM cleanup.
VM now stops gracefully with RequestStop() before forcing termination.

Fixes: #284

Summary by Sourcery

Refactor signal exit handling to notify the main goroutine via a channel instead of calling os.Exit directly to allow graceful VM shutdown.

Bug Fixes:

Ensure VM cleanup routines run on interrupt or termination signals instead of abruptly exiting

Enhancements:

Replace direct os.Exit in the exit handler with sending a signal on an exitChan for controlled shutdown
Wire exitChan into main to listen for exit signals and invoke VM.Stop before termination
Update util.SetupExitSignalHandling API to accept exitChan and propagate signals accordingly

Tests:

Adjust exit handler tests to match the new SetupExitSignalHandling signature without requiring a channel

Summary by Sourcery

Register a channel-based exit handler to gracefully stop the VM on termination signals instead of abruptly exiting

Bug Fixes:

Ensure VM.Stop is invoked on interrupt or termination signals to allow proper VM cleanup

Enhancements:

Refactor util.SetupExitSignalHandling to use an exit channel instead of calling os.Exit directly
Use util.RegisterExitHandler in main.go to invoke VM.Stop on exit signals

Tests:

Update exit handler tests to match the new SetupExitSignalHandling signature and channel-based design

sourcery-ai · 2025-06-02T11:01:39Z

Reviewer's Guide

The PR refactors signal-based termination to use a channel-driven exit handler, replacing direct os.Exit calls with controlled shutdown logic so that the VM’s Stop method is invoked before process termination.

Sequence Diagram for Graceful VM Shutdown via Exit Channel

sequenceDiagram
    actor User/OS
    participant SH as "Signal Handler (util.SetupExitSignalHandling)"
    participant EC as "exitChan"
    participant MG as "Main Goroutine (vfkit)"
    participant REH as "Registered Exit Handler"
    participant VM as "VirtualMachine (vfVM)"

    User/OS->>SH: Termination Signal (e.g. SIGINT, SIGTERM)
    SH->>EC: Send termination event on channel
    MG->>EC: Receives termination event from channel
    Note over MG: Initiates graceful shutdown sequence
    alt Graceful Shutdown Process
        MG->>REH: Runtime (or equivalent mechanism) invokes registered handler
        REH->>VM: Stop()
        VM-->>REH: VM Stopped successfully
    end
    Note over MG: Process exits cleanly

Class Diagram: Signal Handling and VM Control Components

classDiagram
    class MainApp {
        <<Go Application>>
        +runVFKit(vmConfig *config.VirtualMachine, opts *cmdline.Options) error
        #vfVM: VirtualMachine
        #exitChan: chan os.Signal
    }
    class Util {
        <<Go Package>>
        +SetupExitSignalHandling(exitChan chan os.Signal) void
        +RegisterExitHandler(handler func()) void
    }
    class VirtualMachine {
        +Stop() error
    }

    MainApp --> Util : uses signal handling utilities
    MainApp --> VirtualMachine : controls lifecycle

File-Level Changes

Change	Details	Files
Refactor exit handling to use channel-based signaling instead of os.Exit	Remove direct os.Exit calls in the util exit handler Modify SetupExitSignalHandling API to accept an exitChan Emit signals into exitChan on interrupt or termination	`util/exit.go` `util/exit_test.go`
Wire exitChan into main command to perform graceful VM shutdown	Call util.RegisterExitHandler in main.runVFKit Send log.Debugf and invoke vfVM.Stop() inside the handler Handle errors from VM.Stop with log.Error	`cmd/vfkit/main.go`
Update exit handler tests to match new SetupExitSignalHandling signature	Remove explicit channel setup in tests Assert correct signal propagation via exitChan Adapt mocks to use RegisterExitHandler	`util/exit_test.go`

Possibly linked issues

Sending termination signal does not trigger graceful shutdown #284: The PR registers the VM shutdown as an exit handler to ensure graceful termination on signals, fixing the abrupt exit issue.

Tips and commands

Interacting with Sourcery

Trigger a new review: Comment @sourcery-ai review on the pull request.
Continue discussions: Reply directly to Sourcery's review comments.
Generate a GitHub issue from a review comment: Ask Sourcery to create an
issue from a review comment by replying to it. You can also reply to a
review comment with @sourcery-ai issue to create an issue from it.
Generate a pull request title: Write @sourcery-ai anywhere in the pull
request title to generate a title at any time. You can also comment
@sourcery-ai title on the pull request to (re-)generate the title at any time.
Generate a pull request summary: Write @sourcery-ai summary anywhere in
the pull request body to generate a PR summary at any time exactly where you
want it. You can also comment @sourcery-ai summary on the pull request to
(re-)generate the summary at any time.
Generate reviewer's guide: Comment @sourcery-ai guide on the pull
request to (re-)generate the reviewer's guide at any time.
Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
pull request to resolve all Sourcery comments. Useful if you've already
addressed all the comments and don't want to see them anymore.
Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
request to dismiss all existing Sourcery reviews. Especially useful if you
want to start fresh with a new review - don't forget to comment
@sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

Enable or disable review features such as the Sourcery-generated pull request
summary, the reviewer's guide, and others.
Change the review language.
Add, remove or edit custom review instructions.
Adjust other review settings.

Getting Help

Contact our support team for questions or feedback.
Visit our documentation for detailed guides and information.
Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

sourcery-ai

Hey @vyasgun - I've reviewed your changes - here's some feedback:

Consider making exitChan buffered or using a non‐blocking select when sending to it to avoid blocking the signal handler goroutine if no receiver is ready.
setupExitSignalHandling should guard against a nil exitChan (e.g. by checking before sending) to prevent deadlocks when tests pass nil.
Rather than launching a separate goroutine in main, you could use util.RegisterExitHandler to register the VM stop logic and keep all shutdown handlers in one place.

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

vyasgun · 2025-06-02T11:12:03Z

@sourcery-ai review

sourcery-ai

Hey @vyasgun - I've reviewed your changes and they look great!

Here's what I looked at during the review

🟢 General issues: all looks good
🟢 Security: all looks good
🟢 Testing: all looks good
🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

cfergeau · 2025-06-02T15:41:44Z

cmd/vfkit/main.go

+		if err != nil {
+			log.Error(err)
+		}
+	})


I don’t think we want to always run vfVM.Stop() in all cases when we exit vfkit.
As Nir summarized, there are (at least) 3 ways to exit vfkit

guest stops and vfkit exits

the REST API is used to stop/hard stop the VM

a signal is sent to the vfkit process

It’s only for 3) that we want to try to run vfVM.Stop(), ie run it somehow here

vfkit/pkg/util/exithandler.go

Lines 42 to 48 in b67809f

for sig := range sigChan {

log.Printf("captured %v, calling exit handlers and exiting..", sig)

ExecuteExitHandlers()

if doExit {

os.Exit(1)

}

}

and don’t call ExecuteExitHandler/os.Exit(), but instead let the VM stop, and then the cleanup at exit for 3) can also be done by this code:

vfkit/cmd/vfkit/root.go

Line 34 in b67809f

defer util.ExecuteExitHandlers()

For 3), maybe we also need to do hardstop after a timeout if the VM does not exit in a reasonable time?

I updated the PR so that there is a channel which captures when exitHandlers are executing and doExit is set. Upon receiving the signal, it will RequestStop the VM and a hardstop if the state of the vm doesn't change to Stopped after 5 seconds.

vyasgun · 2025-06-10T09:08:45Z

@sorcery-ai summary

cmd/vfkit/main.go

pkg/util/exithandler.go

cfergeau · 2025-06-10T19:46:38Z

pkg/util/exithandler.go

 // When one of these signals is received, all the registered exit handlers will be invoked.
 // It is possible to prevent the program from exiting by setting the doExit param to false (used for testing)
-func setupExitSignalHandling(doExit bool) {
+func setupExitSignalHandling(exitChan chan bool, doExit bool) {


An alternative to this exitChan approach would be to pass a handler func() similar to what is done with RegisterExitHandler. But both approaches are fine, mentioning it here for completeness, changes are not required.

I changed the code so it accepts a shutdown function and executes it. This makes testing also easier as I can provide a test implementation of the shutdown func

cfergeau

Apart from the ExecuteExitHandler discussion, this looks good to me.

Replace os.Exit() with shutdown func to allow proper VM cleanup. VM now stops gracefully with RequestStop() before forcing termination.

cfergeau

/lgtm
/approve

openshift-ci · 2025-06-16T10:44:09Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: cfergeau

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details

Needs approval from an approver in each of these files:

~~OWNERS~~ [cfergeau]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

openshift-ci bot requested review from cfergeau and praveenkumar June 2, 2025 11:01

sourcery-ai bot reviewed Jun 2, 2025

View reviewed changes

vyasgun force-pushed the pr/graceful-shutdown branch from 4f88261 to c42e449 Compare June 2, 2025 11:10

vyasgun changed the title ~~refactor: replace direct os.Exit with graceful shutdown via channel~~ fix: register VM shutdown as exit handler for graceful termination Jun 2, 2025

sourcery-ai bot reviewed Jun 2, 2025

View reviewed changes

cfergeau reviewed Jun 2, 2025

View reviewed changes

vyasgun force-pushed the pr/graceful-shutdown branch from c42e449 to d6a65a5 Compare June 10, 2025 09:00

vyasgun changed the title ~~fix: register VM shutdown as exit handler for graceful termination~~ feat: add graceful VM shutdown on exit signals Jun 10, 2025

cfergeau reviewed Jun 10, 2025

View reviewed changes

vyasgun force-pushed the pr/graceful-shutdown branch from d6a65a5 to d424730 Compare June 12, 2025 09:18

cfergeau reviewed Jun 13, 2025

View reviewed changes

vyasgun force-pushed the pr/graceful-shutdown branch 3 times, most recently from 920d736 to 3597a2d Compare June 16, 2025 07:58

feat: add graceful VM shutdown on exit signals

8297bfc

Replace os.Exit() with shutdown func to allow proper VM cleanup. VM now stops gracefully with RequestStop() before forcing termination.

vyasgun force-pushed the pr/graceful-shutdown branch from 3597a2d to 8297bfc Compare June 16, 2025 07:59

cfergeau approved these changes Jun 16, 2025

View reviewed changes

openshift-ci bot assigned cfergeau Jun 16, 2025

openshift-ci bot added the lgtm label Jun 16, 2025

openshift-ci bot added the approved label Jun 16, 2025

openshift-merge-bot bot merged commit a74a927 into crc-org:main Jun 16, 2025
7 of 8 checks passed

	for sig := range sigChan {
	log.Printf("captured %v, calling exit handlers and exiting..", sig)
	ExecuteExitHandlers()
	if doExit {
	os.Exit(1)
	}
	}

feat: add graceful VM shutdown on exit signals #316

feat: add graceful VM shutdown on exit signals #316

Uh oh!

Conversation

vyasgun commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by Sourcery

Summary by Sourcery

Uh oh!

sourcery-ai bot commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reviewer's Guide

Sequence Diagram for Graceful VM Shutdown via Exit Channel

Class Diagram: Signal Handling and VM Control Components

File-Level Changes

Possibly linked issues

Interacting with Sourcery

Customizing Your Experience

Getting Help

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

vyasgun commented Jun 2, 2025

Uh oh!

sourcery-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

cfergeau Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vyasgun Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

vyasgun commented Jun 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

cfergeau Jun 10, 2025

Choose a reason for hiding this comment

Uh oh!

vyasgun Jun 16, 2025

Choose a reason for hiding this comment

Uh oh!

cfergeau left a comment

Choose a reason for hiding this comment

Uh oh!

cfergeau left a comment

Choose a reason for hiding this comment

Uh oh!

openshift-ci bot commented Jun 16, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

vyasgun commented Jun 2, 2025 •

edited

Loading

sourcery-ai bot commented Jun 2, 2025 •

edited

Loading

cfergeau Jun 2, 2025 •

edited

Loading