fix: add finch vm settings subcommand

haytok commented 3 months ago

The current implementation requires updating ~/.finch/finch.yaml to change the CPU and Memory settings in the VM.

However, the following issue provides a feature request to set the CPU and memory resources to be allocated to the VM from CLI options.

https://github.com/runfinch/finch/issues/683

Therefore, this fix adds the finch vm settings command to add the ability to set the CPU and memory resources allocated to VMs from the CLI.

Issue #, if available: #683

Description of changes: Details are described in this commit message.

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.

Shubhranshu153 commented 2 months ago

@haytok some errors in the unit test and you might need to sign the commit. Thanks

haytok commented 2 months ago

We are looking into resolving the following error.

import cycle not allowed in test

Errors

```bash haytok ~/workspace/haytok/finch [add-settings-subcommand] > make test-unit go test github.com/runfinch/finch/benchmark github.com/runfinch/finch/benchmark/all github.com/runfinch/finch/benchmark/container github.com/runfinch/finch/benchmark/vm github.com/runfinch/finch/cmd/finch github.com/runfinch/finch/pkg/command github.com/runfinch/finch/pkg/config github.com/runfinch/finch/pkg/dependency github.com/runfinch/finch/pkg/dependency/credhelper github.com/runfinch/finch/pkg/dependency/vmnet github.com/runfinch/finch/pkg/disk github.com/runfinch/finch/pkg/flog github.com/runfinch/finch/pkg/fmemory github.com/runfinch/finch/pkg/fssh github.com/runfinch/finch/pkg/lima github.com/runfinch/finch/pkg/lima/wrapper github.com/runfinch/finch/pkg/mocks github.com/runfinch/finch/pkg/path github.com/runfinch/finch/pkg/support github.com/runfinch/finch/pkg/system github.com/runfinch/finch/pkg/version github.com/runfinch/finch/pkg/winutil -shuffle on # github.com/runfinch/finch/pkg/config package github.com/runfinch/finch/pkg/config imports github.com/runfinch/finch/pkg/mocks imports github.com/runfinch/finch/pkg/config: import cycle not allowed in test FAIL github.com/runfinch/finch/pkg/config [setup failed] ? github.com/runfinch/finch/benchmark [no test files] ok github.com/runfinch/finch/benchmark/all 0.449s [no tests to run] ok github.com/runfinch/finch/benchmark/container 0.635s [no tests to run] ok github.com/runfinch/finch/benchmark/vm 0.274s [no tests to run] ok github.com/runfinch/finch/cmd/finch 0.740s ok github.com/runfinch/finch/pkg/command 0.572s ? github.com/runfinch/finch/pkg/flog [no test files] ? github.com/runfinch/finch/pkg/fmemory [no test files] ok github.com/runfinch/finch/pkg/dependency 0.616s ? github.com/runfinch/finch/pkg/lima/wrapper [no test files] ? github.com/runfinch/finch/pkg/mocks [no test files] ok github.com/runfinch/finch/pkg/dependency/credhelper 0.595s ? github.com/runfinch/finch/pkg/system [no test files] ? github.com/runfinch/finch/pkg/version [no test files] ok github.com/runfinch/finch/pkg/dependency/vmnet 0.573s ok github.com/runfinch/finch/pkg/disk 0.566s ok github.com/runfinch/finch/pkg/fssh 0.656s ok github.com/runfinch/finch/pkg/lima 0.817s ok github.com/runfinch/finch/pkg/path 0.882s ok github.com/runfinch/finch/pkg/support 1.982s ok github.com/runfinch/finch/pkg/winutil 1.068s FAIL make: *** [test-unit] Error 1 haytok ~/workspace/haytok/finch [add-settings-subcommand] > make check-licenses go mod download GOBIN=/Users/haytok/workspace/haytok/finch/tools_bin go install github.com/google/go-licenses /Users/haytok/workspace/haytok/finch/tools_bin/go-licenses check --ignore golang.org/x,github.com/runfinch/finch --allowed_licenses Apache-2.0,BSD-2-Clause,BSD-3-Clause,ISC,MIT --include_tests ./... W0412 23:48:14.745128 34321 library.go:141] "github.com/shirou/gopsutil/v3/disk" contains non-Go code that can't be inspected for further dependencies: /Users/haytok/go/pkg/mod/github.com/shirou/gopsutil/v3@v3.24.3/disk/iostat_darwin.c /Users/haytok/go/pkg/mod/github.com/shirou/gopsutil/v3@v3.24.3/disk/iostat_darwin.h F0412 23:48:14.745449 34321 main.go:75] errors for ["github.com/runfinch/finch/benchmark" "github.com/runfinch/finch/benchmark/all" "github.com/runfinch/finch/benchmark/container" "github.com/runfinch/finch/benchmark/vm" "github.com/runfinch/finch/pkg/flog" "github.com/runfinch/finch/pkg/system" "github.com/runfinch/finch/pkg/command" "github.com/runfinch/finch/pkg/fmemory" "github.com/runfinch/finch/pkg/fssh" "github.com/runfinch/finch/pkg/config" "github.com/runfinch/finch/pkg/dependency" "github.com/runfinch/finch/pkg/path" "github.com/runfinch/finch/pkg/dependency/credhelper" "github.com/runfinch/finch/pkg/dependency/vmnet" "github.com/runfinch/finch/pkg/disk" "github.com/runfinch/finch/pkg/lima" "github.com/runfinch/finch/pkg/lima/wrapper" "github.com/runfinch/finch/pkg/version" "github.com/runfinch/finch/pkg/support" "github.com/runfinch/finch/cmd/finch" "github.com/runfinch/finch/e2e" "github.com/runfinch/finch/e2e/container" "github.com/runfinch/finch/e2e/vm" "github.com/runfinch/finch/pkg/mocks" "github.com/runfinch/finch/pkg/winutil" "github.com/runfinch/finch/benchmark/all [github.com/runfinch/finch/benchmark/all.test]" "github.com/runfinch/finch/benchmark/all.test" "github.com/runfinch/finch/benchmark/container [github.com/runfinch/finch/benchmark/container.test]" "github.com/runfinch/finch/benchmark/container.test" "github.com/runfinch/finch/benchmark/vm [github.com/runfinch/finch/benchmark/vm.test]" "github.com/runfinch/finch/benchmark/vm.test" "github.com/runfinch/finch/cmd/finch [github.com/runfinch/finch/cmd/finch.test]" "github.com/runfinch/finch/cmd/finch.test" "github.com/runfinch/finch/e2e/container [github.com/runfinch/finch/e2e/container.test]" "github.com/runfinch/finch/e2e/container.test" "github.com/runfinch/finch/e2e/vm [github.com/runfinch/finch/e2e/vm.test]" "github.com/runfinch/finch/e2e/vm.test" "github.com/runfinch/finch/pkg/command [github.com/runfinch/finch/pkg/command.test]" "github.com/runfinch/finch/pkg/command_test [github.com/runfinch/finch/pkg/command.test]" "github.com/runfinch/finch/pkg/command.test" "github.com/runfinch/finch/pkg/config [github.com/runfinch/finch/pkg/config.test]" "github.com/runfinch/finch/pkg/config.test" "github.com/runfinch/finch/pkg/dependency [github.com/runfinch/finch/pkg/dependency.test]" "github.com/runfinch/finch/pkg/dependency.test" "github.com/runfinch/finch/pkg/dependency/credhelper [github.com/runfinch/finch/pkg/dependency/credhelper.test]" "github.com/runfinch/finch/pkg/dependency/credhelper.test" "github.com/runfinch/finch/pkg/dependency/vmnet [github.com/runfinch/finch/pkg/dependency/vmnet.test]" "github.com/runfinch/finch/pkg/dependency/vmnet.test" "github.com/runfinch/finch/pkg/disk [github.com/runfinch/finch/pkg/disk.test]" "github.com/runfinch/finch/pkg/disk.test" "github.com/runfinch/finch/pkg/fssh [github.com/runfinch/finch/pkg/fssh.test]" "github.com/runfinch/finch/pkg/fssh.test" "github.com/runfinch/finch/pkg/lima_test [github.com/runfinch/finch/pkg/lima.test]" "github.com/runfinch/finch/pkg/lima.test" "github.com/runfinch/finch/pkg/path [github.com/runfinch/finch/pkg/path.test]" "github.com/runfinch/finch/pkg/path.test" "github.com/runfinch/finch/pkg/support [github.com/runfinch/finch/pkg/support.test]" "github.com/runfinch/finch/pkg/support.test" "github.com/runfinch/finch/pkg/winutil [github.com/runfinch/finch/pkg/winutil.test]" "github.com/runfinch/finch/pkg/winutil.test"]: github.com/runfinch/finch/pkg/config: -: import cycle not allowed in test make: *** [check-licenses] Error 1 ```

Other errors have been fixed.

haytok commented 2 months ago

@Shubhranshu153 Refactored and fixed so that the test passes. Could you review it again?

haytok commented 2 months ago

The details of the macos-latest CI failure are as follows

Use "finch [command] --help" for more information about a command.
[71---](https://github.com/runfinch/finch/actions/runs/8689692438/job/23829416605?pr=887#step:5:72) FAIL: TestSettingsVMAction_run (0.00s)
[72](https://github.com/runfinch/finch/actions/runs/8689692438/job/23829416605?pr=887#step:5:73) --- FAIL: TestSettingsVMAction_run/should_update_vm_settings (0.00s)
[73](https://github.com/runfinch/finch/actions/runs/8689692438/job/23829416605?pr=887#step:5:74) validate_darwin.go:39: Unexpected call to *mocks.Logger.Infof([The specified number of CPUs (%d) is greater than CPUs available on this system (%d ), 74 which may lead to severe performance degradation.
[74](https://github.com/runfinch/finch/actions/runs/8689692438/job/23829416605?pr=887#step:5:75) which may lead to severe performance degradation 6 4]) at /Users/runner/work/finch/finch/pkg/config/validate_darwin.go:39 because: there are no expected calls of the method "Infof" for that receiver

Logger.Infof() was not called in the local environment, but this function is called due to the small number of CPUs allocated to the GitHub Actions job.

https://docs.github.com/en/actions/using-github-hosted-runners/about-github-hosted-runners/about-github-hosted-runners#standard-github-hosted-runners-for-public-repositories

Fix to use fewer CPUs in test cases.

haytok commented 2 months ago

The unit-tests (windows-latest) is different on Windows than on macOS environments, where validation() is not implemented.

https://github.com/runfinch/finch/blob/main/pkg/config/validate_windows.go

This was causing errors in CI. I implemented the tests by separating the test cases for macOS and Windows to pass CI.

haytok commented 2 months ago

Fix tests so that CI passes. Could you please run CI again and review ?

haytok commented 2 months ago

the same error as below has occured.

https://github.com/runfinch/finch/pull/887#issuecomment-2057114167

Therefore, correct the test cpus.

haytok commented 2 months ago

The fix was completed so that the test passes.

I ran the tests again with the cpus set to the cpus (3) used in the GitHub Actions job at the following location in the local environment.

https://github.com/runfinch/finch/blob/main/pkg/config/validate_darwin.go#L37

haytok ~/workspace/haytok/finch [add-settings-subcommand]
> git diff pkg/config/validate_darwin.go
diff --git a/pkg/config/validate_darwin.go b/pkg/config/validate_darwin.go
index 141fedf..a7a54bf 100644
--- a/pkg/config/validate_darwin.go
+++ b/pkg/config/validate_darwin.go
@@ -35,6 +35,7 @@ func validate(cfg *Finch, log flog.Logger, systemDeps LoadSystemDeps, mem fmemor
        }

        totalCPUs := systemDeps.NumCPU()
+       totalCPUs = 3
        if *cfg.CPUs > totalCPUs {
                log.Infof(
                        "The specified number of CPUs (%d) is greater than CPUs available on this system (%d),\n"+

Check test and lint

``` haytok ~/workspace/haytok/finch/cmd/finch [add-settings-subcommand] > go test -run TestNewSettingsMCommand PASS ok github.com/runfinch/finch/cmd/finch 0.150s haytok ~/workspace/haytok/finch/cmd/finch [add-settings-subcommand] > go test -run TestSettingsVMAction_runAdapter PASS ok github.com/runfinch/finch/cmd/finch 0.151s haytok ~/workspace/haytok/finch/cmd/finch [add-settings-subcommand] > go test -run TestSettingsVMAction_run PASS ok github.com/runfinch/finch/cmd/finch 0.154s haytok ~/workspace/haytok/finch/cmd/finch [add-settings-subcommand] > cd ../../pkg/config/ haytok ~/workspace/haytok/finch/pkg/config [add-settings-subcommand] > go test -run Test_ModifyFinchConfig PASS ok github.com/runfinch/finch/pkg/config 0.120s haytok ~/workspace/haytok/finch/pkg/config [add-settings-subcommand] > go test -run Test_loadFinchConfig PASS ok github.com/runfinch/finch/pkg/config 0.119s ``` ``` haytok ~/workspace/haytok/finch [add-settings-subcommand] > make lint env GOOS=windows /Users/haytok/go/bin/golangci-lint run env GOOS=darwin /Users/haytok/go/bin/golangci-lint run ```

haytok commented 2 months ago

The following error occurred in windows-e2e-tests (self-hosted, windows, amd64, test, test-e2e-container).

Erros in the workflow

``` ------------------------------ Finch Container Development E2E Tests inspect a container should show the information of a container with --type=container flag C:/Users/Administrator/go/pkg/mod/github.com/runfinch/common-tests@v0.7.21/tests/inspect.go:46 3efcb40acce727ddba120f23d0f642421f743bca35f39e6625e436deae108c13 No containers to be removed fb9c9aef62af No images to be removed f4d57724a12f864ab57bcd06ecb5c5a4bc6544704b5c2285893bfe8e7605616a bridge host none No networks to be removed 2024-04-18 02:42:16.483567601 +0000 UTC finch /images/create {"name":"localhost:49273/docker/library/alpine:latest"} localhost:49273/docker/library/alpine:latest: resolved |++++++++++++++++++++++++++++++++++++++| index-sha256:dc9e1ce1980c9c1208022dc39d40e3724a5dff1a4387534ddef3e5ec8a7fe42e: done |++++++++++++++++++++++++++++++++++++++| manifest-sha256:6457d53fb065d6f250e1504b9bc42d5b6c65941d57532c072d929dd0628977d0: done |++++++++++++++++++++++++++++++++++++++| config-sha256:05455a08881ea9cf0e752bc48e61bbd71a34c029bb13df01e40e3e70e0d007bd: done |++++++++++++++++++++++++++++++++++++++| elapsed: 0.1 s total: 0.0 B (0.0 B/s) 2024-04-18 02:42:16.599484662 +0000 UTC finch /snapshot/prepare {"key":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","parent":"sha256:d4fc045c9e3a848011de66f34b81f052d4f2c15a17bb196d637e526349601820","snapshotter":"overlayfs"} 2024-04-18 02:42:16.612872681 +0000 UTC finch /containers/create {"id":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","image":"localhost:49273/docker/library/alpine:latest","runtime":{"name":"io.containerd.runc.v2","options":{"type_url":"containerd.runc.v1.Options"}}} 2024-04-18 02:42:16.667723037 +0000 UTC finch /snapshot/remove {"key":"/tmp/initialC3139841479","snapshotter":"overlayfs"} 2024-04-18 02:42:16.749663969 +0000 UTC finch /tasks/create {"container_id":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","bundle":"/run/containerd/io.containerd.runtime.v2.task/finch/8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","rootfs":[{"type":"overlay","source":"overlay","options":["index=off","workdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/192/work","upperdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/192/fs","lowerdir=/var/lib/containerd/io.containerd.snapshotter.v1.overlayfs/snapshots/20/fs"]}],"io":{"stdout":"/run/containerd/fifo/1165971291/8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1-stdout","stderr":"/run/containerd/fifo/1165971291/8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1-stderr"},"pid":95754} 2024-04-18 02:42:16.754736501 +0000 UTC finch /tasks/start {"container_id":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","pid":95754} 2024-04-18 02:42:16.756662516 +0000 UTC finch /tasks/exit {"container_id":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","id":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","pid":95754,"exited_at":{"seconds":1713408136,"nanos":756482205}} time="2024-04-18T02:42:17Z" level=warning msg="failed to inspect NetNS" error="failed to Statfs \"/proc/95754/ns/net\": no such file or directory" id=8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1 localhost:49273/docker/library/alpine:latest ctr-test time="2024-04-18T02:42:18Z" level=warning msg="failed to inspect NetNS" error="failed to Statfs \"/proc/95754/ns/net\": no such file or directory" id=8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1 3efcb40acce727ddba120f23d0f642421f743bca35f39e6625e436deae108c13 8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1 2024-04-18 02:42:20.095623863 +0000 UTC finch /tasks/delete {"container_id":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","pid":95754,"exited_at":{"seconds":1713408136,"nanos":756482205}} 2024-04-18 02:42:20.108607091 +0000 UTC finch /snapshot/remove {"key":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1","snapshotter":"overlayfs"} 2024-04-18 02:42:20.11408024 +0000 UTC finch /containers/delete {"id":"8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1"} 8fca1c568806a7e77bdb81356ec443d7b9fddd800e2d2aafba0105e3078ceaa1 time="2024-04-18T02:42:20Z" level=fatal msg="exit status 255" [FAILED] in [AfterEach] - C:/Users/Administrator/go/pkg/mod/github.com/runfinch/common-tests@v0.7.21/command/command.go:122 @ 04/18/24 02:42:20.824 + [FAILED] [9.439 seconds] Finch Container Development E2E Tests inspect a container [AfterEach] should show the information of a container with --type=container flag [AfterEach] C:/Users/Administrator/go/pkg/mod/github.com/runfinch/common-tests@v0.7.21/tests/inspect.go:20 [It] C:/Users/Administrator/go/pkg/mod/github.com/runfinch/common-tests@v0.7.21/tests/inspect.go:46 [FAILED] Expected : 1 to match exit code: : 0 In [AfterEach] at: C:/Users/Administrator/go/pkg/mod/github.com/runfinch/common-tests@v0.7.21/command/command.go:122 @ 04/18/24 02:42:20.824 ------------------------------ ```

The part of the test that failed is not related to this changes.

Also, this CI did not fail in the workflows we ran in the past.

Therefore, could you please run the CI again when you have time, to see if the test failure is reproducible? Note that the tests for the code added by this modification has passed.

haytok commented 2 months ago

Thanks for running the CI again! The workflow that was failed (windows-e2e-tests (self-hosted, windows, amd64, test, test-e2e-container)) has passed, so CI has passed !!!

Shubhranshu153 commented 2 months ago

Thank you for the contribution.

haytok commented 2 months ago

Thanks for review !!!!!

runfinch / finch

fix: add finch vm settings subcommand #887

License Acceptance