lancachenet / lancache-dns

DNS Docker service for a lancache.
https://hub.docker.com/r/lancachenet/lancache-dns/
MIT License
282 stars 74 forks source link

Switch to dnsmasq #14

Closed kixelated closed 7 years ago

kixelated commented 7 years ago

Another large set of personal changes I've made. We're using steamcache for 20 gaming PCs here at Twitch and I kind of got carried away with changes. Let me know what you guys would like ported or reverted.

The biggest change is switching to dnsmasq instead of bind. This significantly reduces the amount of configuration required and makes it a lot more user-friendly. I removed the separate steamcache IP environment variables because I'm using a merged steamcache/generic setup but I can add them back if you like dnsmasq. The removal of overlay and using .dockerignore is personal preference so let me know what you think.

JasonRivers commented 7 years ago

This is quite a large PR, and might need cutting down so we can look in to it without causing errors. While it's nice to have "easier" configuration, it has come at the cost of flexibility. The current live image you can disable serving DNS for a given network, EG DISABLE_ORIGIN=true would disable origin caching. This is sometimes important in large networks or if one of the caches goes wrong/disk fills up/etc we can re-deploy the DNS server much faster than rebuilding it or modifying the config in the container (which one ill no doubt forget to take a copy of).

I have nothing against using dnsmasq over bind, but being able to disable a single network for caching I think is an important feature. It's also important to be able to run different networks on different cache machines, We commonly run the steamcache as well as a generic cache, but don't cache windows updates, for example.

I'll close the PR, but feel free to open a new PR keeping the features I have pointed out.

bntjah commented 7 years ago

Perhaps to fill in a bith further, wouldn't it be interesting to write a small Bash script that would check if the DISABLE_ORIGIN=true would then just rewrite the dnsmasq with everything but origin ? And if this is set on startup would just do the same?

Just thinking out loud here ofcourse...

JasonRivers commented 7 years ago

Sure, We do similar with the current bind config, the configs are all there but commented out in BIND, we uncomment them depending on the variables being parsed (IIRC default is to enable everything). I'll be happy to look at a PR using DNSMasq as long as we still have this functionality.

astrolox commented 7 years ago

Completely agree with Jason, backwards compatibility with existing features needs to be maintained. I just wanted to add that we totally appreciate the effort and we look forward to seeing more pull requests.

kixelated commented 7 years ago

Okay, I'll split the changes up and implement the missing features. I don't need those flags and wanted to gauge interest before reimplementing.

kixelated commented 7 years ago

Updated the code review in #16