Skip to content

limactl start logic is broken in 1.0.4 #3142

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

Closed
jandubois opened this issue Jan 23, 2025 · 5 comments · Fixed by #3144
Closed

limactl start logic is broken in 1.0.4 #3142

jandubois opened this issue Jan 23, 2025 · 5 comments · Fixed by #3144
Labels
regression Used to work but has been broken

Comments

@jandubois
Copy link
Member

Description

$ make native
...
$ rm -rf _output/share/lima/templates/
$ make install
...
$ limactl create --tty=false my-vm.yaml
...
INFO[0000] Run `limactl start my-vm` to start the instance.
$ limactl start my-vm
INFO[0000] Creating an instance "my-vm" from template://default (Not from template://my-vm)
WARN[0000] This form is deprecated. Use `limactl create --name=my-vm template://default` instead
FATA[0000] open /usr/local/share/lima/templates/default.yaml: no such file or directory

The message about creating an instance is wrong (the instance already exists). And it throws a fatal error when the default template doesn't exist, even though there is no reason to access it.

The original VM seems to start correctly (and is not recreated based of the default template) when the default template exists, but the INFO and WARN lines are still false and misleading.

I don't have time to look into this right now, but suspect it is due to #3120.

@jandubois jandubois added the regression Used to work but has been broken label Jan 23, 2025
@nirs
Copy link
Member

nirs commented Jan 23, 2025

Why templates would be empty? is this a real use case?

But there is also another issue - limactl create recommends to use:

INFO[0000] Run `limactl start my-vm` to start the instance.

Which is deprecated form:

$ limactl start my-vm
INFO[0000] Creating an instance "my-vm" from template://default (Not from template://my-vm)
WARN[0000] This form is deprecated. Use `limactl create --name=my-vm template://default` instead

Sounds like we don't have good test coverage for limactl start output. We use logging which is typically not tested, so we don't test interaction properly.

@jandubois
Copy link
Member Author

Why templates would be empty? is this a real use case?

Yes, for applications that embed Lima (like Rancher Desktop, or finch), and don't expose it directly to the user. They have no need to bundle generic templates because they will never be used.

FWIW, this is how I noticed this was broken.

But there is also another issue - limactl create recommends to use:

INFO[0000] Run `limactl start my-vm` to start the instance.

Which is deprecated form:

$ limactl start my-vm
INFO[0000] Creating an instance "my-vm" from template://default (Not from template://my-vm)
WARN[0000] This form is deprecated. Use `limactl create --name=my-vm template://default` instead

You are confusing create and start. limactl start my-vm is not deprecated to start an existing instance. It is actually the canonical way to do it. It is deprecated to use it for creating a new instance.

Sounds like we don't have good test coverage for limactl start output. We use logging which is typically not tested, so we don't test interaction properly.

That is correct. Not sure what the right solution would be.

@nirs
Copy link
Member

nirs commented Jan 23, 2025

Why templates would be empty? is this a real use case?

Yes, for applications that embed Lima (like Rancher Desktop, or finch), and don't expose it directly to the user. They have no need to bundle generic templates because they will never be used.

Good point.

But there is also another issue - limactl create recommends to use:

INFO[0000] Run `limactl start my-vm` to start the instance.

Which is deprecated form:

$ limactl start my-vm
INFO[0000] Creating an instance "my-vm" from template://default (Not from template://my-vm)
WARN[0000] This form is deprecated. Use `limactl create --name=my-vm template://default` instead

You are confusing create and start. limactl start my-vm is not deprecated to start an existing instance. It is actually the canonical way to do it. It is deprecated to use it for creating a new instance.

But why lima start complains about the canonical usage? The previous command was lima create. The instance was not created since the template was missing?

It sounds like we try to help by recommending a better command but the result is mess. Maybe it is better to simplify: lima create create an instance, lima start starts an instance of fail.

Sounds like we don't have good test coverage for limactl start output. We use logging which is typically not tested, so we don't test interaction properly.

That is correct. Not sure what the right solution would be.

In qemu-img some tests are using golden output files, including filtered output from the program. If the output changes, you need to update the output file to make the test pass. This avoids unintended output changes.

@jandubois
Copy link
Member Author

But why lima start complains about the canonical usage? The previous command was lima create. The instance was not created since the template was missing?

That is the bug that was introduced in #3120. I'm reverting it in #3144 and fix the original issue properly.

@jandubois
Copy link
Member Author

In qemu-img some tests are using golden output files, including filtered output from the program. If the output changes, you need to update the output file to make the test pass. This avoids unintended output changes.

It is harder for us because we have an interactive UI that shows a prompt and waits for user input. You can check the output for --tty=false, but then does not cover the UI part.

And #3120 was about fixing the problem that the UI was displayed before the error was reported. So if you skip the UI then the test wouldn't cover this scenario.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
regression Used to work but has been broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants