✨ Helm Chart for VideoQnA Application by krish918 · Pull Request #497 · opea-project/GenAIInfra

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 <ze.pan@intel.com> * debug Signed-off-by: ZePan110 <ze.pan@intel.com> * debug2 Signed-off-by: ZePan110 <ze.pan@intel.com> * Fix issue Signed-off-by: ZePan110 <ze.pan@intel.com> * Remove debug output Signed-off-by: ZePan110 <ze.pan@intel.com> * test Signed-off-by: ZePan110 <ze.pan@intel.com> * Fix issue. Signed-off-by: ZePan110 <ze.pan@intel.com> * Fix issue Signed-off-by: ZePan110 <ze.pan@intel.com> * Restore test file Signed-off-by: ZePan110 <ze.pan@intel.com> --------- Signed-off-by: ZePan110 <ze.pan@intel.com> Signed-off-by: Krishna Murti <krishna.murti@intel.com>

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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

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

@@ -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

@@ -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

@@ -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


 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

@@ -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 <krishna.murti@intel.com>

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

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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

+  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.

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

mkbhanda

Persistent volume support. And a typo.

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

+
+```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

+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


 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.

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

          resources:
            {{- toYaml .Values.resources | nindent 12 }}
      volumes:
+        {{- if .Values.global.cacheUseHostPath }}


See comment above.

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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

+              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

+              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 <krishna.murti@intel.com>

✅ Added helm tests for all services | linting fixes

1f121e9

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

✏ typo fix

418edb7

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

🛂 running vdms as root to fix errors

24b4e3f

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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

880e406

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

⚡Updated and fixed several tests, values during integration

0110b85

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

🔥 removed hardcoded extraArgs for some comps

56afc63

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

🔥 removed hardcoded extraArgs for some comps

1a5f762

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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 <krishna.murti@intel.com>

🔨 passed nginx proxy endpoints to UI using templates

a29691b

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

🔨 passed nginx proxy endpoints to UI using templates

e8f58bb

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

💚 added symbolic links to values files for CI

a8cbdbf

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

💚 added symbolic links to values files for CI

63bf7f6

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

📝 Added readme files for videoqna charts and subcharts

cf4360f

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

💚 Added missing symbolic links for some values files

2d02cf9

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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 <krishna.murti@intel.com>

💚 Added several CI fixes

fc17e30

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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 <krishna.murti@intel.com>

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 <krishna.murti@intel.com>

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 <krishna.murti@intel.com>

updated videoqna-ui values filename

3cf4f29

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

Merge branch 'main' into helm/videoqna

0e37189

🔥 Updates based on lvm-uservice from visualqna

d13bdcb

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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 <krishna.murti@intel.com>

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 <krishna.murti@intel.com>

reverted to INDEX_NAME key

91f96ec

Signed-off-by: Krishna Murti <krishna.murti@intel.com>

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


		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`.

	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`.

Conversation

krish918 commented Oct 24, 2024

Description

Issues

Type of change

Dependencies

Tests

Uh oh!

lianhao left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lianhao Nov 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mkbhanda left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lianhao commented Nov 8, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lianhao Nov 1, 2024 •

edited

Loading