Allow configuration of serviceip used during consul registration

[r0ps3c]

Hi,

currently, serviceRegistration is called with an addr equal to ui.addr, which defaults to ":9998". When not configured (i.e. as per default), the ipaddr part is filled in the the local ip (from config.LocalIP()), which, when running inside a container, means that fabio registers the container's IP in consul, which seems incorrect/unnecessary, as other hosts would not be able to get to that address.

The initial potential workaround seemed to me to change ui.addr to the externally-accessible IP, but that doesn't work as fabio attempts to bind to that IP, which is non-local within the container, and everything falls over (note I am trying to avoid running the container with net=host)

The best option seems to be to introduce a variable such as consul.register.ip (the partner to consul.register.name) and allow the registered/serviceip to be overridden that way. An alternative may also be to set this to the same value as proxy.localip.

As I need to get to a working configuration in this situation, I'm going to go ahead and take a stab at implementation if needed, so just checking what makes more sense, or if I'm missing something obvious.

[magiconair]

Good point. I am refactoring the backend registry configuration for issue #12 anyway right now and I'll add that. I'll see that this gets merged quickly.

[magiconair]

Accidentally amended the commit for issue #12 instead of creating a separate one. You'd have to build a docker image yourself for now with this change until I've released 1.0.9. I was planning to do this next week on 17 Feb 2016 when Go 1.6 comes out. Let me know if that works for you.

[magiconair]

I've pushed magiconair/fabio:1.0.9-pre to docker hub so that you can test it. It is master+issues #12 and this one. Please note that I did not test this except for go test ./... Let me know whether that works for you.

[r0ps3c]

Thanks, will check it out

[r0ps3c]

Now hitting an issue I'd noticed previously (but not yet reported :-): in serviceRegistration, unless config.LocalIP() can't find a local address, the addr supplied to it is always ignored. I'd say move the

givenip := net.ParseIP(ipstr)
                if givenip == nil {

check earlier and only call LocalIP() if the given addr doesn't have a hostip part

[r0ps3c]

Please see #49 for fix

[magiconair]

I've pushed a 1.0.9-pre2 docker image from master which contains all changes including your PR #49. Would be awesome if you could test it.

[r0ps3c]

Thanks, should be able to do that in the next couple of days, will get back to you with results soon after

[r0ps3c]

Hi @magiconair,

in testing, the docker image seems to be working well for me, so I'd say this issue can be considered resolved.

Thanks

[magiconair]

Cool and thank you very much for testing. Then I'm going to wait for the Go 1.6 release and push 1.0.9 with these changes out.

[r0ps3c]

Hi,

have these changes or the issue 12 branch been merged into the 1.0.9 release? I see the docker image with these has been removed so wondering what the plan is if they haven't been merged

[magiconair]

No, they haven't since I needed a point release for the configuration of the read and write timeout based on stock 1.0.8. master was too far ahead. The changes are in 1.1rc1 (incl. the Docker image) since I've decided to push the version number and started thinking a bit more about RC and pre releases.

Since Go 1.6 got released yesterday I'm going to provide a 1.1-1.5.3 and 1.1-1.6 build based on master. Should be live soon.

[r0ps3c]

Thanks for the clarification, will check out the new images