Fix for worker healthcheck

[brent-stone]

superset-worker container health check on provided Docker Compose fails because a Celery dependency expects the CELERY_BROKER_URL environment variable which is currently not set in the 2.1.0rc3 release of Superset.

How to reproduce the bug

1a. In the current docker-compose.yml and docker-compose-non-dev.yml the healthcheck will fail because Celery's dependency, Kombu, implicitly expects an environment variable CELERY_BROKER_URL in order for the CLI celery inspect ping to work.
1b. With the superset-worker container running, docker exec -it superset-worker /bin/sh then run the CLI command celery inspect ping.
2. Kombu will throw a ConnectionRefusedError: [Errno 111] Connection refused because the CELERY_BROKER_URL environment variable is not set.

Expected results

celery inspect ping should return a JSON response like {"OK": "pong"} if Celery is properly configured to see a broker.

Actual results

ConnectionRefusedError: [Errno 111] Connection refused resulting in the container being marked as unhealthy

Screenshots

This screenshot of an interactive terminal in the superset-worker container before and after adding the CELERY_BROKER_URL environment variable.

This follow-up screenshot shows how the currently written health check will still fail due to the unrecognized -A flag for the celery inspect sub-command:

Environment

(please complete the following information):

• browser type and version: Any
• superset version: 2.1.0rc3
• python version: 3.8.16 (running inside the superset-worker container)
• node.js version: N/A
• any feature flags active: N/A

Checklist

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

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

Additional context

This may be most easily fixed by adding the following to the ./docker/.env-non-dev environment variables:

# This is still needed to ensure Celery's upstream Kombu dependency understands which
# broker and backend port to use.
CELERY_BROKER_URL: redis://redis/0

I also recommend simplifying the health check from:

healthcheck:
      test: ["CMD-SHELL", "celery inspect ping -A superset.tasks.celery_app:app -d celery@$$HOSTNAME"]

to

healthcheck:
      test: [ "CMD-SHELL", "celery inspect ping | grep 'pong'" ]
      interval: 5s
      timeout: 30s
      retries: 10

The current version of the health check throws an error that the -A flag is not recognized.

A script to ensure the configs stay in sync with the REDIS_HOST, REDIS_PORT, REDIS_CELERY_DB, and REDIS_RESULTS_DB used in superset_config.py would be ideal. I can submit a pull request with the recommended fix if the maintainers would like that.

Finally, adding the recommended patch and health check to the celery-worker-beat container may be desirable.

Thanks for all your hard work

[mdeshmu]

@eschutho ^

[nytai]

that's just a warning, it isn't actually failing, is it? the command just needs to be updated to

"CMD-SHELL", "celery -A superset.tasks.celery_app:app inspect ping -d celery@$$HOSTNAME"

[brent-stone]

CELERY_BROKER_URL

The update to the celery command would fix the warning; however, the health check would still have issues.

Without the CELERY_BROKER_URL environment variable being set, Docker compose and docker ps would mark the container as unhealthy because Kombu will throw an exception whenever celery inspect ping is called. Easy to fix with the current setup by using os to programmatically set the environment variable in the ./docker scripts or elsewhere.

Happy to set up a pull request adding CELERY_BROKER_URL and update the health check command if you'd like.

[rusackas]

This is likely fixed by now, and is pretty out of date if not. If people are still encountering this in current versions (3.x) please open a new Issue or a PR to address the problem.