-
Notifications
You must be signed in to change notification settings - Fork 152
Add Sourcebot Helm Chart #370
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Andre Nogueira <[email protected]>
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This is awesome thanks Andre! Let us know if there's anything we can help with to get this PR out of drafts |
The chart it self should be good to go. In order to use it, the typical way is to provide a way to fetch the packaged chart. This means hosting it on a registry. Alternatively, Github pages can be used for this - here is the documentation reference: https://helm.sh/docs/topics/chart_repository/ Added it as a Draft to discuss which approach would work best here. |
@aanogueira I haven't looked at the code yet (I'll be happy to give a review if you're interested ; we've built our own Helm chart internally and I was also considering contributing it to the community) but I was wondering if you were able to run Sourcebot in Kubernetes just as is with the embedded postgres server (starting from V3)? Asking because we faced errors related to the volume mount (#263) and had to use a separate Postgres in the end. |
kubernetes/chart/README.md
Outdated
| command | list | `[]` | Override the default command of the container. | | ||
| config | object | `{"$schema":"https://raw.githubusercontent.com/sourcebot-dev/sourcebot/main/schemas/v3/index.json","connections":{},"settings":{}}` | Configure Sourcebot-specific application settings. | | ||
| containerSecurityContext | object | `{}` | Set the container-level security context. | | ||
| database | object | `{}` | Configure the database secret. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading this, it's unclear what is expected in the database
key.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in the values.yaml
, but added to the generated docs as well for clarity.
kubernetes/chart/README.md
Outdated
| image | object | `{"pullPolicy":"Always","repository":"ghcr.io/sourcebot-dev/sourcebot","tag":"latest"}` | Configure the container image. | | ||
| image.pullPolicy | string | `"Always"` | Image pull policy. | | ||
| image.repository | string | `"ghcr.io/sourcebot-dev/sourcebot"` | Container image repository. | | ||
| image.tag | string | `"latest"` | Container image tag. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we want to default to "latest"? It's usually a bad practice to use "latest" as you can't be sure of which version is deployed and it's not reproducible. I would stick a fixed version here by default, and let user change it optionally.
This also means that the release workflow should include bumping the default version of the image in the container and release a new version of the Helm chart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was me testing it out, and forgot to remove it from the default values.
Added the specific version to the appVersion
section of the Chart.yaml
metadata.
kubernetes/chart/README.md
Outdated
| ingress.hosts | list | `[]` | List of hostnames and paths for ingress rules. | | ||
| ingress.tls | list | `[]` | TLS settings for ingress. | | ||
| initContainers | list | `[]` | Configure init containers to run before the main container. | | ||
| license | object | `{}` | Configure the enterprise license key secret. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, unclear what kind of value is expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in the values.yaml
, but added to the generated docs as well for clarity.
kubernetes/chart/README.md
Outdated
| readinessProbe.httpGet.port | string | `"http"` | Port to check. | | ||
| readinessProbe.initialDelaySeconds | int | `10` | Initial delay before the first probe. | | ||
| readinessProbe.periodSeconds | int | `10` | Frequency of the probe. | | ||
| redis | object | `{}` | Configure the Redis secret. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same comment here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's in the values.yaml
, but added to the generated docs as well for clarity.
kubernetes/chart/README.md
Outdated
| startupProbe.periodSeconds | int | `30` | Initial delay before the first probe. | | ||
| tolerations | list | `[]` | Set tolerations for pod scheduling. See: https://kubernetes.io/docs/concepts/scheduling-eviction/taint-and-toleration/ | | ||
| volumeMounts | list | `[]` | Define volume mounts for the container. | | ||
| volumes | list | `[]` | Define additional volumes. | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably give some guidance as to what to configure for volumes (especially path expected by Sourcebot). Unless I misread, if nothing is specified, there's no persistence in the current version of the chart.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The volumes
/volumeMounts
follow the default Kubernetes specs.
As for persistence, I added support on the new commit to the storage
option to support the usage of the internal DB, mounting to the pod under /data
path.
No, there was no persistence. For live deployments, I was not assuming the usage of the internal DB, but indeed there might be some scenario where it could be useful. For those cases, we could deploy a StatefulSet instead of Deployment. Added a new block (storage) to add persistence storage, changing the deployable from a Deployment to a StatefulSet. |
Signed-off-by: Andre Nogueira <[email protected]>
Hey @aanogueira thanks for the PR!! Apologies for the delay on our end - has been hectic shipping Ask SB 🤠 Planning on getting to this this week. I've only briefly used helm charts before, so I'll probably ask you a bunch of dumb questions as I ramp up on things 😄 On my side, I will get a minikube instance running locally and test out the chart |
I am following this PR mostly to see how we are supposed to scale the sourcebot deployment past as single pod. Sure, we can run StatefulSet and also an external database but what about the storage itself? Most storage drivers do not allow many pods writing to the same PVC but let's say we can, how does Sourcebot handle this correctly? |
Hey @komapa - not certain on what the approach will be here, but speaking to multiple-writers: I think the only thing that needs to write to the volume are the "workers" (i.e., Does this approach make sense? *Edit: working on a RFC here: #439 |
I think I would prefer Github pages since we can keep everything under one umbrella. I was reading up on it and it seems the best option for that is to host the chart from a separate repo, so I created this one: https://github.com/sourcebot-dev/sourcebot-k8s |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with merging this. Thanks @gaeljw for also reviewing!! Once this is in, I can figure out deploying it on GitHub pages, and then write up some quick docs.
I think next up w.r.t., deployment stuff: I'm going to get started on a RFC in #439 with some of my thoughts on packaging our individual services into separate containers (i.e., ghcr.io/sourcebot-dev/(webapp|worker|zoekt)
), as well as some thoughts on how we can support multiple replicas per service. Please feel free to add your own thoughts!
For the record, the setup we currently use at my company is the following:
|
@brendan-kellam I believe naming the repo |
I can help with that if needed! 😄 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aanogueira really good stuff, thanks for putting this together. 🙇
Added a few nits but overall LGTM.
Signed-off-by: Andre Nogueira <[email protected]>
Signed-off-by: Andre Nogueira <[email protected]>
Signed-off-by: Andre Nogueira <[email protected]>
Signed-off-by: Andre Nogueira <[email protected]>
Signed-off-by: Andre Nogueira <[email protected]>
Signed-off-by: Andre Nogueira <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This PR is being reviewed by Cursor Bugbot
Details
You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @aanogueira for your hard work on this!! 🎉 Also @gaeljw @komapa and @pjbgf for jumping in too! Much appreciated.
Going to merge this now to close this thread - will then migrate the chart over to https://github.com/sourcebot-dev/sourcebot-helm-chart and get it deployed on GitHub pages. Will also add some docs for deploying to k8s.
Thanks again!
This PR adds an Helm Chart enabling the deployment of Sourcebot to Kubernetes.