✨ Helm Chart for VideoQnA Application #497

krish918 · 2024-10-24T03:55:29Z

Description

This PR introduces Helm Chart for VideoQnA application for easy setup of the application suite along with all dependencies.

Issues

N/A

Type of change

List the type of change like below. Please delete options that are not relevant.

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds new functionality)
Breaking change (fix or feature that would break existing design and interface)

Dependencies

No Dependencies.

Tests

All helm charts are accompanied with appropriate tests.

* Optimize path and link validity check. Signed-off-by: ZePan110 <[email protected]> * debug Signed-off-by: ZePan110 <[email protected]> * debug2 Signed-off-by: ZePan110 <[email protected]> * Fix issue Signed-off-by: ZePan110 <[email protected]> * Remove debug output Signed-off-by: ZePan110 <[email protected]> * test Signed-off-by: ZePan110 <[email protected]> * Fix issue. Signed-off-by: ZePan110 <[email protected]> * Fix issue Signed-off-by: ZePan110 <[email protected]> * Restore test file Signed-off-by: ZePan110 <[email protected]> --------- Signed-off-by: ZePan110 <[email protected]> Signed-off-by: Krishna Murti <[email protected]>

Signed-off-by: Krishna Murti <[email protected]>

lianhao

Thanks very much for contributing this.

Besides the embedded comment, we should also:

add the README.md
enable the CI, please find the ci-xxx-values.yaml files in the component helm charts

helm-charts/common/data-prep/values.yaml

lianhao · 2024-10-25T01:31:51Z

helm-charts/common/data-prep/vdms-values.yaml

@@ -0,0 +1,18 @@
+# Copyright (C) 2024 Intel Corporation


We need to change the file name vdms-values.yaml to variant_vdms-values.yaml so the update_manifest.sh can automatically pick this to generate manifests files for GMC. Please see llm_uservice as example

lianhao · 2024-10-25T01:40:53Z

helm-charts/common/embedding-usvc/vdms-values.yaml

@@ -0,0 +1,15 @@
+# Copyright (C) 2024 Intel Corporation


again, please change use the variant_vdms-values.yaml as the file names so update_manifest.sh can pick it up automatically.

lianhao · 2024-10-25T01:45:40Z

helm-charts/common/reranking-usvc/vdms-values.yaml

@@ -0,0 +1,12 @@
+# Copyright (C) 2024 Intel Corporation


again, filename change is required here to variant_vdm-values.yaml

lianhao · 2024-10-25T01:55:35Z

helm-charts/common/retriever-usvc/values.yaml

@@ -22,7 +25,7 @@ TEI_EMBEDDING_ENDPOINT: ""
 LOCAL_EMBEDDING_MODEL: ""

 REDIS_URL: ""
-INDEX_NAME: "rag-redis"
+indexName: "rag-redis"


there is no need to change it to lower case, I think

This is related to the first conversation. Based on that, please let me know if this is fine or need to be reverted back?

reverted back to INDEX_NAME as per this conversation.

lianhao · 2024-10-25T01:55:52Z

helm-charts/common/retriever-usvc/vdms-values.yaml

@@ -0,0 +1,19 @@
+# Copyright (C) 2024 Intel Corporation


again, filename change is required here to variant_vdm-values.yaml

Signed-off-by: Krishna Murti <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: Krishna Murti <[email protected]>

lianhao · 2024-11-01T01:27:50Z

helm-charts/videoqna/values.yaml

+  nameOverride: "ui"
+
+  # Following template value will be resolved in videoqna-ui ConfigMap
+  BACKEND_SERVICE_ENDPOINT: http://{{ .Release.Name | trunc 57 | trimSuffix "-" }}-nginx/v1/videoqna


Should this be the "/v1/videoqna", just what we put here for all other UIs, e.g. https://github.com/opea-project/GenAIInfra/blob/main/helm-charts/codegen/values.yaml#L50, and let the nginx reverse proxy to prepend the the scheme and hostname part. I'll let @Ruoyu-y to chime in here because she knows more about all the nginx settings.

Because the UI for videoqna is based on Streamlit, which seems to be running code in backend first before sending the results to the browser. The requests library being used in the Python backend code requires the complete URL along with scheme and host, otherwise fails (unlike frontend js frameworks being used in other UIs, which can take relative URLs without scheme and host). I found, I could resolve this by using the service-name for nginx as host, as all pods can communicate with each other with service name. Please let me know, if there are other better ways to do this.

Ok, seems the videoqna-ui is designed to run the code which connects to videoqna mega gateway service in the ui container itself. In this case, we don't need the nginx part at all. We can set the BACKEND_SERVICE_ENDPOINT to the corresponding k8s svc url of the viedoqna mega gateway service, and remove all the nginx related part, i.e.

BACKEND_SERVICE_ENDPOINT: http://{{ include "videoqna.fullname" . }}:{{ .Values.service.port }}/v1/videoqna

we can change the ui service type to NodePort, and ask the user to directly connect to that NodePort in web browser

May be, nginx would be helpful for future UI updates, in case we have UIs in react or svelte. Please let me know if that's not the case and I should proceed with removing nginx.

Also, UI being a sub-chart for videoqna mega-application, doesn't seem to have access to values and templates defined in the parent chart i.e. videoqna. So, I am finding it difficult to access videoqna.fullname and other videoqna chart value in the UI sub-chart. However, I can access the videoqna svc name the same way I am accessing nginx svc name right now. Please help me if I am missing something here.

helm-charts/common/ui/templates/configmap.yaml

Signed-off-by: Krishna Murti <[email protected]>

for more information, see https://pre-commit.ci

Signed-off-by: Krishna Murti <[email protected]>

mkbhanda

Persistent volume support. And a typo.

mkbhanda · 2024-11-04T21:18:35Z

helm-charts/common/data-prep/README.md

+
+```bash
+# 1) Download a sample video in current dir:
+curl -svLO "https://github.com/opea-project/GenAIExamples/raw/refs/heads/main/VideoQnA/docker_compose/intel/cpu/xeon/data/op_1_0320241830.mp4"


Would it not be better for this sample video to live in some common VidoQnA directory versus in docker_compose/intel/cpu/xeon/data

I suppose requested change needs a PR in GenAIExamples rather than here.

mkbhanda · 2024-11-04T21:31:28Z

helm-charts/common/lvm-serving/README.md

+export HFTOKEN="insert-your-huggingface-token-here"
+
+# Set a dir to cache downloaded Video-Llama Model
+export MODELDIR=/mnt/opea-models


Using persistent volumes better than a node level directory. We have some charts with PV support. I believe submitted by @poussa

any possibility, we can take this up in future in further improvements to the videoqna chart?

mkbhanda · 2024-11-04T21:33:27Z

helm-charts/common/lvm-uservice/README.md

+
+In this setup, lvm-uservice depends on TGI, you should set LVM_ENDPOINT as tgi endpoint.
+
+### (Option1): Installing the chart separately

 First, you need to install the tgi chart, please refer to the [tgi](../tgi) chart for more information.

 After you've deployted the tgi chart successfully, please run `kubectl get svc` to get the tgi service endpoint, i.e. `http://tgi`.


Suggested change

After you've deployted the tgi chart successfully, please run `kubectl get svc` to get the tgi service endpoint, i.e. `http://tgi`.

After you've deployed the tgi chart successfully, please run `kubectl get svc` to get the tgi service endpoint, i.e. `http://tgi`.

This is fixed now. This also came in from VisualQnA PR merge.

helm-charts/common/lvm-uservice/values.yaml

.github/workflows/_helm-e2e.yaml

helm-charts/common/data-prep/ci-videoqna-values.yaml

helm-charts/common/data-prep/templates/deployment.yaml

yongfengdu · 2024-11-05T09:13:05Z

helm-charts/common/data-prep/templates/deployment.yaml

@@ -63,6 +80,12 @@ spec:
          resources:
            {{- toYaml .Values.resources | nindent 12 }}
      volumes:
+        {{- if .Values.global.cacheUseHostPath }}


See comment above.

helm-charts/common/lvm-serving/Chart.yaml

Signed-off-by: Krishna Murti <[email protected]>

yongfengdu · 2024-11-06T07:01:23Z

helm-charts/common/data-prep/templates/deployment.yaml

+              subPath: clip
+            - mountPath: /home/user/.cache/huggingface/hub
+              name: model-volume
+              subPath: huggingface/hub


When using model-volume, the subPath is not necessary.

agree, not necessary. But I suppose, as same volume is being re-used to mount different paths from different pod/containers, having a subPath helps to have a proper directory hierarchy inside model-volume and helps with what is coming from which container path instead of dumping everything in the root of model-volume.

The model-volume was used to save the models downloaded from huggingface hub, which is the HUGGINGFACE_HUB_CACHE here https://huggingface.co/docs/text-generation-inference/en/reference/launcher#huggingfacehubcache

lianhao · 2024-11-08T00:33:18Z

@krish918 please fix the chart lint error and resolve the conflict, thanks!

yongfengdu · 2024-11-06T08:01:31Z

helm-charts/common/data-prep/templates/deployment.yaml

+              subPath: clip
+            - mountPath: /home/user/.cache/huggingface/hub
+              name: model-volume
+              subPath: huggingface/hub


The model-volume was used to save the models downloaded from huggingface hub, which is the HUGGINGFACE_HUB_CACHE here https://huggingface.co/docs/text-generation-inference/en/reference/launcher#huggingfacehubcache

chensuyue · 2024-11-18T06:57:53Z

Can we close the CI issue by today?

ZePan110 and others added 10 commits October 17, 2024 01:23

🚨 linter and pre-commit check fixes

7cd488a

Signed-off-by: Krishna Murti <[email protected]>

✅ Added helm tests for all services | linting fixes

1f121e9

Signed-off-by: Krishna Murti <[email protected]>

✏ typo fix

418edb7

Signed-off-by: Krishna Murti <[email protected]>

🛂 running vdms as root to fix errors

24b4e3f

Signed-off-by: Krishna Murti <[email protected]>

✅ updated tests for vdms-vector-db | typo fix

880e406

Signed-off-by: Krishna Murti <[email protected]>

⚡Updated and fixed several tests, values during integration

0110b85

Signed-off-by: Krishna Murti <[email protected]>

🔥 removed hardcoded extraArgs for some comps

56afc63

Signed-off-by: Krishna Murti <[email protected]>

🔥 removed hardcoded extraArgs for some comps

1a5f762

Signed-off-by: Krishna Murti <[email protected]>

Merge branch 'main' into helm/videoqna

5fbfe85

lianhao requested changes Oct 25, 2024

View reviewed changes

krish918 and others added 9 commits October 28, 2024 00:37

🩹 fixes for request going to nginx instead of videoqna

3bb015d

Signed-off-by: Krishna Murti <[email protected]>

🔨 passed nginx proxy endpoints to UI using templates

a29691b

Signed-off-by: Krishna Murti <[email protected]>

🔨 passed nginx proxy endpoints to UI using templates

e8f58bb

Signed-off-by: Krishna Murti <[email protected]>

💚 added symbolic links to values files for CI

a8cbdbf

Signed-off-by: Krishna Murti <[email protected]>

💚 added symbolic links to values files for CI

63bf7f6

Signed-off-by: Krishna Murti <[email protected]>

📝 Added readme files for videoqna charts and subcharts

cf4360f

Signed-off-by: Krishna Murti <[email protected]>

💚 Added missing symbolic links for some values files

2d02cf9

Signed-off-by: Krishna Murti <[email protected]>

Merge branch 'main' into helm/videoqna

d03a52f

[pre-commit.ci] auto fixes from pre-commit.com hooks

1e5eea1

for more information, see https://pre-commit.ci

krish918 marked this pull request as ready for review October 29, 2024 02:19

krish918 requested a review from yongfengdu as a code owner October 29, 2024 02:19

krish918 added 2 commits October 29, 2024 08:16

💚 added variant prefix to all videoqna values file

6e832ea

Signed-off-by: Krishna Murti <[email protected]>

💚 Added several CI fixes

fc17e30

Signed-off-by: Krishna Murti <[email protected]>

krish918 requested review from daisy-ycguo and mkbhanda as code owners October 30, 2024 11:27

updated videoqna-ui vars and configmap

5ee3dc0

Signed-off-by: Krishna Murti <[email protected]>

krish918 force-pushed the helm/videoqna branch from 42dec51 to 5ee3dc0 Compare October 30, 2024 11:49

krish918 added 2 commits October 30, 2024 17:20

Merge branch 'main' into helm/videoqna

ea180a5

🩹 Fixes in reranking values for enabling data-prep

ef695c8

Signed-off-by: Krishna Murti <[email protected]>

lianhao reviewed Nov 1, 2024

View reviewed changes

lianhao requested a review from Ruoyu-y November 1, 2024 01:29

krish918 and others added 7 commits November 1, 2024 12:45

🩹 fixes in chart.yaml | updates in UI configmap

7be67a1

Signed-off-by: Krishna Murti <[email protected]>

updated videoqna-ui values filename

3cf4f29

Signed-off-by: Krishna Murti <[email protected]>

Merge branch 'main' into helm/videoqna

0e37189

🔥 Updates based on lvm-uservice from visualqna

d13bdcb

Signed-off-by: Krishna Murti <[email protected]>

Merge branch 'main' into helm/videoqna

c932c31

[pre-commit.ci] auto fixes from pre-commit.com hooks

a993f1a

for more information, see https://pre-commit.ci

🩹 fix videoqna new dependency values for lvm-uservice

773922e

Signed-off-by: Krishna Murti <[email protected]>

mkbhanda requested changes Nov 4, 2024

View reviewed changes

yongfengdu requested changes Nov 5, 2024

View reviewed changes

krish918 added 2 commits November 6, 2024 10:32

lvm-serving rename | replaced cachedir with modeldir

4bd8b04

Signed-off-by: Krishna Murti <[email protected]>

reverted to INDEX_NAME key

91f96ec

Signed-off-by: Krishna Murti <[email protected]>

yongfengdu reviewed Nov 6, 2024

View reviewed changes

lianhao added this to the v1.1 milestone Nov 8, 2024

yongfengdu approved these changes Nov 15, 2024

View reviewed changes

chensuyue removed this from the v1.1 milestone Nov 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Helm Chart for VideoQnA Application #497

✨ Helm Chart for VideoQnA Application #497

krish918 commented Oct 24, 2024

lianhao left a comment

lianhao Oct 25, 2024

krish918 Nov 4, 2024

lianhao Oct 25, 2024

krish918 Nov 4, 2024

lianhao Oct 25, 2024

krish918 Nov 4, 2024

lianhao Oct 25, 2024

krish918 Nov 4, 2024

krish918 Nov 6, 2024

lianhao Oct 25, 2024

krish918 Nov 4, 2024

lianhao Nov 1, 2024

krish918 Nov 1, 2024

lianhao Nov 1, 2024 •

edited

Loading

krish918 Nov 4, 2024

mkbhanda left a comment

mkbhanda Nov 4, 2024

krish918 Nov 6, 2024

mkbhanda Nov 4, 2024

krish918 Nov 6, 2024

mkbhanda Nov 4, 2024

krish918 Nov 6, 2024

yongfengdu Nov 5, 2024

krish918 Nov 6, 2024

yongfengdu Nov 6, 2024

krish918 Nov 6, 2024

yongfengdu Nov 6, 2024

lianhao commented Nov 8, 2024

yongfengdu Nov 6, 2024

chensuyue commented Nov 18, 2024

	After you've deployted the tgi chart successfully, please run `kubectl get svc` to get the tgi service endpoint, i.e. `http://tgi`.
	After you've deployed the tgi chart successfully, please run `kubectl get svc` to get the tgi service endpoint, i.e. `http://tgi`.

✨ Helm Chart for VideoQnA Application #497

Are you sure you want to change the base?

✨ Helm Chart for VideoQnA Application #497

Conversation

krish918 commented Oct 24, 2024

Description

Issues

Type of change

Dependencies

Tests

lianhao 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

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

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

lianhao Nov 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mkbhanda 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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lianhao commented Nov 8, 2024

Choose a reason for hiding this comment

chensuyue commented Nov 18, 2024

lianhao Nov 1, 2024 •

edited

Loading