feat(helm): Helm template for Celery beat (for reporting and alerting) #13116

Yann-J · 2021-02-14T16:27:44Z

SUMMARY

This PR adds an optional kubernetes Deployment to run the Celery beat, which is needed in order to trigger the scheduled alerts and reports (as per the excellent unpublished guide at https://github.com/apache/superset/blob/4fa3b6c7185629b87c27fc2c0e5435d458f7b73d/docs/src/pages/docs/installation/email_reports.mdx).

It is off by default, and needs to be enabled with supersetBeat.enabled.

The new pod is defined pretty much exactly like the worker pod except that:

replicas is always 1, since this needs to be a singleton
The command is different

Note that for the chart to be able to execute reports, we still need to check all the other boxes, in particular:

Make sure one webdriver is installed - for this, the commands mentioned in the guide above can be added as a custom supersetWorker.command (this is only needed in the worker container) with the existing chart, with something like this in your values.yaml:

supersetWorker:
  command:
    - /bin/sh
    - -c
    - |
      # Install chrome webdriver
      # See https://github.com/apache/superset/blob/4fa3b6c7185629b87c27fc2c0e5435d458f7b73d/docs/src/pages/docs/installation/email_reports.mdx
      apt update
      wget https://dl.google.com/linux/direct/google-chrome-stable_current_amd64.deb
      apt install -y --no-install-recommends ./google-chrome-stable_current_amd64.deb
      wget https://chromedriver.storage.googleapis.com/88.0.4324.96/chromedriver_linux64.zip
      unzip chromedriver_linux64.zip
      chmod +x chromedriver
      mv chromedriver /usr/bin
      apt autoremove -yqq --purge
      apt clean
      rm -f google-chrome-stable_current_amd64.deb chromedriver_linux64.zip

      # Run
      . {{ .Values.configMountPath }}/superset_bootstrap.sh; celery --app=superset.tasks.celery_app:app worker

Perform all the required Celery setup by overriding the superset_config.py. This is now possible with the latest chart from master by specifying overrides in your values.yaml such as:

configOverrides:
  celery_conf: |
    from celery.schedules import crontab

    class CeleryConfig(object):
      BROKER_URL = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
      CELERY_IMPORTS = ('superset.sql_lab', )
      CELERY_RESULT_BACKEND = f"redis://{env('REDIS_HOST')}:{env('REDIS_PORT')}/0"
      CELERY_ANNOTATIONS = {'tasks.add': {'rate_limit': '10/s'}}
      CELERY_IMPORTS = ('superset.sql_lab', "superset.tasks", "superset.tasks.thumbnails", )
      CELERY_ANNOTATIONS = {
          'sql_lab.get_sql_results': {
              'rate_limit': '100/s',
          },
          'email_reports.send': {
              'rate_limit': '1/s',
              'time_limit': 600,
              'soft_time_limit': 600,
              'ignore_result': True,
          },
      }
      CELERYBEAT_SCHEDULE = {
          'reports.scheduler': {
              'task': 'reports.scheduler',
              'schedule': crontab(minute='*', hour='*'),
          },
          'reports.prune_log': {
              'task': 'reports.prune_log',
              'schedule': crontab(minute=0, hour=0),
          },
          'cache-warmup-hourly': {
              'task': 'cache-warmup',
              'schedule': crontab(minute='*/30', hour='*'),
              'kwargs': {
                  'strategy_name': 'top_n_dashboards',
                  'top_n': 10,
                  'since': '7 days ago',
              },
          }
      }

    CELERY_CONFIG = CeleryConfig
  reports: |
    EMAIL_PAGE_RENDER_WAIT = 60
    WEBDRIVER_BASEURL = "http://superset:8088/"
    WEBDRIVER_BASEURL_USER_FRIENDLY = "https://superset.qa.oneacrefund.org/"
    WEBDRIVER_TYPE= "chrome"
    WEBDRIVER_OPTION_ARGS = [
        "--force-device-scale-factor=2.0",
        "--high-dpi-support=2.0",
        "--headless",
        "--disable-gpu",
        "--disable-dev-shm-usage",
        "--no-sandbox",
        "--disable-setuid-sandbox",
        "--disable-extensions",
    ]
  feature_flags: |
    import ast

    FEATURE_FLAGS = {
        "ALERT_REPORTS": True,
    }
    EMAIL_NOTIFICATIONS = ast.literal_eval(os.getenv("EMAIL_NOTIFICATIONS", "True"))
    SMTP_HOST = os.getenv("SMTP_HOST","localhost")
    SMTP_STARTTLS = ast.literal_eval(os.getenv("SMTP_STARTTLS", "True"))
    SMTP_SSL = ast.literal_eval(os.getenv("SMTP_SSL", "False"))
    SMTP_USER = os.getenv("SMTP_USER","superset")
    SMTP_PORT = os.getenv("SMTP_PORT",25)
    SMTP_PASSWORD = os.getenv("SMTP_PASSWORD","superset")
    SMTP_MAIL_FROM = os.getenv("SMTP_MAIL_FROM","superset@superset.com")

    SLACK_API_TOKEN = os.getenv("SLACK_API_TOKEN",None)

extraSecretEnv:
  SLACK_API_TOKEN: xoxb-...
  SMTP_PASSWORD: ...

extraEnv:
  SMTP_HOST: smtp.gmail.com
  SMTP_USER: ...
  SMTP_PORT: "587"
  SMTP_MAIL_FROM: ...

The above would probably need to be added to the doc together with PR #13104 for the Kubernetes case.

TEST PLAN

Update values.yaml with supersetBeat.enabled: true and upgrade your chart release
See your new pod running!

ADDITIONAL INFORMATION

Has associated issue: chore: update docs for new alerts and reporting feature #13104
Changes UI
Requires DB Migration.
Confirm DB Migration upgrade and downgrade tested.
Introduces new feature or API
Removes existing feature or API

vnourdin

Thanks for your PR! I said on Slack that I would open one, but as our templates aren't up-to-date with those of master it took me some time 😅

vnourdin · 2021-02-15T08:09:51Z

helm/superset/templates/deployment-beat.yaml

+apiVersion: apps/v1
+kind: Deployment
+metadata:
+  name: {{ template "superset.fullname" . }}-beat


I initially named the pods the same way you did, but some colleague said that beat is kind of generic, and it would be better naming it {{ template "superset.fullname" . }}-celerybeat.
I let you judge 😉

Yeah that makes sense...

vnourdin · 2021-02-15T08:16:25Z

helm/superset/templates/deployment-worker.yaml

+        checksum/superset_config.py: {{ include "superset-config" . | sha256sum }}
+        checksum/connections: {{ .Values.supersetNode.connections | toYaml | sha256sum }}
+        checksum/extraConfigs: {{ .Values.extraConfigs | toYaml | sha256sum }}
+        checksum/extraSecretEnv: {{ .Values.extraSecretEnv | toYaml | sha256sum }}
+        checksum/configOverrides: {{ .Values.configOverrides | toYaml | sha256sum }}


[K8S newbie] What's the point of those annotations?

Since those values will change on any content updates of the source values, this will force a patch of the Deployment, and a restart of the pods whenever we publish an update to these (otherwise, updates to a ConfigMap does not automatically force the pods mounting them to restart).

helm/superset/values.yaml

vnourdin · 2021-02-15T08:46:23Z

Also, we need to add documentation on the K8S part about this celery beat, I think this PR is the good one to do it!
Otherwise, we can add it in the other one that document Alerts&Reports, but seem more atomic to do it here.
I can help on this, I'm on the Superset's Slack if you want to reach me 😉

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

Yann-J · 2021-02-16T09:26:22Z

Also, we need to add documentation on the K8S part about this celery beat, I think this PR is the good one to do it!
Otherwise, we can add it in the other one that document Alerts&Reports, but seem more atomic to do it here.
I can help on this, I'm on the Superset's Slack if you want to reach me 😉

Yes, I agree this deserves a documentation update.

Another thing I've been wondering about is Celery Flower... I'm a total noob about Celery / Flask... and in fact Python in general, so I'm not 100% sure what flower does... but I have the impression we might need one pod for it as well?

vnourdin · 2021-02-16T09:39:34Z

Flower is an interface that allow us to monitor celery workers and see which worker run which task!

helm/superset/values.yaml

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

Yann-J · 2021-02-16T09:43:49Z

Flower is an interface that allow us to monitor celery workers and see which worker run which task!

OK. So from a Helm point of view this would mean that:

We would also want a service to expose its port(s)
Potentially we'll want to scale it - and potentially separately from the main pods

vnourdin · 2021-02-16T09:51:29Z

Yes, we want to expose its port, it's done on our instance, I'll check what differs from master charts.
I don't know if this instance must be a singleton or not, should check.

vnourdin · 2021-02-16T10:05:29Z

helm/superset/values.yaml

+  command:
+    - "/bin/sh"
+    - "-c"
+    - ". {{ .Values.configMountPath }}/superset_bootstrap.sh; celery beat --app=superset.tasks.celery_app:app --pidfile /tmp/celerybeat.pid --schedule /tmp/celerybeat-schedule"


There is no trace of flower in the master chart in fact 😮 To add it, we should do something really similar to this PR, plus a service dedicated to flower.
I think this is out of the scope of this PR, my team will switch back to the master charts, and we will open PR to add what's missing, but not sure when this will be. 😕
If you have some time to do it, I would gladly help!

Agreed, different PR... and probably not high prio...

Agree, but flower by default does not support auth, and it exposes dangerous functionality and sensitive info. May be a bit out of scope

helm/superset/templates/deployment-beat.yaml

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

Yann-J · 2021-02-16T10:12:10Z

Sorry, I forgot to update the template to reflect the supersetCeleryBeat name change!

vnourdin · 2021-02-16T10:22:59Z

No problem, Github suggestions isn't perfect 😅
Sorry for flooding the PR.

dpgaspar

LGTM, the suggestion on how to add the webdriver is not ideal. Calling @craig-rueda here for some additional thoughts

dpgaspar · 2021-02-16T11:00:29Z

helm/superset/values.yaml

+  command:
+    - "/bin/sh"
+    - "-c"
+    - ". {{ .Values.configMountPath }}/superset_bootstrap.sh; celery beat --app=superset.tasks.celery_app:app --pidfile /tmp/celerybeat.pid --schedule /tmp/celerybeat-schedule"


Agree, but flower by default does not support auth, and it exposes dangerous functionality and sensitive info. May be a bit out of scope

Yann-J · 2021-02-16T13:26:35Z

LGTM, the suggestion on how to add the webdriver is not ideal. Calling @craig-rueda here for some additional thoughts

Yes I definitely agree... installing stuff at runtime isn't a really good practice since it creates extra risks and often requires running as root. The best would really be to have a pre-built image for it, but I'm not aware of any official ones, and this is out of scope of this PR anyway...

Btw I would argue that the same goes for the way extra pip packages are installed (which also requires running the containers as root...)

craig-rueda

Lgtm

Yann-J added 3 commits February 12, 2021 19:09

Custom superset_config.py + secret envs

603959c

Helm template for celery beat

a9e3962

Merge master

9cd0d8c

pull-request-size bot added the size/L label Feb 14, 2021

Yann-J mentioned this pull request Feb 14, 2021

chore: update docs for new alerts and reporting feature #13104

Merged

6 tasks

Fix end of file

2bf6548

vnourdin reviewed Feb 15, 2021

View reviewed changes

Update helm/superset/values.yaml

ed4b43d

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

Rename pods

ed2805a

vnourdin reviewed Feb 16, 2021

View reviewed changes

helm/superset/values.yaml Outdated Show resolved Hide resolved

Update helm/superset/values.yaml

2d1f719

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

vnourdin reviewed Feb 16, 2021

View reviewed changes

Yann-J and others added 5 commits February 16, 2021 12:09

Update helm/superset/templates/deployment-beat.yaml

9750732

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

Update helm/superset/templates/deployment-beat.yaml

ff6653a

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

Update helm/superset/templates/deployment-beat.yaml

327573d

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

Update helm/superset/templates/deployment-beat.yaml

cb18a2d

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

Update helm/superset/templates/deployment-beat.yaml

29412d0

Co-authored-by: Valentin Nourdin <valentin.nourdin@lilo.org>

dpgaspar reviewed Feb 16, 2021

View reviewed changes

Yann-J added 2 commits February 16, 2021 15:02

Added Kubernetes documentation

a764989

Data source declarations

6febd34

pull-request-size bot added size/XL and removed size/L labels Feb 16, 2021

craig-rueda approved these changes Feb 16, 2021

View reviewed changes

craig-rueda merged commit 2e93784 into apache:master Feb 16, 2021

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(helm): Helm template for Celery beat (for reporting and alerting) #13116

feat(helm): Helm template for Celery beat (for reporting and alerting) #13116

Yann-J commented Feb 14, 2021

vnourdin left a comment

vnourdin Feb 15, 2021

Yann-J Feb 16, 2021

vnourdin Feb 15, 2021

Yann-J Feb 16, 2021

vnourdin commented Feb 15, 2021

Yann-J commented Feb 16, 2021

vnourdin commented Feb 16, 2021

Yann-J commented Feb 16, 2021

vnourdin commented Feb 16, 2021

vnourdin Feb 16, 2021

Yann-J Feb 16, 2021

dpgaspar Feb 16, 2021

Yann-J commented Feb 16, 2021

vnourdin commented Feb 16, 2021

dpgaspar left a comment

dpgaspar Feb 16, 2021

Yann-J commented Feb 16, 2021 •

edited

Loading

craig-rueda left a comment

feat(helm): Helm template for Celery beat (for reporting and alerting) #13116

feat(helm): Helm template for Celery beat (for reporting and alerting) #13116

Conversation

Yann-J commented Feb 14, 2021

SUMMARY

TEST PLAN

ADDITIONAL INFORMATION

vnourdin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vnourdin commented Feb 15, 2021

Yann-J commented Feb 16, 2021

vnourdin commented Feb 16, 2021

Yann-J commented Feb 16, 2021

vnourdin commented Feb 16, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yann-J commented Feb 16, 2021

vnourdin commented Feb 16, 2021

dpgaspar left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Yann-J commented Feb 16, 2021 • edited Loading

craig-rueda left a comment

Choose a reason for hiding this comment

Yann-J commented Feb 16, 2021 •

edited

Loading