[bitnami/redis] Redis config changes are ignored

[armsnyder]

Name and Version

bitnami/redis 17.0.6

What steps will reproduce the bug?

1. Install the redis chart
2. Set the replica.configuration value and update the release
3. New configuration does not take effect

Are you using any custom parameters or values?

Values for step 1:

auth:
  enabled: false
sentinel:
  enabled: true

Values for step 2:

auth:
  enabled: false
sentinel:
  enabled: true
replica:
  configuration: |
    maxmemory 2gb
    maxmemory-policy noeviction

What is the expected behavior?

Expected the redis pods to restart and use new configuration.

What do you see instead?

Redis pods restarted but retained the original configuration, evidenced by both the config get maxmemory redis command as well as checking the configuration files in /opt/bitnami/redis:

The mounted ConfigMap has the correct config:

$ cat /opt/bitnami/redis/mounted-etc/replica.conf

dir /data
# User-supplied replica configuration:
maxmemory 2gb
maxmemory-policy noeviction

rename-command FLUSHDB ""
rename-command FLUSHALL ""
# End of replica configuration

However the resulting config is not updated:

$ cat /opt/bitnami/redis/etc/replica.conf

dir /data
# User-supplied replica configuration:
rename-command FLUSHDB ""
rename-command FLUSHALL ""
# End of replica configuration

<snip>

Additional information

The cause appears to be this line of the start-script:

https://github.com/bitnami/charts/blob/5223c9ce5a64ffbc6bffb76e0ca1c1a62d2e5082/bitnami/redis/templates/scripts-configmap.yaml#L175-L177

The previous config is present after pod restart because it is stored in an emptyDIr volume:

https://github.com/bitnami/charts/blob/754325f57d391180aa54f224273e264642195e2d/bitnami/redis/templates/sentinel/statefulset.yaml#L601-L608

I assume this is a bug, because care was taken to ensure that the pod restarts when config is updated:

https://github.com/bitnami/charts/blob/5223c9ce5a64ffbc6bffb76e0ca1c1a62d2e5082/bitnami/redis/templates/sentinel/statefulset.yaml#L43

Commit where the bug was introduced: https://git.app.uib.no/caleno/helm-charts/-/commit/34ee421566848cf688dccbc7f9e31af3c19398f1

[armsnyder]

I found another issue with the same root cause, which is that every time the redis container restarts, the /opt/bitnami/redis/etc/replica.conf config file is appended with the same data over and over. After three restarts it looks like this:

$ cat /opt/bitnami/redis/etc/replica.conf

dir /data
# User-supplied replica configuration:
rename-command FLUSHDB ""
rename-command FLUSHALL ""
# End of replica configuration
replica-announce-port 6379
replica-announce-ip prototype-redis-node-0.prototype-redis-headless.prototype.svc.cluster.local

replica-announce-port 6379
replica-announce-ip prototype-redis-node-0.prototype-redis-headless.prototype.svc.cluster.local

replica-announce-port 6379
replica-announce-ip prototype-redis-node-0.prototype-redis-headless.prototype.svc.cluster.local

replica-announce-port 6379
replica-announce-ip prototype-redis-node-0.prototype-redis-headless.prototype.svc.cluster.local

[rafariossaa]

Hi, Configuration is only created the first time the chart is launched. Once it persist data in the PVC, the configuration is not going to be recreated. This is intended and common in all our charts. The reason is that changing configuration is complex because it could affect the existing data.

[armsnyder]

Thanks for the response. I can understand wanting consistency across Bitnami charts, but as a user who is not familiar with other Bitnami charts it is unexpected behavior.

Can this at least be better documented as far as which values cannot be updated after initial install? This also affects other values like replica.disableCommands which are used to generate the configuration during install only.

Overall I would still prefer having the option to change configuration after the first install. I don't see a downside to giving users that control.

[rafariossaa]

Hi, I will send this to the team to see what could be done to improve the documentation. As I said before, this is not about not giving the user control, it is to keep a known status of your deployment. I you need to change something, you could make a new deployment with the new configuration.

[armsnyder]

I will send this to the team to see what could be done to improve the documentation.

I appreciate it! 😄

this is not about not giving the user control, it is to keep a known status of your deployment

It's both. It's removing a functionality of a Helm value in order to prevent the user from potentially shooting themselves in the foot. The issue I have is that there are valid cases where you would want to update configuration without performing a data migration, like increasing a memory limit.

[armsnyder]

After trying the nats chart, I think it's possible we're not talking about the same thing. 😨

Once it persist data in the PVC, the configuration is not going to be recreated.

To clarify, I'm taking about the Redis config files, which are persisted into an emptyDir host volume after being copied from the configuration Helm value the first time the Redis container starts. No PVC involved as far as I am aware. Were you referring to the Redis AOF and RBD data?

This is intended and common in all our charts.

When I tried the nats chart, it does allow me to update the configuration value.

[rafariossaa]

Regarding this:

I found another issue with the same root cause, which is that every time the redis container restarts, the /opt/bitnami/redis/etc/replica.conf config file is appended with the same data over and over. After three restarts it looks like this: ...

Do you mind opening a new issue on this ? That way we can properly take care of it .

[rafariossaa]

The issue I have is that there are valid cases where you would want to update configuration without performing a data migration, like increasing a memory limit.

I agree, but as you are free to set whatever parameters you need, not only with specific parameters in the chart but also with free form ones like master.configuration, there are a myriad of combinations. Some of them safe to be reconfigured and others not.

In any case, feel free to send a PR with a proposal. We will be glad to review and merge it.

[armsnyder]

Do you mind opening a new issue on this ?

Sure, no problem: #11629

In any case, feel free to send a PR with a proposal.

Opened one here: #11418. (I included several changes in the same PR because one of the changes probably necessitates a major version bump.)

there are a myriad of combinations. Some of them safe to be reconfigured and others not.

I don't think that an unsafe combination can occur without an explicit user action, such as upgrading to a new major version or changing the configuration Helm value input. Thus the user can make their own determination.

[rafariossaa]

Hi, Thanks for sending the PR. It will be reviewed.

[bitnami-bot]

This Issue has been automatically marked as "stale" because it has not had recent activity (for 15 days). It will be closed if no further activity occurs. Thanks for the feedback.