Skip to content

[Metro AI Suite] Live Video Captioning Helm Chart#2351

Open
yogeshmpandey wants to merge 41 commits intoopen-edge-platform:mainfrom
yogeshmpandey:lvc_helm_chart
Open

[Metro AI Suite] Live Video Captioning Helm Chart#2351
yogeshmpandey wants to merge 41 commits intoopen-edge-platform:mainfrom
yogeshmpandey:lvc_helm_chart

Conversation

@yogeshmpandey
Copy link
Copy Markdown
Contributor

Description

Helm chart updates for Live Video Captioning.

Any Newly Introduced Dependencies

N/A

How Has This Been Tested?

Tested to on local k8s setup

Checklist:

  • I agree to use the APACHE-2.0 license for my code changes.
  • I have not introduced any 3rd party components incompatible with APACHE-2.0.
  • I have not included any company confidential information, trade secret, password or security token.
  • I have performed a self-review of my code.

yogeshmpandey and others added 30 commits February 18, 2026 10:56
…ate Docker image and improve model download script with weight format options
- Introduced helper functions to build pipeline parameters and payloads for starting runs.
- Updated the start_run endpoint to utilize the new helper functions for cleaner code.
- Added tests to ensure optional pipeline parameters are correctly forwarded to the server.
- Created AGENTS.md for project documentation and guidelines.
…pdate documentation for improved clarity on deployment requirements and known issues.
{{/*
Copyright (C) 2026 Intel Corporation
SPDX-License-Identifier: Apache-2.0
*/}}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thought: Collector helm here or with the metrics?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I have decoupled it to individual subcharts.

*/}}

{{- define "collector.name" -}}
{{- default .Chart.Name .Values.nameOverride | trunc 63 | trimSuffix "-" }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

add | lower?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added | lower to *.name, *.fullname, and *.chart template functions to enforce Kubernetes lowercase naming requirements.

app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
{{- end }}
app.kubernetes.io/managed-by: {{ .Release.Service }}
app.kubernetes.io/part-of: live-video-captioning
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we parameterize this "part-of" in case of multiple other consumers of this microservice?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Replaced hardcoded app.kubernetes.io/part-of: live-video-captioning.

interval = "10s"
[[inputs.execd]]
command = ["python3", "/app/qmassa_reader.py"]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same comment applies in other places. Assumed to exist right? What if it is not there?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added comments clarifying that read_cpu_freq.sh and qmassa_reader.py are bundled inside the collector container image

path: {{ .Values.collectorSignalsHostPath | quote }}
type: DirectoryOrCreate
- name: host-sys
hostPath:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

assuming privileged mounting is required given it is telemetry

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes

replicas: {{ .Values.replicaCount }}
strategy:
type: Recreate
rollingUpdate: null
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Needed?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes, with RollingUpdate, Kubernetes tries to start the new pod before terminating the old one. That would fail immediately with a port-already-in-use error as we are running on host.
Recreate is the correct strategy here as the pod will terminate before the next is created.

ports:
- name: turn-udp
containerPort: {{ .Values.service.port }}
hostPort: {{ .Values.service.nodePort }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

replica > 1 will fail right due to conflict on host port?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added guards in each deployment template, updated helm README also.

ports:
- name: metrics
containerPort: {{ .Values.service.port }}
hostPort: {{ .Values.service.nodePort }}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same issue with host port as stun server. check if applicable elsewhere

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Added guards in each deployment template, updated helm README also.

@yogeshmpandey
Copy link
Copy Markdown
Contributor Author

@bharagha @hteeyeoh Please check if this issues are addressed and PR can be approved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants