Repository navigation
feat: add DNS server to guest boot args - #1051
slash-init wants to merge 6 commits into
Conversation
✅ Deploy Preview for urunc ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
hey @cmainas , just following up on this PR since it's been a couple weeks. |
|
Hello @slash-init , please sign your commits. |
1cc92ea to
e585a8e
Compare
|
The commit is still unsigned. |
Signed-off-by: slash <amvermagaurav007@gmail.com>
e585a8e to
b97c928
Compare
|
@cmainas thanks for pointing that out |
cmainas
left a comment
There was a problem hiding this comment.
Thank you @slash-init for the updates. I have added some comments. Also, please do not forget to add yourself in https://github.com/urunc-dev/urunc/blob/main/.github/contributors.yaml
Signed-off-by: slash <amvermagaurav007@gmail.com>
Signed-off-by: slash <amvermagaurav007@gmail.com>
|
@cmainas thanks! |
cmainas
left a comment
There was a problem hiding this comment.
Hello @slash-init ,
I tested out the changes and unfortunately they do not work.
- For mirage the format is wrong. It should have a
udportcpprefix. See https://github.com/mirage/mirage/blob/dccd7db3c3358587f52350cd7f4d412f6cec82bf/lib/devices/dns.ml#L34-L41 - For mirage again the dns client must be present in the unikernel, otherwise there cli option can not be consumed. This is the reason that the tests fail.
- In case of hermit-rs, networking does not work at all. That is a separate bug.
So focusing on mirage, we need to use an annotation and let users set it at build time to see if the unikernel actually has a dns client.
Signed-off-by: slash <amvermagaurav007@gmail.com>
|
hi! @cmainas |
cmainas
left a comment
There was a problem hiding this comment.
Thank you @slash-init for the changes. They are in the right direction. I just added some comments. Overall:
- Just a naming comment
- we do not need to pass the new annotation value to the unikernel implementatios, we cna simply set the DNS value to empty in case the new annotation (renamed as :advertiseDNS") is false.
|
Also, a few more comments:
|
Signed-off-by: slash <amvermagaurav007@gmail.com>
|
hi! @cmainas, pushed the latest changes. take a look when you get a chance. |
cmainas
left a comment
There was a problem hiding this comment.
Hello @slash-init ,
there is a regression for Unikraft with the last commit. We should limit the check only for Mirage (for the time being, we need to update the images). Let;s also add a TODO comment for that.
Signed-off-by: slash <amvermagaurav007@gmail.com>
|
hi @cmainas, pushed the latest changes. Scoped the gate to Mirage only for the time being, added error handling for |
Description
This adds DNS server handling for guests that support configuring DNS through boot/runtime arguments.
The DNS server is read from the OCI
/etc/resolv.confmount and passed throughNetDevParamsto the guest-specific network configuration.Currently this adds DNS handling for:
Tests have also been added for the new DNS handling.
Rumprun is left out for now based on the discussion in #1039, since it does not appear to support configuring DNS through boot arguments.
Related issues
How was this tested?
/etc/resolv.conf->NetDevParams.DNSServerpath still needs end-to-end verification.LLM usage
GPT 5.6 Luna was used to assist with implementation. All generated code was reviewed.
Checklist
make lint).make test_ctr,make test_nerdctl,make test_docker,make test_crictl).