Skip to content

Conversation

rkuo-danswer
Copy link
Contributor

Description

Fixes https://linear.app/danswer/issue/DAN-2009/add-k8s-probes-to-helm

How Has This Been Tested?

[Describe the tests you ran to verify your changes]

Backporting (check the box to trigger backport action)

Note: You have to check that the action passes, otherwise resolve the conflicts manually and tag the patches.

  • This PR should be backported (make sure to check that the backport attempt succeeds)
  • [Optional] Override Linear Check

Copy link

vercel bot commented May 23, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
internal-search ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 23, 2025 0:40am

@rkuo-danswer rkuo-danswer marked this pull request as ready for review May 23, 2025 00:33
@rkuo-danswer rkuo-danswer requested a review from a team as a code owner May 23, 2025 00:33
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

PR Summary

Added Kubernetes health probes (startup, readiness, liveness) across all Celery worker deployments to improve container lifecycle management and monitoring.

  • Configured shared probe settings in values.yaml with specific timeouts: startup (2m), readiness (2m15s), and liveness (5m)
  • Added Python-based health check script execution in /deployment/helm/charts/onyx/templates/celery-worker-*.yaml files
  • Each worker type (light, heavy, indexing, etc.) uses unique temporary files for probe status tracking
  • Startup probe checks for /app/onyx/main.py existence while readiness/liveness probes execute custom health checks
  • Potential concern: Probe script paths and temp file permissions need runtime verification

8 file(s) reviewed, 12 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines 29 to 30
* k -n onyx get pvc
* k -n onyx delete pvc vespa-storage-da-vespa-0
Copy link
Contributor

Choose a reason for hiding this comment

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

style: Consider using full kubectl command instead of shorthand 'k' in documentation for clarity

Suggested change
* k -n onyx get pvc
* k -n onyx delete pvc vespa-storage-da-vespa-0
* kubectl -n onyx get pvc
* kubectl -n onyx delete pvc vespa-storage-da-vespa-0

@Weves Weves added this pull request to the merge queue May 23, 2025
Merged via the queue into main with commit e822afd May 23, 2025
11 checks passed
@Weves Weves deleted the feature/helm-k8s-probes branch May 23, 2025 03:26
ferdinandl007 pushed a commit to ferdinandl007/onyx that referenced this pull request May 27, 2025
* add probes

* lint fixes

---------

Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>
aronszanto pushed a commit to aronszanto/onyx that referenced this pull request May 27, 2025
* add probes

* lint fixes

---------

Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>
ZhipengHe pushed a commit to ZhipengHe/onyx that referenced this pull request Jun 6, 2025
* add probes

* lint fixes

---------

Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>
AnkitTukatek pushed a commit to TukaTek/onyx that referenced this pull request Sep 23, 2025
* add probes

* lint fixes

---------

Co-authored-by: Richard Kuo (Onyx) <rkuo@onyx.app>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants