feat(network): add support for forward guest localhost DNS for unikraft - #966
feat(network): add support for forward guest localhost DNS for unikraft#966alimx07 wants to merge 1 commit into
Conversation
✅ Deploy Preview for urunc canceled.
|
|
However, please complete the PR description correctly. |
|
Hello @alimx07 , let;s make this as a standalone PR and fix the hardcoded value of unikraft's DNS configuration. What we need is to simply read the |
8ea34bd to
95a1e35
Compare
Hi @cmainas , I tried to make it minimal, it does the required functionality but even that we still need the docker PR for docker case and TC rules. |
cmainas
left a comment
There was a problem hiding this comment.
Thank you @alimx07 ,
this PR fixes an existing bug for k8s deployments so it has priority over the docker case.
A few changes for the CI, so we can merge:
- sign-off and squash your commits
- add yourself in https://github.com/urunc-dev/urunc/blob/main/.github/contributors.yaml
- mark this as ready for review. You can can mark the sidecar test PR as draft
- the test will never succeed. I have created a new image
harbor.nbfc.io/nubificus/urunc/dns-test-qemu-unikraft-initrd:latest. The expected output should begithub.com OK. I have also added these comments in the respective lines.
| TestFunc: namespaceTest, | ||
| }, | ||
| { | ||
| Image: "ghcr.io/alimx07/dns-test-qemu-unikraft-initrd:latest", |
There was a problem hiding this comment.
Replace with harbor.nbfc.io/nubificus/urunc/dns-test-qemu-unikraft-initrd:latest
| StaticNet: false, | ||
| SideContainers: []string{}, | ||
| Skippable: true, | ||
| ExpectOut: "Address: 8.8.8.8", |
95a1e35 to
d097271
Compare
Unikraft guests used a hardcoded DNS server, so name resolution inside the unikernel ignored the DNS configuration of the container. Parse the nameserver entry from the container's resolv.conf and pass it to the guest through the kernel command line. Signed-off-by: Ali Mohamed <amx746@gmail.com>
d097271 to
73ae987
Compare
cmainas
left a comment
There was a problem hiding this comment.
Hello @alimx07 ,
thank you for the rebase, but the it does not build. Also, now we need to take into account libcontainer and move the DNS server discovery in the "urunc create" process, since "urunc monitor" in libcontainer does not have a view of the host rootfs.
| // virtual ethernet interface inside the namespace | ||
| netArgs.MAC = networkInfo.EthDevice.MAC | ||
| netArgs.MTU = networkInfo.EthDevice.MTU | ||
| netArgs.DNSServer = getDNSServer(mounts) |
There was a problem hiding this comment.
We need to move this to InitisalSetup so it can also take effect in libcontainer. The monitorSpec struct can be updated to store the value of DNS so it can be then read from the urunc reexec and urunc monitor processes.
|
Hi @cmainas, sorry, I have been a bit busy these days. If others are blocked, feel free to assign someone else to continue with this PR. Otherwise, I’ll look into it over the weekend. |
Description
This PR will add support for localhost semantics added in #793 for unikraft unikernels. This will be done by modifying the CMD line defined here:
as the new DNSServer will be either:
internal/constants/network_constants(e.g. 192.168.100.100)The exact of this happens is defined in #793 but as breif, we detect if we have localhost resolvers ,then rewrite them to point to our Custom resolver, where we apply
TC rulesto intercept traffic and redirect it according to our rulesRelated issues
How was this tested?
LLM usage
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).