cmd: add support for restarting a running instance #3323

unsuman · 2025-03-09T20:06:12Z

This PR adds a new command limactl restart INSTANCE which gracefully terminates and restarts Lima instances.

cmd/limactl/shell.go

jandubois · 2025-03-10T06:53:53Z

cmd/limactl/shell.go

+		}
+
+		launchHostAgentForeground := false
+		time.Sleep(500 * time.Millisecond)


Needing a sleep is always a red flag; it means there is a race, and there is still a chance that the sleep is not long enough.

I suspect the sleep exists because we need to wait for the PID files to be deleted before we can start the instance again. I wonder if it isn't possible to make sure StopGracefully will not return until the files are deleted, and return an error when the files still exist after a somewhat longish timeout.

I suspect the sleep exists because we need to wait for the PID files to be deleted before we can start the instance again.

It is indeed the case, the StopGracefully() function is not waiting for the host agent and driver processes to shut down, let me see if I can fix this in this PR itself.

cmd/limactl/shell.go

pkg/instance/restart.go

AkihiroSuda · 2025-03-10T12:29:41Z

cmd/limactl/restart.go

+	restartCmd := &cobra.Command{
+		Use:               "restart INSTANCE",
+		Short:             "Restart a running instance",
+		Args:              WrapArgsError(cobra.MinimumNArgs(1)),


The implementation seems ignoring args[1:]

cmd/limactl/restart.go

pkg/instance/restart.go

jandubois · 2025-03-10T18:35:59Z

pkg/instance/restart.go

+	return nil
+}
+
+func waitForInstanceShutdown(ctx context.Context, inst *store.Instance) error {


I think this should be part of waitForHostAgentTermination in pkg/instance/stop.go, or at least called from StopGracefully. That makes it available to an external caller and protects limactl stop && limactl start from failing too.

Is there a reason we would ever want StopGracefully to return before the processes have shut down?

The reason I added this function is because I was facing this issue while restarting a VM:

FATA[0001] instance "default" seems running (hint: remove "/Users/ansumansahoo/.lima/default/ha.pid" if the instance is not running)

But we don't need this function anymore because networks.Reconcile() ensures everything is shutdown before a restart.

But we don't need this function anymore because networks.Reconcile() ensures everything is shutdown before a restart.

How does it ensure that? It is not immediately obvious to me. Does it just take sufficient time on your machine, so the problem no longer appears (in which case this would be a variant of the sleep mechanism, and not a guarantee)?

You are correct; I didn't think of it that way. Thanks! To be on the safer side, I will add the function that ensures the instance has been shutdown before starting it again.

cmd/limactl/restart.go

jandubois

Needs checking for maximum number of args
I don't understand why we don't need to wait for the instance to be stopped
commits should be squashed

cmd/limactl/restart.go

jandubois · 2025-03-11T20:43:22Z

pkg/instance/restart.go

+	return nil
+}
+
+func waitForInstanceShutdown(ctx context.Context, inst *store.Instance) error {


But we don't need this function anymore because networks.Reconcile() ensures everything is shutdown before a restart.

How does it ensure that? It is not immediately obvious to me. Does it just take sufficient time on your machine, so the problem no longer appears (in which case this would be a variant of the sleep mechanism, and not a guarantee)?

jandubois · 2025-03-11T20:54:43Z

I just tested restarting a stopped instance:

❯ limactl restart
FATA[0000] expected status "Running", got "Stopped" (maybe use `limactl stop -f`?)

I think this is correct, you need to specify limactl restart -f.

But I wonder if the error message is correct. The user obviously expects the instance to be running when the command returns. So the suggested command should probably be limactl start (or limactl restart -f).

@lima-vm/maintainers WDYT?

AkihiroSuda · 2025-03-12T03:54:39Z

I just tested restarting a stopped instance:

It probably should just start the instance automatically, with a warning

unsuman · 2025-03-12T10:34:19Z

pkg/instance/stop.go

 	if inst.Status != store.StatusRunning {
+		if isRestart, ok := ctx.Value("restart").(bool); ok && isRestart {


Is this the correct way of doing?

Doesn't seem so? Is there any reason to use ctx here?

I just tested restarting a stopped instance:

It probably should just start the instance automatically, with a warning

I used ctx here to find out who is calling StopGracefully. If the instance is being restarted and is already shut down, it won’t cause an error and will continue with starting the instance.

Why not just add an argument to the function?

Signed-off-by: Ansuman Sahoo <anshumansahoo500@gmail.com>

AkihiroSuda reviewed Mar 9, 2025

View reviewed changes

cmd/limactl/shell.go Outdated Show resolved Hide resolved

jandubois reviewed Mar 10, 2025

View reviewed changes