Helm Chart Support

[JayGhiya]

Have added helm charts and externalised all possible values in values.yaml.

Currently the pr is not ready to be merged facing weird issue with web server.

[vercel]

@JayGhiya is attempting to deploy a commit to the Danswer Team on Vercel.

A member of the Team first needs to authorize it.

[JayGhiya]

current issue is : nginx not forwarding request to webserver . post this chart would be in usable state. Debugging!

[JayGhiya]

ok issue is resolved. indentation issues haha in reading env! Now doing final testing.

[JayGhiya]

Charts have resource requirements added in except web server. The charts are ready for usage @yuhongsun96 :)

[JayGhiya]

@yuhongsun96 the pr from #1186 looks more feature complete when it comes to helm standards. thanks Arnaud :) . @arnaud-ritti. i will combine missing piece such as OpenEbs storage for readwritemany ,cpu/mem contrainsts and ha practices so it has best of both worlds. Also as arnaud's pr is more feature complete when it comes to helm standards lets discard this one once we take useful. things from this pr and merge from #1186 .

[JayGhiya]

so we both had a call me and @arnaud-ritti . we will be closing this pr as we will be working primarily from #1186 .

[JayGhiya]

Current tasks:

• Create Hosted Vespa charts For Upstream so that could be used directly.
• Add Vespa Hosted Charts as Dependency and override values as per danswer requirements in parent values.yaml
• Add Nginx as Dependency and override values as per danswer requirements in parent values.yaml
• Add new volume/service for model serving
• Add Resource Requirments as per Danswer resource recommendations.
• Add Github CI/Cd for packaging and hosting danswer helm chart.
• Add Documentation for the same.

[JayGhiya]

also cc: @yuhongsun96 this pr already contains helm charts without subchart mechanism allowing people to try through helm. once subchart mechanism is complete we will remove the non-standard helm charts

[yuhongsun96]

Hey folks, in the latest version, we've removed the shared volume between the api server container and background container. It's all been moved to Postgres. Hopefully this simplifies things for the Helm deployment as well!

Is #1186 the more recent one you guys are working on? I can review/test that one if it's ready

[JayGhiya]

1177 is where we are working right now. Yes so this is good. It should simplify the helm deployment. There is a checklist that has been created just above this comment so that is what we are following. Openebs/nfs is not required right now. Hopefully today or tomr aft completing checklist we could start testing

[yuhongsun96]

Oh, I thought the other PR was the more recent one, apologies for the confusion! May need to rebase 😅
A couple questions/comments:

• I saw in feat: add Helm chart #1186, nginx was gone and replaced by some ingress options. We use nginx to rewrite some of the paths so if not then this API_PREFIX env variable has to be set. I prefer not removing nginx though as it's best to keep the deployments the same across different options like docker compose, k8s yamls, helm.

• In this one, I see there some some significant amount of duplicate code and 2 sets of charts, I prefer the layout of feat: add Helm chart #1186, it looked very reasonable.

• We want to get it to a point where running helm install just works with no modifications so people can easily test it out on minikube or docker desktop. Having a working base state makes customizing much easier.

• The shared volumes are not necessary anymore. The only two required persistent storage is for Postgres and Vespa now. Additionally an optional one to have would be the model cache https://github.com/danswer-ai/danswer/blob/main/deployment/docker_compose/docker-compose.dev.yml#L77 but this does not need to be shared between the services

I'll be available now to work closely with you guys to get this finalized. I'm not familiar with Helm but I'm happy to share knowledge on how the rest of the stuff works and interacts.

Thank you both again for all the great work that's gone into this!

[JayGhiya]

amazing right @yuhongsun96 . so we are going to keep nginx. and yes the layout is going to be based out of 1186. And yes we want to make sure that it runs easily on minikube or docker desktop.

[JayGhiya]

@JayGhiya, I have checked out your changes, was able to resolve all the issues and tested it locally with successful results. Could you or someone else please advise on the next steps for this pull request? Thank you!

Thankyou for solving pending issues. I will add you as a collaborator then you could collab with me on same issue.

[JayGhiya]

@JayGhiya, I have checked out your changes, was able to resolve all the issues and tested it locally with successful results. Could you or someone else please advise on the next steps for this pull request? Thank you!

Thankyou for solving pending issues. I will add you as a collaborator then you could collab with me on same issue.

Have added you as collaborator. You could push to the same pr.Also if you have less of time and want to get this done asap we can get on a call tomr if that works for you.

[acquia-ankith]

@JayGhiya I have pushed my changes to same PR. Checkout my changes and test locally. We can have a call if required, I'm free tomorrow.

[JayGhiya]

@JayGhiya I have pushed my changes to same PR. Checkout my changes and test locally. We can have a call if required, I'm free tomorrow.

thankyou @acquia-ankith will test this tomr and then we could get @yuhongsun96 for final tests and then release :)

[JayGhiya]

@acquia-ankith i have commented for one change and fixed one image issue. thats all post that i can check.

[acquia-ankith]

@JayGhiya It looks good. But regarding the secrets for postgres_username and postgres_password, the existing secrets route didn't work for me locally, it was throwing authentication failed error in API Server. So I used the auth.username and auth.password to set those and used the same to set env variables in api-deployment.yaml and background-deployment.yaml files.

[JayGhiya]

@JayGhiya It looks good. But regarding the secrets for postgres_username and postgres_password, the existing secrets route didn't work for me locally, it was throwing authentication failed error in API Server. So I used the auth.username and auth.password to set those and used the same to set env variables in api-deployment.yaml and background-deployment.yaml files.

Sure yea that shud be fixed. I am working towards finding why authentication issue occurs.

[JayGhiya]

@acquia-ankith @yuhongsun96 charts are in good shape. please start testing then could work with @yuhongsun96 on github actions for packaging/signing charts with automatic release changelog

[JayGhiya]

@acquia-ankith brother if you could start testing basic features of danswer and also optimise on resources(it takes time right now) according to danswer documentation it would of huge help.

[JayGhiya]

Most of high resources across danswer stack are fixed. tested github connector. Search worked also :

cc: @yuhongsun96 @acquia-ankith thankyou guys

[JayGhiya]

@yuhongsun96 we could release charts now and then work towards improvements :) . Guys have been waiting for eternity and i feel guilty lets release

[JayGhiya]

Shoutout to @arnaud-ritti too for base helm charts . Will request you to test and provide feedback we could take all the feedback onto different issues post this release @arnaud-ritti

[yuhongsun96]

Thank you all so much for the amazing work! I will test it in the morning!

[yuhongsun96]

Looks good, somehow nginx isn't working for me out of the box but I can iterate on top of this.

Great work!

[JayGhiya]

Okay i checked localhost:3000 while testing it worked. But it takes a while to come up. @acquia-ankith did you get a chance to try

[acquia-ankith]

@JayGhiya I tried to install helm charts using main branch after merge. I am getting following error in web server pod.

2024-05-09 11:22:43 Error fetching user: TypeError: fetch failed
2024-05-09 11:22:43 Some fetch failed for the main search page - TypeError: fetch failed
2024-05-09 11:22:43 TypeError: fetch failed
2024-05-09 11:22:43 at node:internal/deps/undici/undici:12618:11
2024-05-09 11:22:43 at async globalThis.fetch (/app/.next/server/chunks/1638.js:1:36428)
2024-05-09 11:22:43 at async Promise.all (index 0)
2024-05-09 11:22:43 at async l (/app/.next/server/chunks/5566.js:40:13507)
2024-05-09 11:22:43 at async u (/app/.next/server/chunks/5566.js:40:13017) {
2024-05-09 11:22:43 cause: Error: unknown scheme
2024-05-09 11:22:43 at makeNetworkError (node:internal/deps/undici/undici:5840:35)
2024-05-09 11:22:43 at schemeFetch (node:internal/deps/undici/undici:10745:34)
2024-05-09 11:22:43 at node:internal/deps/undici/undici:10615:26
2024-05-09 11:22:43 at mainFetch (node:internal/deps/undici/undici:10634:11)
2024-05-09 11:22:43 at fetching (node:internal/deps/undici/undici:10582:7)
2024-05-09 11:22:43 at fetch (node:internal/deps/undici/undici:10446:20)
2024-05-09 11:22:43 at fetch (node:internal/deps/undici/undici:12617:10)
2024-05-09 11:22:43 at fetch (node:internal/bootstrap/web/exposed-window-or-worker:79:16)
2024-05-09 11:22:43 at i (/app/node_modules/next/dist/compiled/next-server/app-page.runtime.prod.js:12:177566)
2024-05-09 11:22:43 at F (/app/.next/server/chunks/1638.js:1:39684)
2024-05-09 11:22:43 }
2024-05-09 11:22:43 [Error: An error occurred in the Server Components render. The specific message is omitted in production builds to avoid leaking sensitive details. A digest property is included on this error instance which may provide additional details about the nature of the error.] {
2024-05-09 11:22:43 digest: '3395698411'
2024-05-09 11:22:43 }

[JayGhiya]

I can check again then let's figure out if there is old data that is causing this in your K8s.

[acquia-ankith]

@JayGhiya Should we update this line in indexing-model-deployment.yaml
image: danswer/danswer-model-server:latest
to use variable that can be updated with values.yaml ??