Sentinel: Fix initial Hello source address

[mattsta]

I previously added source binding to sentinelReconnectInstance(),
but I missed adding it to the Hello flow.

All tests pass.

Thanks to @dkong for tracking down the problem in
dkong@9b6eb0e

[antirez]

Good idea, but the implementation has some issues (memcpy of buffer with a fixed length, but the source pointer is not guaranteed to point to REDIS_IP_STR_LEN bytes). I'm rewriting it to merge today for 2.8.14.

[antirez]

No sorry I still don't fully understand the goal of this patch, because in theory, with anetSockName(), we are already extracting the address from the socket which should already be bound to the right address because of @mattsta previous patches.

Please could you specify what is going on here? Thanks. I tried to read the @dkong patch but apparently it tries to do a different thing (to fix the known issue of a public address that is not among the actual local addresses).

[antirez]

p.s. in the meantime I applied the "Abort Hello quicker if not connected …" commit at least.

[mattsta]

We have people doing this:

Datacenter A (or VM or container):
  External IP 1 -> NAT -> Redis Sentinel 1

Datacenter B:
  External IP 2 -> NAT -> Redis Sentinel 2

Attempts at automatic IP discovery will not be correct in this situation since Redis can never see the public IP. Redis then starts to gossip around an internal IP no other hosts can connect against.

We previously fixed this in reconnection logic, but not in the Hello section.

So...... this patch is in the /* Try to obtain our own IP address. */ section so the user can force the announce IP instead of broadcasting their internal IP (which is behind NAT an unreachable by any other host).

[antirez]

@mattsta the part I don't understand is how the user can bind an address which is not among the listed local addresses. In other terms, this is the proposed change:

+    if (REDIS_BIND_ADDR != NULL) {
+        memcpy(ip, REDIS_BIND_ADDR, REDIS_IP_STR_LEN);
+    } else {
+        if (anetSockName(ri->cc->c.fd,ip,sizeof(ip),NULL) == -1)
+            return REDIS_ERR;
+    }

at a first glance, if the bind address was configured, then, because of your past changes, anetSockName() will already return this bound address. Otherwise if the address provided by bind is not among the local addresses of the instance, the server will not start and exit with an error.

Basically the code above does not replicate (unless I'm missing something, which is probable) what the original user intended, that was the ability to announce an address which is a totally arbitrary address that the computer where Redis is running does not have any idea about.

[mattsta]

Two quick notes before longer message:

• this is a fix proposed by a user who discovered one instance announcing itself with two IPs (public and private) (https://groups.google.com/forum/#!msg/redis-db/PVVvjO4nMd0/l5zNQriFrOsJ).
• Their problem looks like, somehow, the internal IP gets used in addition to the public IP announcements in Hello messages? I'm not exactly sure how (haven't tried to reproduce it yet), but the most direct fix is to rely on the user-given IP and not system-reported IP in the Hello message.

I don't understand is how the user can bind an address which is not among the listed local addresses.

Linux has options allowing bind of IPs that don't exist on the machine (/proc/sys/net/ipv4/ip_nonlocal_bind, CAP_NET_ADMIN). People must manually enable that feature outside of Redis if they use this strange "Public IP proxies to Private IP" architecture.

Otherwise if the address provided by bind is not among the local addresses of the instance, the server will not start and exit with an error.

Correct, if the nonlocal_bind feature isn't enabled. Binding to non-existing IPs is typically used for HA situations where one server is standby for another. Then the standby software can be pre-running and bound to the failover IP even though it doesn't exist on the standby server until the failover happens.

if the bind address was proposed, then, because of your past changes, anetSockName() will already return this bound address.

Looking closer, that should be what is happening. Is it possible for a socket connected with an explicit outbound bind to report a getsockname() of something else? (again, haven't tested possibilities yet.)

At first, I thought Hello was happening before sentinelReconnectInstance(), but Reconnect obviously happens first to populate the ri->cc->c.fd (so Reconnect is just a regular Connect). There shouldn't be a way to enter sentinelSendHello() without the bind connection already happening... but then what caused the user to see one Sentinel announcing itself from alternating IP addresses?

[dkong]

Hi @mattsta @antirez . I can reproduce the issue consistently with my AWS EC2 Ubuntu instances. Let me know if there's anything I can do to help debug.

However, the proposed solution of using the bind address for sentinel to announcements still does not work in my case. Specifically, changing the sentinel server to bind to the public IP address prevents remote machines from successfully communicating with it (I believe it's related to the NAT from public to private).

Remote machines can connect when sentinel binds to everything:

root@redis-west:~$sudo lsof -n | grep LISTEN
redis-ser 16070     ubuntu    4u     IPv6             172762      0t0        TCP *:26379 (LISTEN)
redis-ser 16070     ubuntu    5u     IPv4             172763      0t0        TCP *:26379 (LISTEN)

root@redis-east:~$ nmap -P0 -p 26379 54.13.18.118

Starting Nmap 5.21 ( http://nmap.org ) at 2014-09-03 22:27 UTC
Nmap scan report for ec2-54-13-18-118.us-west-1.compute.amazonaws.com (54.13.18.118)
Host is up (0.079s latency).
PORT      STATE SERVICE
26379/tcp open  unknown

Nmap done: 1 IP address (1 host up) scanned in 0.11 seconds

Remote machines cannot connect when sentinel binds to its public IP:

root@redis-west:~$sudo lsof -n | grep LISTEN
redis-ser 15992     ubuntu    4u     IPv4              86191      0t0        TCP 54.13.18.118:26379 (LISTEN)

root@redis-east:~$ nmap -P0 -p 26379 54.13.18.118

Starting Nmap 5.21 ( http://nmap.org ) at 2014-09-03 22:28 UTC
Nmap scan report for ec2-54-13-18-118.us-west-1.compute.amazonaws.com (54.13.18.118)
Host is up (0.082s latency).
PORT      STATE  SERVICE
26379/tcp closed unknown

Nmap done: 1 IP address (1 host up) scanned in 0.11 seconds

In my patch dkong@9b6eb0e, I separated the two features into distinct config options:

• bind - the listening address for the server process
• announce - (new) the IP address annnounced to other sentinels which they use to connect

That approach worked for me.

Thanks.

[mattsta]

Remote machines cannot connect when sentinel binds to its public IP:

Yeah, looking at your setup again, specifying the external IP here won't work at all. EC2 is just doing a Public IP->Private IP NAT, so your system has to bind to the private interface. We saw something similar before at #1667, but they claim the bind fix worked for them.

So, with your setup, there's no way for Sentinel to advertise the public IP because Redis can't self-discover the public IP to announce it to other instances, so that's why you added the new config directive.

The only way Sentinel can announce an arbitrary IP is to add a new config directive like you did in your patch.

[antirez]

Thank you both for your detailed explanation, I was not aware of this Linux option that allowed to bind non-local addresses. Between this being Linux-only, and the complexity involved (apparently it does not always work), I think it is a better pick for us to have something like:

sentinel claim-ip 1.2.3.4

Where Sentinel will just pretend to be that address in the HELLO messages.
If this is enough to fix the problem I can write a quick patch and release it with 2.8.15.

I'm reviewing the patch originally provided by @dkong before to write some code, news ASAP.

[antirez]

Ok I'm using @dkong commit, and committing changes to this commit in order to modify a bit the code and the behavior. More info ASAP.

[antirez]

Final set of commits:

• 0a6cbab Sentinel: don't set announce-ip if is empty.
• c9437fe Sentinel: clarify announce-ip/port options in sentinel.conf.
• cd576a1 Sentinel: announce ip/port changes + rewrite.
• 3d93926 sentinel: Decouple bind address from address sent to other sentinels

The original patch was modified in the following ways:

1. Now we have two options, announce-ip and announce-port, it is possible to use just one. This way we are allowing any kind of NAT / proxy possible, even in the case the TCP port mapped changes.
2. The configuration rewrite of the two options is now handled.
3. The code no longer uses Sentinel addresses structures, this way it was simpler now that we have two options.

I tested the implementation manually for some time, but there is still no unit test for this feature since it is not trivial to implement and the feature is non critical compared to other tests still missing. Merging into 2.8 and 3.0 branch. Thank you for the help!

[antirez]

p.s. in order to merge to non-unstable branches I'll wait for your comments.

[dkong]

I confirmed sentinel now behaves as expected in my AWS multi-region test environment with the latest changes in unstable branch.

Thanks a bunch @antirez and @mattsta!

[joshula]

hi, when are you guys planning to push to stable? this is critical for Docker instances of sentinel (and hence my application). thanks for all your hard work (in general) on Redis... it's an awesome tool, and community... I look forward to contributing some day (when I'm a better coder).

[antirez]

@joshula it was already released with 2.8.15, however there is still the problem of the fact that if you spin your salves with Docker as well, they'll be listed with a wrong address in the Redis master INFO output, so Sentinel will still not work.

[joshula]

I am doing that, but it doesn't seem to be a problem for me. I have 1 master and 2 slaves, each running in a separate docker container, all on different hosts. When I REDIS-CLI into the master and run INFO or ROLE, I see the correct addresses for my 2 slaves (the IP address of the Hosts machines). Also, when I look at my sentinel logs, it knows about my slaves, and has them listed at their proper addresses. It's the other sentinel addresses (in the sentinel logs) that are incorrect (listing the IP of the docker VM instead of the host machine). I am running 2.8.15, and I've attempted to start them up with a bunch of different commands (to fix this):
redis-sentinel /sentinel.conf --announce
redis-sentinel /sentinel.conf --announce-ip
However, none of them work. When I look at a sentinel's logs - they still believe the other sentinels are at the wrong address. Am I implementing this fix incorrectly?

[joshula]

@anyone
Am I implementing the fix properly?

[mattsta]

The main issue here is people are starting to use Redis in environments where Redis wasn't designed to work.

A normal deployment has all services contained and reachable in the same IP space. But now, some wacky people want to run each Redis server in a little IP island with translation units sitting between every IP internal IP space.

So, instead of running: Gateway -> [All Servers Mutually Reachable], people are doing:

Container A -> [Reachable IP A]
    [Isolated IP 1]

Container B -> [Reachable IP B]
    [Isolated IP 2]

Container C -> [Reachable IP C]
    [Isolated IP 3]

Redis is designed to announce itself using the IP address it's using, but when [Isolated IP 1] gets announced outside of Container A, nothing can talk to the hidden IP.

Redis will need a more extensive review in how to properly define and announce IPs for Sentinel and Cluster use cases when the IP Redis is using isn't actually reachable by any other servers.

The easiest way to fix the current problem is to use interface bridging so in-container IPs are directly reachable from outside the container too.

[dkong]

I am running 2.8.15, and I've attempted to start them up with a bunch of different commands (to fix this):
redis-sentinel /sentinel.conf --announce
redis-sentinel /sentinel.conf --announce-ip

@joshula The fix was implemented as a config option. Did you modify the sentinel config appropriately?

https://github.com/antirez/redis/blob/2.8.15/sentinel.conf#L7-L8

# sentinel.conf
sentinel announce-ip <reachable IP that nodes use to connect to this sentinel instance>
sentinel announce-port <reachable port that nodes use to connect to this sentinel instance>

# And then start sentinel server on command line
redis-sentinel /path/to/sentinel.conf

[mattsta]

So, the new sentinel announce options let sentinel use specific announced addresses, but the individual Redis servers (and Sentinel) still don't know the Redis server reachable IP addresses.

One trick you can try: conrigure the bind for each in-container Redis server to have the public IP first then the internal isolated IP second. Then, Redis will announce itself using the reachable IP, but it will still listen and accept connections on the internal isolated IP.

We covered that a bit here: #1908 (comment) but I think I forgot to mention you should also list the internal IP after the reachable IP so Redis is still listening locally. (If that works, we don't actually need the sentinel announce options if your NAT layer port-forwards the same ports to the backend server.)

[joshula]

Thanks for helping. I really appreciate it.

@dkong
I'm using service discovery (via coreOS). I've got 2 separate images, 1 Redis, 1 Sentinel. I then deploy N Redis and N Sentinel containers to multiple hosts using systemd units files (which means config params need to be set at run-time because you don't know the IP ahead of time). This is usually fine because you can config Redis or Sentinel at run-time using the '--' command. For example: redis-sentinel /sentinel.conf --appendonly. So, modifying the sentinel.conf file ahead of time won't really work. Is my problem that I set 'announce-ip' instead of 'sentinel announce-ip'?

@mattsta
I'm binding my host IP/PORT to the (internal) Docker IP/PORT already -- using the '-p' command when I run my Docker Redis/Sentinal images. I'm actually having no problem whatsoever with my Redis master and slaves finding each other. Also, my Sentinels can find my Redis master and slaves just fine. All of that is working perfectly. The only issue is when the Sentinels attempt to gossip over PUB/SUB. Each Sentinel is looking for the other at their (internal) Docker IP/PORT.

[dkong]

@joshula My mistake. I didn't know about the "passing config arguments via command line" feature.

This command seems to work for me:

redis-sentinel sentinel.conf --"sentinel announce-ip" 1.2.3.4

[vmmello]

Similar problems with FTP and SIP behind NAT exposing internal address are solved in the kernel with iptables/nftables helper modules (e.g. FTP, SIP). So eventually there could be a nf_conntrack_redis.c module maintained separately, as it requires some work and effort to make redis and sentinel handle correctly this translation internally.

[joshula]

@dkong
I'll give that a try right now. Stay tuned!

[joshula]

I did a bunch of testing. Still doesn't work. But I identified a new problem (that I didn't know I had before). I can't connect to my running Sentinel containers (using redis-cli) at all (it's telling me connection refused). However, I know they are running, and can properly discover my master and slaves (by viewing their logs). That may be what @mattsta was trying to tell me before? They aren't listening at the proper address? Any ideas on next steps?

[joshula]

@mattsta
Do you have any experience with Docker? I don't have a ton of experience with networking, but I if we can get a Dockerfile created for Sentinel (using your --bind-- solution above) I think it'd be really useful for anyone looking to deploy HA Redis on Docker. I've posted my current Sentinel Dockerfile here... it doesn't include any of your recommendations yet...
http://stackoverflow.com/questions/25914814/redis-sentinel-docker-image-dockerfile

[mattsta]

Nope, no idea about any of that yet.

I'd recommend re-reading the Docker Docs to figure out how things work, then use the tricks we've mentioned above to make Redis do the right thing. 👹

[joshula]

After a ton of work, I ended up figuring this out. Here's to making it simple for anyone else who wants to deploy a highly available redis instance via Docker. Thanks for all the guidance.

https://registry.hub.docker.com/u/joshula/redis-sentinel/

[MrMMorris]

@joshula thanks so much for creating that image! It seems that the creating of the config file from command line arguments is broken. Check the repo comments and let me know when that's resolved 😄