lima icon indicating copy to clipboard operation
lima copied to clipboard

add validation for nestedVirtualization, rosetta, and mountType

Open olamilekan000 opened this issue 10 months ago • 6 comments

change validates config values for `nestedVirtualization`, `rosetta`, and `mountType` 
when a user starts or tries to create a VM.

Fixes https://github.com/lima-vm/lima/issues/3126

olamilekan000 avatar Jan 19 '25 04:01 olamilekan000

@AkihiroSuda kindly review. Also, I am not sure why unit tests are failing for windows, go version 1.22 and 1.23, even though everything looks fine.

olamilekan000 avatar Jan 19 '25 09:01 olamilekan000

https://github.com/lima-vm/lima/actions/runs/12853756300/job/35837296730?pr=3127

=== RUN   TestValidateRosetta
time="2025-01-19T12:59:05Z" level=debug msg="ResolveVMType: resolved VMType \"qemu\" (explicitly specified in []*LimaYAML{o,y,d}[1])"
    validate_test.go:202: assertion failed: expected an error, got nil
--- FAIL: TestValidateRosetta (0.00s)
=== RUN   TestValidateMountTypeOS
time="2025-01-19T12:59:05Z" level=debug msg="ResolveVMType: resolved VMType \"qemu\" (default for GOOS=\"linux\")"
time="2025-01-19T12:59:05Z" level=debug msg="ResolveVMType: resolved VMType \"qemu\" (default for GOOS=\"linux\")"
time="2025-01-19T12:59:05Z" level=warning msg="`mountType: virtiofs` on Linux is experimental"
    validate_test.go:272: assertion failed: expected an error, got nil
--- FAIL: TestValidateMountTypeOS (0.01s)

AkihiroSuda avatar Jan 20 '25 02:01 AkihiroSuda

I have tried to replicate it locally but it passes. Not sure what I'm missing

olamilekan000 avatar Jan 20 '25 03:01 olamilekan000

@AkihiroSuda Found and fixed the issue. Thanks

olamilekan000 avatar Jan 20 '25 05:01 olamilekan000

@olamilekan000 I don't know if all your pushes where necessary to deal with CI problems, but please be aware that each full CI run costs about $6 due to our use of large GitHub runners for macOS, and this PR had like 18 force-pushes[^1]. That's ok if the problem is only reproducible in CI, but otherwise please try to resolve problems locally first.

[^1]: I have not checked, maybe the macOS jobs where skipped due to earlier compile/lint failures, in which case the cost will have been minimal.

jandubois avatar Jan 20 '25 06:01 jandubois

@olamilekan000 I don't know if all your pushes where necessary to deal with CI problems, but please be aware that each full CI run costs about $6 due to our use of large GitHub runners for macOS, and this PR had like 18 force-pushes1. That's ok if the problem is only reproducible in CI, but otherwise please try to resolve problems locally first.

Footnotes

  1. I have not checked, maybe the macOS jobs where skipped due to earlier compile/lint failures, in which case the cost will have been minimal.

Thanks for bringing this to my attention. The reason for that many pushes was due to me trying to figure why the test passes locally but fails with the CI. Apologies for the inconvenience

olamilekan000 avatar Jan 20 '25 08:01 olamilekan000