[Helm Chart] redis password not set for celery brokers

[elyzov]

I've set REDIS_PASSWORD environment variable, but celery worker can't connect to redis, as the password not set in connection string.

Expected results

REDIS_PASSWORD environment variable is used in all redis configs.

Actual results

REDIS_PASSWORD environment variable is used only in cache configuration.

How to reproduce the bug

1. Deploy redis with password.
2. Set REDIS_PASSWORD environment variable.
3. Deploy superset via helm chart.
4. See errors in worker logs.

Environment

(please complete the following information):

• superset version: 1.2.0
• helm chart version: 0.3.5

Checklist

Make sure to follow these steps before submitting your issue - thank you!

• I have checked the superset logs for python stacktraces and included it here as text if there are any.
• I have reproduced the issue with at least the latest released version of superset.
• I have checked the issue tracker for the same issue and I haven't found one similar.

[nytai]

You can configure celery however you want via the superset_config overrides https://superset.apache.org/docs/installation/running-on-kubernetes#superset_configpy

[elyzov]

I seе, I can do some workaround, but nevertheless, there is a bug in the default configuration.

[ad-m]

@elyzov could you check if that issue still is relevant after merged & released #18642 ?

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. For admin, please label this issue .pinned to prevent stale bot from closing the issue.

[naharoo]

The issue causing this behavior is this snippet from the Helm chart.

class CeleryConfig(object):
  CELERY_IMPORTS = ('superset.sql_lab', )
  CELERY_ANNOTATIONS = {'tasks.add': {'rate_limit': '10/s'}}
{{- if .Values.supersetNode.connections.redis_password }}
  BROKER_URL = f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
  CELERY_RESULT_BACKEND = f"redis://:{env('REDIS_PASSWORD')}@{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- else }}
  BROKER_URL = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
  CELERY_RESULT_BACKEND = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
{{- end }}

It basically checks whether supersetNode.connections.redis_password is provided or not and includes the password in redis url based on that.

We can work around this with just providing a dummy value to redis_password helm configuration property and overriding it with the env variable REDIS_PASSWORD.

supersetNode:
  connections:
    redis_password: "dummy-placeholder"

extraEnvRaw:
  - name: "REDIS_PASSWORD"
    valueFrom:
      secretKeyRef:
        name: "redis-credentials"
        key: "password"

If this behavior is not implemented intentionally, I can open a PR with a fix.

[lijiajia3515]

docker-compose deploy mode +1 ,

superset_worker_beat  | [2023-03-23 12:19:41,343: ERROR/MainProcess] beat: Connection error: Authentication required.. Trying again in 28.0 seconds...
superset_worker       | [2023-03-23 12:19:43,586: ERROR/MainProcess] consumer: Cannot connect to redis://192.168.40.32:6379/0: Authentication required..

hope add environment
REDIS_URL or REDIS_USER & REDIS_PASSWORD & REDIS_DB

[michael-s-molina]

@villebro @craig-rueda Can you check if the issue was resolved in the latest version of the Helm chart? If yes, please close this issue.

[rusackas]

Assuming this is resolved. It was also reported as part of Superset 1.x, which we no longer support, so I'll close it. If there are further needs to dive into configuration issues, I'd encourage you to open it as a GitHub Discussion, and/or bring it up on slack. Alternatively, I can reopen this and convert it to a discussion if anyone would prefer that.