Fix start command: return error early when instance already exists

[jandubois]

Without the additional check, we would offer the user to edit the template, and only then throw an error that the instance already exists.

Fixes #2575
Fixes #3142
Reverts #3120

Intentionally not squashing commits because one is a revert, and the other is new changes.

[nirs]

I don't fully understand the complicated code around the change, but this looks a reasonable minimal fix.

We need a new test to cover this use case.

[nirs]

This is very confusing API - if the template does not exist I would expect limatmpl.Read() to return an error, not an object with empty Bytes.

[jandubois]

I agree, this was mostly to accommodate the complex logic in loadOrCreateInstance().

I don't think all the error checking and reporting belongs into limatmpl.Read(), which is just code factored out from loadOrCreateInstance(), so it can be called from other places as well. It would benefit from further refactoring, but that is not a priority right now. Please don't touch this code until #3072 is done and merged.

[nirs]

This looks good for a minimal change, but we need to eliminate the complicated nesting later. If we handled the case when Bytes > 0 by returning, we don't need the else block.

[jandubois]

We need a new test to cover this use case.

I don't know how that test should work, and I don't think it needs to be in this PR, which is just fixing a regression in the latest release.

[nirs]

Reproduced the original issue:

% rm -rf _output/share/lima/templates

% _output/bin/limactl create --tty=0 test.yaml
...
INFO[0019] Run `limactl start test` to start the instance. 

% _output/bin/limactl start test                  
INFO[0000] Creating an instance "test" from template://default (Not from template://test) 
WARN[0000] This form is deprecated. Use `limactl create --name=test template://default` instead 
FATA[0000] open /Users/nsoffer/src/lima/_output/share/lima/templates/default.yaml: no such file or directory 

Tested the fix:

% rm -rf _output/share/lima/templates         
% _output/bin/limactl create --tty=0 test.yaml 
...

% _output/bin/limactl start test              
INFO[0000] Using the existing instance "test"           
INFO[0000] Starting the instance "test" with VM driver "vz" 
INFO[0000] [hostagent] hostagent socket created at /Users/nsoffer/.lima/test/ha.sock 
INFO[0000] [hostagent] Starting VZ (hint: to watch the boot progress, see "/Users/nsoffer/.lima/test/serial*.log") 
INFO[0000] [hostagent] reloading dns nameservers to [192.168.50.1:53] 
INFO[0000] SSH Local Port: 63710                        
INFO[0000] [hostagent] [VZ] - vm state change: running  
INFO[0000] [hostagent] Running in plain mode. Mounts, port forwarding, containerd, etc. will be ignored. Guest agent will not be running. 
INFO[0000] [hostagent] Waiting for the essential requirement 1 of 1: "ssh" 
INFO[0010] [hostagent] Waiting for the essential requirement 1 of 1: "ssh" 
INFO[0011] [hostagent] The essential requirement 1 of 1 is satisfied 
INFO[0011] [hostagent] Waiting for the final requirement 1 of 1: "boot scripts must have finished" 
INFO[0011] [hostagent] The final requirement 1 of 1 is satisfied 
INFO[0011] READY. Run `ssh -F "/Users/nsoffer/.lima/test/ssh.config" lima-test` to open the shell.

[alexandear]

Maybe we need to log error if not nil?

[jandubois]

No, if createOnly is true, then we throw an error if the instance already exists. If the instance doesn't exist yet, then everything is fine.

Note that the only time store.Inspect() returns a non-nil error is when the instance doesn't exist. Any other errors would be returned in *inst.Errors. This is not always handled consistently on the caller side (see #2668).