feat: windows persistent disk and other misc fixes/refactors #594

pendo324 · 2023-10-03T23:35:23Z

Issue #, if available:

Description of changes:

Adds persistent disk support to Windows
- This took more work than anticipated, in order to deal with non-Admin users
- pkg/disk/dpgo/ is brand new code, and should be a focus of the review
- pkg/winutil/run_windows.go is also new and requires careful review. This is what allows Finch to run as the regular user, except for when it needs Admin access to call diskpart (to create the persistent disk)
Fix paths in nerdctl_config_applier to make the post-boot/init shelling work
Added winres to allow the finch.exe to have metadata attached to it. This is WIP, need final icons and descriptions etc.
Large (in terms of lines changed) refactor of pkg/path/finch.go, but it should have no impact on functionality (needs careful review)
Fixed the Makefile's clean target for Windows
Most of the other changes are just noise from refactoring (like, literally renaming things). Nothing major, but take a look if possible. Sorry the diff is so large

Testing done:

I've reviewed the guidance in CONTRIBUTING.md

License Acceptance

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Signed-off-by: Vishwas Siravara <[email protected]>

Signed-off-by: Justin Alvarez <[email protected]>

Signed-off-by: Vishwas Siravara <[email protected]>

Signed-off-by: Justin Alvarez <[email protected]>

…n as admin Signed-off-by: Justin Alvarez <[email protected]>

Signed-off-by: Justin Alvarez <[email protected]>

Signed-off-by: Vishwas Siravara <[email protected]>

…n it Signed-off-by: Justin Alvarez <[email protected]>

Signed-off-by: Justin Alvarez <[email protected]>

pkg/disk/dpgo/disk_windows.go

ginglis13

just a few nits on first pass

ginglis13 · 2023-10-03T23:52:56Z

Makefile

+	cd deps/finch-core/src/lima/_output && tar -cf - * | tar -xvf - -C  $(OUTDIR)/lima
+	cd deps/finch-core/_output && tar -cf - * | tar -xvf - -C $(OUTDIR)


nit: repeated line here?

Suggested change

cd deps/finch-core/src/lima/_output && tar -cf - * | tar -xvf - -C $(OUTDIR)/lima

cd deps/finch-core/_output && tar -cf - * | tar -xvf - -C $(OUTDIR)

cd deps/finch-core/src/lima/_output && tar -cf - * | tar -xvf - -C $(OUTDIR)/lima

cmd/finch/virtual_machine_init.go

pkg/config/lima_config_applier.go

pkg/disk/dpgo/disk_windows.go

pkg/disk/disk_windows.go

vsiravar · 2023-10-04T01:06:58Z

Really cool PR, I left a few minor comments from the first pass, especially some files missing licenses.

Signed-off-by: Justin Alvarez <[email protected]>

pkg/disk/disk_windows.go

Signed-off-by: Justin Alvarez <[email protected]>

finch.yaml

pkg/disk/disk_windows.go

Signed-off-by: Justin Alvarez <[email protected]>

pkg/winutil/run_windows.go

pkg/disk/disk_unix.go

pkg/winutil/io.go

vsiravar

looks great. Left a couple of nits.

Signed-off-by: Justin Alvarez <[email protected]>

Issue #, if available: *Description of changes:* - Adds persistent disk support to Windows - This took more work than anticipated, in order to deal with non-Admin users - `pkg/disk/dpgo/` is brand new code, and should be a focus of the review - `pkg/winutil/run_windows.go` is also new and requires careful review. This is what allows Finch to run as the regular user, except for when it needs Admin access to call `diskpart` (to create the persistent disk) - Fix paths in `nerdctl_config_applier` to make the post-boot/init shelling work - Added winres to allow the finch.exe to have metadata attached to it. This is WIP, need final icons and descriptions etc. - Large (in terms of lines changed) refactor of `pkg/path/finch.go`, but it should have no impact on functionality (needs careful review) - Fixed the Makefile's `clean` target for Windows - Most of the other changes are just noise from refactoring (like, literally renaming things). Nothing major, but take a look if possible. Sorry the diff is so large *Testing done:* - [x] I've reviewed the guidance in CONTRIBUTING.md By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: Vishwas Siravara <[email protected]> Signed-off-by: Justin Alvarez <[email protected]> Signed-off-by: Vishwas Siravara <[email protected]> Co-authored-by: Vishwas Siravara <[email protected]> Co-authored-by: Vishwas Siravara <[email protected]>

Issue #, if available: *Description of changes:* - Adds persistent disk support to Windows - This took more work than anticipated, in order to deal with non-Admin users - `pkg/disk/dpgo/` is brand new code, and should be a focus of the review - `pkg/winutil/run_windows.go` is also new and requires careful review. This is what allows Finch to run as the regular user, except for when it needs Admin access to call `diskpart` (to create the persistent disk) - Fix paths in `nerdctl_config_applier` to make the post-boot/init shelling work - Added winres to allow the finch.exe to have metadata attached to it. This is WIP, need final icons and descriptions etc. - Large (in terms of lines changed) refactor of `pkg/path/finch.go`, but it should have no impact on functionality (needs careful review) - Fixed the Makefile's `clean` target for Windows - Most of the other changes are just noise from refactoring (like, literally renaming things). Nothing major, but take a look if possible. Sorry the diff is so large *Testing done:* - [x] I've reviewed the guidance in CONTRIBUTING.md By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: Vishwas Siravara <[email protected]> Signed-off-by: Justin Alvarez <[email protected]> Signed-off-by: Vishwas Siravara <[email protected]> Co-authored-by: Vishwas Siravara <[email protected]> Co-authored-by: Vishwas Siravara <[email protected]> Signed-off-by: Vishwas Siravara <[email protected]>

Issue #, if available: *Description of changes:* - translation logic to wsl paths - persistent disk for windows - CI/CD (workflows to run CI on every PR on windows runners, MSI builder, Windows release automation) This PR combines 4 distinct PRs to a separate windev branch. - additional disk for windows #594 - translation logic for wsl paths #581 - CI #581 - Installer #624 This PR also contains bug fixes and modifications to e2e tests. *Testing done:* Yes - [X] I've reviewed the guidance in CONTRIBUTING.md #### License Acceptance By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license. --------- Signed-off-by: Vishwas Siravara <[email protected]> Signed-off-by: Vishwas Siravara <[email protected]> Signed-off-by: Gavin Inglis <[email protected]> Signed-off-by: Justin Alvarez <[email protected]> Signed-off-by: chaoningusc <[email protected]> Signed-off-by: [email protected] <[email protected]> Signed-off-by: Kevin Li <[email protected]> Co-authored-by: Vishwas Siravara <[email protected]> Co-authored-by: Gavin Inglis <[email protected]> Co-authored-by: Justin <[email protected]> Co-authored-by: Kevin Li <[email protected]> Co-authored-by: chaoningusc <[email protected]> Co-authored-by: Justin Alvarez <[email protected]>

pendo324 requested review from ginglis13 and vsiravar October 3, 2023 23:35

pendo324 self-assigned this Oct 3, 2023

vsiravar and others added 7 commits October 3, 2023 19:44

Windows support

9f8c505

Signed-off-by: Vishwas Siravara <[email protected]>

Add path conversions to nerdctl commands and vm commands

2d44ce4

Signed-off-by: Vishwas Siravara <[email protected]>

add persistent disk support

ff670d2

Signed-off-by: Justin Alvarez <[email protected]>

move all platform specific code out of the static yaml

f7020d1

Signed-off-by: Justin Alvarez <[email protected]>

fix persistent disk and nerdctl config applier

f21c33f

Signed-off-by: Justin Alvarez <[email protected]>

actually mount persistent disk (inverted logic)

19b31f8

Signed-off-by: Justin Alvarez <[email protected]>

fix vm start

ba99272

Signed-off-by: Justin Alvarez <[email protected]>

pendo324 force-pushed the pendo/windev branch from cf14576 to bf1238f Compare October 3, 2023 23:47

Vishwas Siravara and others added 19 commits October 3, 2023 19:48

Update rootfs archive

724ec15

Signed-off-by: Vishwas Siravara <[email protected]>

Fix some path and linting issues

103fadf

Signed-off-by: Justin Alvarez <[email protected]>

Update rootfs and finch-core

2697e94

Signed-off-by: Justin Alvarez <[email protected]>

Add winres

129e3a3

Signed-off-by: Justin Alvarez <[email protected]>

Create new PowerShell Admin window to prevent needing finch.exe to ru…

63c4f87

…n as admin Signed-off-by: Justin Alvarez <[email protected]>

add dpgo command to make creating vdisk easier

cb39a5a

Signed-off-by: Justin Alvarez <[email protected]>

add dpgo to Makefile

b903abf

Signed-off-by: Justin Alvarez <[email protected]>

add template file

17387f3

Signed-off-by: Justin Alvarez <[email protected]>

fix bugs

b537486

Signed-off-by: Justin Alvarez <[email protected]>

add and use RunElevated to start dpgo.exe

42b0a52

Signed-off-by: Justin Alvarez <[email protected]>

add winres to tools and fix makefile

858321b

Signed-off-by: Justin Alvarez <[email protected]>

Add ShellExecuteAndWait

653b260

Signed-off-by: Justin Alvarez <[email protected]>

cleanup and update submodule

7cf3daf

Signed-off-by: Justin Alvarez <[email protected]>

Fix xmain unit test

290b612

Signed-off-by: Vishwas Siravara <[email protected]>

fix all unit tests

67eb2ff

Signed-off-by: Vishwas Siravara <[email protected]>

fix path escaping issues when running from a directory with a space i…

41a5835

…n it Signed-off-by: Justin Alvarez <[email protected]>

properly handle LIMA_CIDATA_HOME and update error messages

ec097ae

Signed-off-by: Justin Alvarez <[email protected]>

fix nca tests

ca918d9

Signed-off-by: Justin Alvarez <[email protected]>

rename dpgo and refactor UTF16 to winutil pkg

07c2d17

Signed-off-by: Justin Alvarez <[email protected]>

pendo324 added 2 commits October 3, 2023 19:48

Add winutil io tests

c341a95

Signed-off-by: Justin Alvarez <[email protected]>

fix linting issues

62ea1d2

Signed-off-by: Justin Alvarez <[email protected]>

pendo324 force-pushed the pendo/windev branch from bf1238f to 62ea1d2 Compare October 3, 2023 23:49

vsiravar reviewed Oct 3, 2023

View reviewed changes

pkg/disk/dpgo/disk_windows.go Show resolved Hide resolved

ginglis13 reviewed Oct 4, 2023

View reviewed changes