Skip to content

fix(helm): enable graceful termination and overrides for celery worker#41175

Open
mjlshen wants to merge 1 commit into
apache:masterfrom
mjlshen:mjlshen/helm-lifecycle
Open

fix(helm): enable graceful termination and overrides for celery worker#41175
mjlshen wants to merge 1 commit into
apache:masterfrom
mjlshen:mjlshen/helm-lifecycle

Conversation

@mjlshen

@mjlshen mjlshen commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown

SUMMARY

Similar to #41173

Currently, the worker startup command runs celery without exec. When the shell script is the container's entrypoint, the shell stays PID 1 and celery runs as its child. Since the shell installs no handler for SIGTERM, the signal delivered on Kubernetes pod termination never reaches celery, causing it to get SIGKILLed when the grace period expires. This drops in-flight tasks and leaves the worker status as online.

Adding exec before the celery command ensures that celery replaces the shell as PID 1, allowing it to receive and handle termination signals directly.

Additionally, this adds support for overriding lifecycle hooks and terminationGracePeriodSeconds for the superset worker deployment, matching the server (supersetNode) capabilities to allow custom shutdown commands (such as celery control shutdown) and extended drain periods."

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

  • Has associated issue: Fixes Superset worker has no lifecycle preStop command #33513
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@dosubot dosubot Bot added the deploy:helm label Jun 17, 2026
@bito-code-review

bito-code-review Bot commented Jun 17, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review Agent Run #b5120c

Actionable Suggestions - 0
Additional Suggestions - 1
  • helm/superset/values.yaml - 1
    • Missing graceful shutdown options in beat · Line 473-476
      The `supersetCeleryBeat` section lacks `lifecycle` and `terminationGracePeriodSeconds` options that were added to both `supersetNode` (lines 363-368) and `supersetWorker` (lines 473-476). This creates an inconsistent configuration API where users cannot configure graceful shutdown for all celery components uniformly.
Review Details
  • Files reviewed - 3 · Commit Range: f0c7eb0..f0c7eb0
    • helm/superset/templates/deployment-worker.yaml
    • helm/superset/templates/deployment.yaml
    • helm/superset/values.yaml
  • Files skipped - 1
    • helm/superset/README.md - Reason: Filter setting
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@mjlshen mjlshen mentioned this pull request Jun 17, 2026
Open
3 tasks
@mjlshen mjlshen force-pushed the mjlshen/helm-lifecycle branch from f0c7eb0 to 88a5c45 Compare June 17, 2026 22:35
@pull-request-size pull-request-size Bot added size/M and removed size/S labels Jun 17, 2026
@bito-code-review

bito-code-review Bot commented Jun 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review Agent Run #00e35f

Actionable Suggestions - 0
Review Details
  • Files reviewed - 4 · Commit Range: 88a5c45..88a5c45
    • helm/superset/Chart.yaml
    • helm/superset/templates/deployment-worker.yaml
    • helm/superset/templates/deployment.yaml
    • helm/superset/values.yaml
  • Files skipped - 1
    • helm/superset/README.md - Reason: Filter setting
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

rusackas
rusackas approved these changes Jun 19, 2026
View reviewed changes

@rusackas rusackas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exec fix LGTM. One thing in README.md: the bumped line reads Version: 0.16.3 but the badge URL still points at Version-0.16.2-informational. Looks like a partial helm-docs regen — can you re-run helm-docs so they match @mjlshen?

@mjlshen
fix(helm): enable graceful termination and overrides for celery worker
f8b200e 
Currently, the worker startup command runs celery without `exec`. When
the shell script is the container's entrypoint, the shell stays PID 1
and celery runs as its child. Since the shell installs no handler for
SIGTERM, the signal delivered on Kubernetes pod termination never
reaches celery, causing it to get SIGKILLed when the grace period
expires. This drops in-flight tasks and leaves the worker status as
online.

Adding `exec` before the celery command ensures that celery replaces
the shell as PID 1, allowing it to receive and handle termination
signals directly.

Additionally, this adds support for overriding `lifecycle` hooks and
`terminationGracePeriodSeconds` for the superset worker deployment,
matching the server (supersetNode) capabilities to allow custom
shutdown commands (such as celery control shutdown) and extended drain
periods."

Signed-off-by: Michael Shen <mishen@umich.edu>
@mjlshen mjlshen force-pushed the mjlshen/helm-lifecycle branch from 88a5c45 to f8b200e Compare June 19, 2026 03:31
@netlify

netlify Bot commented Jun 19, 2026

Copy link
Copy Markdown

✅ Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit f8b200e
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a34b812c3e2a50008aed241
😎 Deploy Preview https://deploy-preview-41175--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@mjlshen

mjlshen commented Jun 19, 2026

Copy link
Copy Markdown
Author

The exec fix LGTM. One thing in README.md: the bumped line reads Version: 0.16.3 but the badge URL still points at Version-0.16.2-informational. Looks like a partial helm-docs regen — can you re-run helm-docs so they match @mjlshen?

Yes - thanks! Just to note there's a bit of a race between this MR and #41173 - whichever merges first should be 0.16.3, I will leave that call up to the maintainers. Once one merges in, I can update the remaining MR to move to version 0.16.4

@bito-code-review

bito-code-review Bot commented Jun 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Code Review Agent Run #c6baed

Actionable Suggestions - 0
Review Details
  • Files reviewed - 4 · Commit Range: f8b200e..f8b200e
    • helm/superset/Chart.yaml
    • helm/superset/templates/deployment-worker.yaml
    • helm/superset/templates/deployment.yaml
    • helm/superset/values.yaml
  • Files skipped - 1
    • helm/superset/README.md - Reason: Filter setting
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rusackas rusackas rusackas approved these changes

@craig-rueda craig-rueda Awaiting requested review from craig-rueda craig-rueda is a code owner

@dpgaspar dpgaspar Awaiting requested review from dpgaspar dpgaspar is a code owner

@villebro villebro Awaiting requested review from villebro villebro is a code owner

@nytai nytai Awaiting requested review from nytai nytai is a code owner

@michael-s-molina michael-s-molina Awaiting requested review from michael-s-molina michael-s-molina is a code owner

@mistercrunch mistercrunch Awaiting requested review from mistercrunch mistercrunch is a code owner

@Antonio-RiveroMartnez Antonio-RiveroMartnez Awaiting requested review from Antonio-RiveroMartnez Antonio-RiveroMartnez is a code owner

Assignees

No one assigned

Labels

deploy:helm size/M

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Superset worker has no lifecycle preStop command

2 participants

@mjlshen @rusackas