-
Notifications
You must be signed in to change notification settings - Fork 94
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
Finch cannot resolve special IP host-gateway
#209
Comments
In Finch, to make the feature parity with Docker, the ideal user stories will be: As a Finch user, if I use
As a Finch user, if I use
|
The solution could be split to multiple stages: At the first stage, we can resolve Lima has a static host IP 192.168.5.2 that can be accessed in containers. The value can be retrieved from code. We can directly resolve host-gateway to it before passing to nerdctl. The value retrieved by Finch will change if Lima changes the value. E2e tests will also be added to validate the ip equals to the value and it can be used to access host. Making |
Signed-off-by: Ziwen Ning <ningziwe@amazon.com> Issue #, if available: runfinch/finch#209 *Description of changes:* Start a server in host and try to curl it from container with `--add-host test-host:host-gateway` *Testing done:* `make run` with the change runfinch/finch#216 - [ 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: Ziwen Ning <ningziwe@amazon.com>
Issue #, if available: #209 *Description of changes:* Support special IP `host-gateway` in `--add-host` flag. Finch will resolve to the IP that can be used to access host from containers. It is not configurable for now. See [issue](#209) for more details. *Testing done:* unit tests and e2e tests runfinch/common-tests#29 - [ 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: Ziwen Ning <ningziwe@amazon.com>
#209 *Description of changes:* parse --add-host special ip with equal sign, like `--add-host=name:host-gateway` *Testing done:* unit tests and e2e tests - [ 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: Ziwen Ning <ningziwe@amazon.com>
The first stage is already completed. The next thing to do it to make it configurable in Finch config for feature parity. It requires the nerdctl change to be promoted to Finch. |
Hi @ningziwen, I want to work on this issue. Can you guide me around it? |
Sure, thanks for the interest! Assigned the issue to you. Feel free to raise any questions directly in the issue. I will be actively monitoring it and helping you. |
Thank you very much, I am kind of clueless, so can you give me a rough idea on how I should tackle the issue? |
@TrungBui59 Yes. You may follow these steps to start working on this.
|
@ningziwen I see. Thank you |
@ningziwen Just to make sure my solution is acceptable. I will add a new field called host-gateway-ip on the finch.yaml. I will then parse by changing the pkg/config.go file and then get the data and pass to the SlirpGateway. Then Im gonna use it when user specify the ip |
My understanding is SlirpGateway IP is a constant value in Lima, which is not designed to be configurable. This nerdctl change introduced a host-gateway-ip field in nerdctl config, which is designed to be configurable. https://github.com/containerd/nerdctl/pull/1978/files So my suggestion is if host-gateway-ip exists in finch config, pass it to host-gateway-ip in nerdctl config. Otherwise, pass SlirpGatewayIP to host-gateway-ip in nerdctl config. Let me know if you have different thoughts. |
@ningziwen I have some issue with the tests. Unit tests keep giving me race condition on file I dont even touch. and e2e test is output something that I dont quite understand |
@TrungBui59 For unit tests, please ignore the race condition related log. The race condition is just a warning and won't cause it failing. If you scroll up you should be able to find the true error causing it failing. Scroll up to here:
If all of them are ? or ok, then you are good. There is an issue tracking the race condition warning. #447 |
@ningziwen so for the e2e test, what does that error mean? |
@TrungBui59 For e2e tests, most of the Finch e2e tests are in common-tests repo. And here is the failing test scenario from your log. https://github.com/runfinch/common-tests/blob/6c7275007bf34fb6ddecc4013c16f1d79ff6d1d0/tests/run.go#L335 The tests are failing because your change broke them. You also need to scroll up to find the failed scenario and get more useful logs from there. |
@ningziwen I have finished the task, but there is one question I want to make sure. If the user update the host_gateway_ip in the finch config file, do we only allow to update it when they init the VM or they can update when they start the VM? |
@TrungBui59 |
@ningziwen Got it. For the PR, it was an accident, I type a command and somehow it closed the PR |
@ningziwen sorry for not really finishing earlier, I was quite busy with my work. Is this issue still open? If it is, I will be able to finish this by the couple next days |
@TrungBui59 No worries. It is still open. Feel free to complete it. |
Retested this, seems this has been implemented:
|
Describe the bug
Finch cannot resolve special IP
host-gateway
infinch run --add-host name:host-gateway IMAGE
Steps to reproduce
Expected behavior
Screenshots or logs
Additional context
In Docker,
Can reproduce in nerdctl in Finch VM:
The text was updated successfully, but these errors were encountered: