[bitnami/redis] Requested chart change to allow for istio with mtls.

[MichaelHudgins]

Which chart:
redis

So like other people that have tried the redis chart with istio enabled with mtls I ran into some issues, but I was able to get redis with sentinel working with the following modifications, one would need chart changes to support. I am going to go ahead and outline what I needed to do in full for anyone who runs into this issue.

The first change is that services need to be named with tcp pre-pending the name, this is particularly important for the headless services as being flagged as tcp is what keeps direct ip traffic from hitting the PassthroughCluster as flagging it this way adds a direct ip endpoint for each redis pod. I purpose changing the names to this in the default chart or making the name of the ports a possible chart variable. I changed both the headless and non headless service to have the following names. (for sentinel tcp-redis-sentinel was too long)

  ports:
  - name: tcp-redis
    port: 6379
    protocol: TCP
    targetPort: redis
  - name: tcp-sentinel
    port: 26379
    protocol: TCP
    targetPort: redis-sentinel

The next issue has to do with how istio handles the ip address, we need to announce our outward facing ip or else we run into all sorts of replica sync issues. I believe these two fixes can be done with the chart when #4970 makes it in.
Near the bottom of start-node.sh:

    if [[ "$REDIS_REPLICATION_MODE" == "slave" ]]; then
        ARGS+=("--replica-announce-ip" "$(hostname -i)")
    fi

Sentinel needed a similar fix to properly failover if the master went down so I did the same for the start-sentinel.sh by adding the following near the bottom:

    sentinel_conf_add "sentinel announce-ip $(hostname -i)"

Finally that previous fix would sometimes have an issue if redis got up before istio, so finally I set a pod annotation so that istio 1.8 + would prevent redis from starting before istio was ready:

slave:
  podAnnotations:
    proxy.istio.io/config: '{ "holdApplicationUntilProxyStarts": true }'

There might be other things needed for some installs but with all three of those changes redis appears to be working with mtls for me with fallover and rolling upgrades working as I expect. Hope this can help some other people who have run into the same issues as me.

[javsalgar]

Hi,

Thank you very much for the information. I will forward the information to the team so we can evaluate and implement any required changes to make the chart compatible.

[MichaelHudgins]

@javsalgar Would you be open for the service names being changed or set as a chart variable? I am having to overwrite my service names on each helm update. I believe the service names having the tcp is the only part of my istio workflow that is not manageable by chart options.

[javsalgar]

Hi,

I think that this small change is something that we could easily implement. If you want to speed up the process, feel free to submit a PR and we will review it. If not, I will work on it during this iteration.

[MichaelHudgins]

@javsalgar I can do it, would you prefer me to go changing the default or making it a chart option?

[javsalgar]

Thank you so much! I think changing the default shouldn't break anything, so I'd go for that option.

[MichaelHudgins]

Sounds good, PR is in