Skip to content
This repository has been archived by the owner on Sep 26, 2021. It is now read-only.

Add the --virtualbox-host-dns-resolver flag #2448

Merged
merged 1 commit into from Dec 2, 2015

Conversation

grampajoe
Copy link
Contributor

This adds a new flag, --virtualbox-host-dns-resolver, that tells VirtualBox to start the VM with --natdnshostresolver1 on instead of off.

A common problem I've run into is getting DNS to work correctly in VirtualBox VMs when I'm connected to a VPN that does special magic to the host's DNS resolver. This makes it easier to set up a Docker host with working DNS without having to shut it down and run the VBoxManage command manually.

The integration test for this isn't great, because I couldn't find a way to inspect the natdnshostresolverX property. I'd love to hear any suggestions for how to properly test this! 馃寛

Signed-off-by: Joe Friedl joe@joefriedl.net

@jeanlaurent jeanlaurent self-assigned this Nov 30, 2015
@jeanlaurent jeanlaurent modified the milestone: 0.5.3 Nov 30, 2015
@jeanlaurent
Copy link
Member

This is working.

I believe the integration test does not bring anything new to the table.
I just spent a good deal of time trying to figure out how we can check that this flag is actually correctly set in the box, but the only place where I found it is in the log file.

Check https://github.com/jeanlaurent/machine/blob/b7b767aafed0ada5b2470e664a88a00ea9c22d30/test/integration/virtualbox/dns.bats that demonstrates how to perform a test while parsing the vbox.log, for your usecase here you should be grepping something like cat VBox.log | grep UseHostResolver | grep -q '(1)'

I suggest you remove the not-that-useful integration test. If you really feel like it, you can use the code above to hack your way into a new integration test.

Either way I tested this works, and I'm ready to move forward with a merge once you took a decision on integration tests.

@nathanleclaire
Copy link
Contributor

My feeling is also that the integration test doesn't really add anything and we'll just have to wait until better unit testing is available for VirtualBox driver to test this option.

So, when the integration test is removed, I'm game to merge this.

Signed-off-by: Joe Friedl <joe@joefriedl.net>
@grampajoe
Copy link
Contributor Author

Thanks for the review! I removed the integration test and added a note about the boolean flag in the docs.

@nathanleclaire
Copy link
Contributor

LGTM

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

Successfully merging this pull request may close these issues.

None yet

4 participants