search

smebberson / docker-alpine

Docker containers running Alpine Linux and s6 for process management. Solid, reliable containers.
MIT License
596 stars 186 forks source link

Have `go-dnsmasq` resolve `.consul` against `Consul`. #34

Closed matthewvalimaki closed 8 years ago

matthewvalimaki commented 8 years ago

Resolves #31. Signed-off-by: Matthew Valimaki matthew.valimaki@gmail.com

smebberson commented 8 years ago

@matthewvalimaki, I've just updated alpine-base. Are you able to rebase this from master and work in your changes again. Sorry about that.

matthewvalimaki commented 8 years ago

@smebberson done. I wonder why no conflicts for the base was shown. A side note: I would recommend we break ENV calls to separate lines like we do with RUN. That way it becomes more readable.

smebberson commented 8 years ago

@matthewvalimaki I went to resolve this and realised something. Consul has been configured to use $CONSUL_DOMAIN so that you could actually change the default Consul domain from consul to something. However, the Consul domain in go-dnsmasq was hard coded to --stubzones=.consul/127.0.0.1:8600. I'm pretty sure it should be referencing $CONSUL_DOMAIN.

Also, how are you testing this is working? I'm not sure how I would test, otherwise I would have updated alpine-consul-base/root/etc/services.d/resolver/run appropriately and push it through myself. Sorry about that.

smebberson commented 8 years ago

@matthewvalimaki would it make sense to remove CONSUL_DOMAIN env from alpine-base and instead put it in alpine-consul as nothing alpine-base references CONSUL_DOMAIN?

Also, should we be updating alpine-consul/root/etc/services.d/consul/run and adding alpine-consul/root/etc/services.d/resolver/run as per your edits to alpine-consul-base in this PR (rather than adding them in alpine-consul-base). That change will mean that everything that inherits from alpine-consul (which includes alpine-consul and alpine-consul-ui will inherit the $CONSUL_DOMAIN configuration (essentially the entire suite of docker-alpine Consul based images). Otherwise alpine-consul and alpine-consul-ui misses out on these improvements...

smebberson commented 8 years ago

@matthewvalimaki (PS. I have a bunch more updates based alpine-base waiting in the wings ready to go). I just wanted to push this one through first so that you don't have to keep remerging this PR.

matthewvalimaki commented 8 years ago

@smebberson you are correct. Stubzones should not be hard coded to .consul but to $CONSUL_DOMAIN. And yes, it would make sense to have this ENV in alpine-base as the domain would not resolve.

For alpine-consul the Consul change does not matter as it works fine without but I did not realize alpine-consul-ui was using it, so yes we should use alpine-consul instead of alpine-consul-base.

The way to test this is to change the domain for alpine-consul-nginx and then do ping against it: ping nginx.service.domain

smebberson commented 8 years ago

@matthewvalimaki, thanks. I'm working on this now.