Skip to content
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

fix: Refresh ~/.finch/confg.json based on finch.yaml #966

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

haytok
Copy link
Member

@haytok haytok commented Jun 5, 2024

Suppose we have configured the creds_helpers in ~/.finch/finch.yaml as follows, and subsequently initialized a VM (finch vm init).

cpus: 6
creds_helpers:
    - ecr-login
memory: 8GiB
vmType: vz
rosetta: true

As a result, ~/.finch/config.json is created, and it contains the following:

{"credsStore":"ecr-login"}

This allows us to utilize the Amazon ECR Docker Credential Helper within Finch.

Subsequently, suppose we stop and remove the VM
(finch vm stop && finch vm remove), and then remove the creds_helpers configuration from finch.yaml.

We then configure the finch.yaml file as follows:

cpus: 6
memory: 8GiB
vmType: vz
rosetta: true

As a result, when we reinitialize the VM (finch vm init), the expected behavior is that it will no longer use the Amazon ECR Docker Credential Helper.

However, when initializing the VM, despite the absence of creds_helpers configuration in finch.yaml, the "credsStore": "ecr-login" remains in config.json, allowing the continued use of the Amazon ECR Docker Credential Helper.

This behavior has been reported in the following issue:

Furthermore, this issue occurs when we stop the VM (finch vm stop), modify finch.yaml, and subsequently start the VM (finch vm start).

Consequently, we will modify the behavior to update config.json in accordance with the creds_helpers configuration in finch.yaml when initiating or starting the VM.

Issue #, if available: #480

Description of changes: The details are described in the commit message.

Testing done: yes

  • 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.

@haytok haytok self-assigned this Jun 5, 2024
@haytok
Copy link
Member Author

haytok commented Jun 5, 2024

The Unit Test is failing in the Windows environment.

 -test.shuffle 1717597156217190000
--- FAIL: Test_RefreshConfigFile (0.00s)
    --- FAIL: Test_RefreshConfigFile/should_return_an_error_if_finch.yaml_is_invalid (0.00s)
        cred_helper_test.go:166: 
            	Error Trace:	D:/a/finch/finch/pkg/dependency/credhelper/cred_helper_test.go:166
            	Error:      	Not equal: 
            	            	expected: true
            	            	actual  : false
            	Test:       	Test_RefreshConfigFile/should_return_an_error_if_finch.yaml_is_invalid
FAIL
FAIL	github.com/runfinch/finch/pkg/dependency/credhelper	0.024s

This is occurring because in the Windows environment, the validate() function is not accurately validating the contents of finch.yam.

As a result, we will modify the code to separate the Unit Test for macOS (darwin) and Windows environments.

@haytok
Copy link
Member Author

haytok commented Jun 6, 2024

The following CI is failing, and I'm currently investigating the cause of the failure as it remains unclear.

The details of the error are as follows:

Details

panic: test timed out after 2h0m0s
running tests:
	TestVM (2h0m0s)

goroutine 82 [running]:
testing.(*M).startAlarm.func1()
	C:/Program Files/Go/src/testing/testing.go:2366 +0x385
created by time.goFunc
	C:/Program Files/Go/src/time/sleep.go:177 +0x2d

goroutine 1 [chan receive, 119 minutes]:
testing.(*T).Run(0xc00017a9c0, {0xeba887?, 0xc03877a3c0?}, 0xf04c70)
	C:/Program Files/Go/src/testing/testing.go:1750 +0x3ab
testing.runTests.func1(0xc00017a9c0)
	C:/Program Files/Go/src/testing/testing.go:2161 +0x37
testing.tRunner(0xc00017a9c0, 0xc0000f1c70)
	C:/Program Files/Go/src/testing/testing.go:1689 +0xfb
testing.runTests(0xc000178228, {0x138eb40, 0x1, 0x1}, {0x1?, 0x9dd493?, 0x13c5680?})
	C:/Program Files/Go/src/testing/testing.go:2159 +0x445
testing.(*M).Run(0xc00011fe00)
	C:/Program Files/Go/src/testing/testing.go:2027 +0x68b
main.main()
	_testmain.go:47 +0x16c

goroutine 18 [runnable]:
github.com/onsi/ginkgo/v2/internal.extractRunningGoroutines()
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/progress_report.go:177 +0xef3
github.com/onsi/ginkgo/v2/internal.NewProgressReport(_, {{0xc0000be3c0, 0x2, 0x2}, {0xc00018c1c0, 0x2, 0x2}, {0xc0000305d0, 0x2, 0x2}, ...}, ...)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/progress_report.go:75 +0x2df
github.com/onsi/ginkgo/v2/internal.(*Suite).generateProgressReport(_, _)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:381 +0x59b
github.com/onsi/ginkgo/v2/internal.(*Suite).runNode(_, {0x16, 0x4, {0xedaf82, 0x33}, 0xc000113710, {{0x105e722, 0x42}, 0x19, {0x0, ...}, ...}, ...}, ...)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:959 +0x119f
github.com/onsi/ginkgo/v2/internal.(*group).attemptSpec(0xc00038f130, 0x1, {{0xc0002d1c08?, 0xc00018c1c0?, 0x2?}, 0x0?})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/group.go:199 +0xc33
github.com/onsi/ginkgo/v2/internal.(*group).run(0xc00038f130, {0xc0000be000?, 0x0?, 0x0?})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/group.go:349 +0x925
github.com/onsi/ginkgo/v2/internal.(*Suite).runSpecs(0xc00014ea88, {0xecd48a, 0x1f}, {0x144df20, 0x0, 0x0}, {0xc00016edb0, 0x2a}, 0x0, {0xc00013d688, ...})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:489 +0xac5
github.com/onsi/ginkgo/v2/internal.(*Suite).Run(_, {_, _}, {_, _, _}, {_, _}, _, {0xfaf790, ...}, ...)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:130 +0x405
github.com/onsi/ginkgo/v2.RunSpecs({0xfa78c0, 0xc00017ab60}, {0xecd48a, 0x1f}, {0x0, 0x0, 0x0})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/core_dsl.go:300 +0x8b3
github.com/runfinch/finch/e2e/vm.TestVM(0xc00017ab60)
	C:/actions-runner/_work/finch/finch/e2e/vm/vm_windows_test.go:55 +0x1ca
testing.tRunner(0xc00017ab60, 0xf04c70)
	C:/Program Files/Go/src/testing/testing.go:1689 +0xfb
created by testing.(*T).Run in goroutine 1
	C:/Program Files/Go/src/testing/testing.go:1742 +0x390

goroutine 19 [syscall, 119 minutes]:
os/signal.signal_recv()
	C:/Program Files/Go/src/runtime/sigqueue.go:152 +0x29
os/signal.loop()
	C:/Program Files/Go/src/os/signal/signal_unix.go:23 +0x13
created by os/signal.Notify.func1.1 in goroutine 18
	C:/Program Files/Go/src/os/signal/signal.go:151 +0x1f

goroutine 20 [select, 119 minutes]:
github.com/onsi/ginkgo/v2/internal/interrupt_handler.(*InterruptHandler).registerForInterrupts.func2(0x0?)
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/interrupt_handler/interrupt_handler.go:131 +0x7d
created by github.com/onsi/ginkgo/v2/internal/interrupt_handler.(*InterruptHandler).registerForInterrupts in goroutine 18
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/interrupt_handler/interrupt_handler.go:128 +0x165

goroutine 21 [select, 119 minutes]:
github.com/onsi/ginkgo/v2/internal.RegisterForProgressSignal.func1()
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/progress_report.go:32 +0x8b
created by github.com/onsi/ginkgo/v2/internal.RegisterForProgressSignal in goroutine 18
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/progress_report.go:30 +0xd9

goroutine 26 [syscall, 119 minutes, locked to thread]:
syscall.SyscallN(0x7ffa06a23bf0?, {0xc0004b1a48?, 0x3?, 0x0?})
	C:/Program Files/Go/src/runtime/syscall_windows.go:544 +0x107
syscall.Syscall(0x3?, 0x3?, 0xc00049e900?, 0x0?, 0x0?)
	C:/Program Files/Go/src/runtime/syscall_windows.go:482 +0x35
syscall.WaitForSingleObject(0x30c, 0xffffffff)
	C:/Program Files/Go/src/syscall/zsyscall_windows.go:1142 +0x5d
os.(*Process).wait(0xc0003356b0)
	C:/Program Files/Go/src/os/exec_windows.go:18 +0x50
os.(*Process).Wait(...)
	C:/Program Files/Go/src/os/exec.go:134
os/exec.(*Cmd).Wait(0xc000160160)
	C:/Program Files/Go/src/os/exec/exec.go:897 +0x45
os/exec.(*Cmd).Run(0xc000160160)
	C:/Program Files/Go/src/os/exec/exec.go:607 +0x2d
github.com/runfinch/finch/e2e/vm.init.func5(0xc0002981b0)
	C:/actions-runner/_work/finch/finch/e2e/vm/vm_test.go:32 +0x2f9
github.com/runfinch/finch/e2e/vm.init.func7.1.1()
	C:/actions-runner/_work/finch/finch/e2e/vm/additional_disk_test.go:26 +0x3a
github.com/onsi/ginkgo/v2/internal.extractBodyFunction.func3({0x0?, 0x0?})
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/node.go:472 +0x13
github.com/onsi/ginkgo/v2/internal.(*Suite).runNode.func3()
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:894 +0x8d
created by github.com/onsi/ginkgo/v2/internal.(*Suite).runNode in goroutine 18
	C:/Users/Administrator/go/pkg/mod/github.com/onsi/ginkgo/[email protected]/internal/suite.go:881 +0xdb0
FAIL	github.com/runfinch/finch/e2e/vm	7200.074s
FAIL
make: *** [test-e2e-vm-serial] Error 1
Error: Process completed with exit code 1.

Additionally, the CI for recently created PR is also failing.

Consequently, it is reasonable to assume that the changes made in this commit are not the root cause of the failure.

Suppose we have configured the `creds_helpers` in `~/.finch/finch.yaml` as
follows, and subsequently initialized a VM (`finch vm init`).

```
cpus: 6
creds_helpers:
    - ecr-login
memory: 8GiB
vmType: vz
rosetta: true
```

As a result, `~/.finch/config.json` is created, and it contains the
following:

```
{"credsStore":"ecr-login"}
```

This allows us to utilize the Amazon ECR Docker Credential Helper within Finch.

Subsequently, suppose we stop and remove the VM
(`finch vm stop` && `finch vm remove`), and then remove the
`creds_helpers` configuration from `finch.yaml`.

We then configure the `finch.yaml` file as follows:

```
cpus: 6
memory: 8GiB
vmType: vz
rosetta: true
```

As a result, when we reinitialize the VM (`finch vm init`), the expected
behavior is that it will no longer use the Amazon ECR Docker Credential Helper.

However, when initializing the VM, despite the absence of `creds_helpers`
configuration in `finch.yaml`, the `"credsStore": "ecr-login"` remains in
`config.json`, allowing the continued use of the Amazon ECR Docker
Credential Helper.

This behavior has been reported in the following issue:

- runfinch#480

Furthermore, this issue occurs when we stop the VM (`finch vm stop`),
modify `finch.yaml`, and subsequently start the VM (`finch vm start`).

Consequently, we will modify the behavior to update `config.json` in
accordance with the `creds_helpers` configuration in `finch.yaml` when
initiating or starting the VM.

Note that in this pull request, the commits are divided as follows:

- Implement the logic to update `config.json` according to `finch.yaml`
  and change the function name (loadFinchConfig) in the config package
- Modify to call the logic to update `config.json` according to
  `finch.yaml` on the VM side
- Add unit tests for the credhelper package and modify unit tests for the
  config package
- Modify and add Behavior-Driven Development (BDD) Tests using Ginkgo on
  the VM side
- Add e2e tests

On the other hand, in this commit, the logic to update `config.json`
according to `finch.yaml` is implemented and the function name
(loadFinchConfig) is changed in the config package.

Signed-off-by: Hayato Kiwata <[email protected]>
This commit fixes the logic to call the VM side to update config.json
according to ~/.finch/finch.yaml.

Signed-off-by: Hayato Kiwata <[email protected]>
…kage

In this commit, unit tests for credhelper package are added and unit tests
for config package are modified.

Signed-off-by: Hayato Kiwata <[email protected]>
In this commit, Behavior-Driven Development (BDD) tests using Ginkgo are
modified and added.

Signed-off-by: Hayato Kiwata <[email protected]>
In this commit, e2e tests for the implementation are added.

Signed-off-by: Hayato Kiwata <[email protected]>
@haytok haytok force-pushed the refresh-config.json-based-on-finch.yaml branch from 392950c to c231fd2 Compare June 8, 2024 12:57
@haytok
Copy link
Member Author

haytok commented Jun 11, 2024

The following error has occured.

Details

Finch Container Development E2E Tests Run a container image when running a container with network related flags should add a custom host-to-IP mapping with --add-host flag with special IP
C:/Users/Administrator/go/pkg/mod/github.com/runfinch/[email protected]/tests/run.go:383
  time="2024-06-10T14:27:57Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl ps --all --quiet --no-trunc], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  6639458223931720f48de6b6afdc3b33dfb632bfc7fd55c56488d89b3fc6638d
  No containers to be removed
  time="2024-06-10T14:27:58Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl images --all --quiet], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  4fac7a8257b1
  No images to be removed
  time="2024-06-10T14:27:59Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl volume ls --quiet], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  fbd4a280b3ac12a87be6b4c9d4c1381023134854d65b67676f90f96c27a6a30f
  time="2024-06-10T14:27:59Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl volume prune --force --all], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  time="2024-06-10T14:28:00Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl network ls --format {{.Name}}], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  bridge
  host
  none
  No networks to be removed
  time="2024-06-10T14:28:06Z" level=debug msg="Resolving special IP \"host-gateway\" to \"172.20.32.1\" for host \"test-host\""
  time="2024-06-10T14:28:06Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl container run -d --name ctr-test --add-host test-host:172.20.32.1 localhost:54245/amazonlinux/amazonlinux:2 sleep infinity], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  2024-06-10 14:28:07.000679828 +0000 UTC finch /snapshot/prepare {"key":"extract-980825813-ceyw sha256:9e1c1e301fa54d41bb458ac8eb55c27666b79d815b1955e62d45edb27a814e3c","snapshotter":"overlayfs"}
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   downloading    |--------------------------------------|    0.0 B/1.4 KiB 
  elapsed: 0.1 s                                                                    total:  846.0  (8.1 KiB/s)                                       
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |+++-----------------------------------|  5.0 MiB/59.7 MiB 
  elapsed: 0.2 s                                                                    total:  5.0 Mi (24.8 MiB/s)                                      
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |+++++++++++++++++---------------------| 27.0 MiB/59.7 MiB 
  elapsed: 0.3 s                                                                    total:  27.0 M (89.3 MiB/s)                                      
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |++++++++++++++++++++++++++++++--------| 48.0 MiB/59.7 MiB 
  elapsed: 0.4 s                                                                    total:  48.0 M (119.6 MiB/s)                                     
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |++++++++++++++++++++++++++++++++++++++| 59.7 MiB/59.7 MiB 
  elapsed: 0.5 s                                                                    total:  59.7 M (119.2 MiB/s)                                     
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    downloading    |++++++++++++++++++++++++++++++++++++++| 59.7 MiB/59.7 MiB 
  elapsed: 0.6 s                                                                    total:  59.7 M (99.3 MiB/s)                                      
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  index-sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88:    done           |++++++++++++++++++++++++++++++++++++++| 
  manifest-sha256:d50f1e912a2da0036b3e2303c2dea1788579e41cdfb0643e94ea354e92bdf132: done           |++++++++++++++++++++++++++++++++++++++| 
  config-sha256:24c69f972b4922d2e8c98dd360dc8717646a27703f7fa80f8ca820937fd0bd48:   done           |++++++++++++++++++++++++++++++++++++++| 
  layer-sha256:8a0995d2bf38a3ce742d376865782dab667a8fd7d3ded687b5775794788440be:    done           |++++++++++++++++++++++++++++++++++++++| 
  elapsed: 0.7 s                                                                    total:  59.7 M (85.2 MiB/s)                                      
  localhost:54245/amazonlinux/amazonlinux:2:                                        resolved       |++++++++++++++++++++++++++++++++++++++| 
  22bf06725070
  4fac7a8257b1
  time="2024-06-10T14:28:24Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl rmi --force 22bf06725070], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  2024-06-10 14:28:24.713404676 +0000 UTC finch /images/delete {"name":"localhost:54245/amazonlinux/amazonlinux:2"}
  2024-06-10 14:28:24.71898673 +0000 UTC finch /snapshot/remove {"key":"sha256:9e1c1e301fa54d41bb458ac8eb55c27666b79d815b1955e62d45edb27a814e3c","snapshotter":"overlayfs"}
  Untagged: localhost:54245/amazonlinux/amazonlinux:2@sha256:22bf06725070287a989d60f6ad5c036957515956c0408c4140839a18a632ce88
  Deleted: sha256:9e1c1e301fa54d41bb458ac8eb55c27666b79d815b1955e62d45edb27a814e3c
  time="2024-06-10T14:28:25Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl images --all --format {{.Repository}}:{{.Tag}}], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  public.ecr.aws/docker/library/registry:latest
  No images to be removed
  time="2024-06-10T14:28:25Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl volume ls --quiet], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  fbd4a280b3ac12a87be6b4c9d4c1381023134854d65b67676f90f96c27a6a30f
  time="2024-06-10T14:28:26Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl volume prune --force --all], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  time="2024-06-10T14:28:27Z" level=debug msg="Creating limactl command: ARGUMENTS: [shell --workdir /mnt/c/actions-runner/_work/finch/finch/e2e/container finch sudo -E nerdctl network ls --format {{.Name}}], LIMA_HOME: C:\\actions-runner\\_work\\finch\\finch\\_output\\lima\\data"
  bridge
  host
  none
  No networks to be removed
+ [FAILED] [30.300 seconds]
Finch Container Development E2E Tests Run a container image when running a container with network related flags [It] should add a custom host-to-IP mapping with --add-host flag with special IP
C:/Users/Administrator/go/pkg/mod/github.com/runfinch/[email protected]/tests/run.go:383

  [FAILED] Timed out after 10.008s.
  Expected process to exit.  It did not.
  In [It] at: C:/Users/Administrator/go/pkg/mod/github.com/runfinch/[email protected]/command/command.go:115 @ 06/10/24 14:28:20.263

I'm investigating the cause of the test failure.

@haytok haytok requested a review from a team June 20, 2024 11:10
@pendo324
Copy link
Member

The following error has occured.

* https://github.com/runfinch/finch/actions/runs/9428915702/job/26026006047?pr=966

Details

I'm investigating the cause of the test failure.

I think this is unrelated. Right now, the way we enable Windows => WSL2 communication is via a firewall rule. To make this more reliable, it might be best to install/configure mDNS inside the guest VM rootfs and resolve the special host-gateway IP via "$(hostname).local".

Another option is the new mirrored networking mode but that's not available on all versions of Windows which support WSL (and specifically not available on our test runners)

Copy link
Member

@pendo324 pendo324 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good generally, just left a few comments


// RefreshConfigFile refreshes config.json according to finch.yaml.
func RefreshConfigFile(fs afero.Fs, logger flog.Logger, finchConfigPath, configJSONPath string) error {
systemDeps := system.NewStdLib()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you pass these in as dependencies? Makes mocking/testing simpler

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree.
However, depending on the discussion below, there is no need to call config.LoadFinchConfig().

So the following call may no longer be necessary.

systemDeps := system.NewStdLib()
mem := fmemory.NewMemory()

systemDeps := system.NewStdLib()
mem := fmemory.NewMemory()

finchCfg, err := config.LoadFinchConfig(fs, finchConfigPath, logger, systemDeps, mem)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same for config, we can pass it as a dependency to the vm commands, and then to this function instead of loading it again

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pendo324

Does this mean that the fc being passed in the following is passed to newVirtualMachineCommand(), which in turn is passed to newStartVMCommand() and newInitVMCommand(), to avoid re-loading the config?

Comment on lines +25 to +33
var vmType string

ginkgo.BeforeEach(func() {
if runtime.GOOS == "windows" {
vmType = "wsl2"
} else {
vmType = "vz"
}
})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this actually necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it is necessary.

testFinchConfigFile() is called in both darwin and windows tests, but the vmType in finch.yaml is different for those environments.

The following implementation is also helpful.

@haytok
Copy link
Member Author

haytok commented Jun 27, 2024

Thanks you for review!!!

I'm checking ....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants