Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new Helm chart for deploying compute nodes that can be used by VLM nodes, VPS nodes, and other future compute node types. The chart provides a complete Kubernetes deployment solution with server, UI, database, and storage components.
- Creates a comprehensive Helm chart with support for PostgreSQL database, persistent storage, and ingress configuration
- Implements separate deployments for server (StatefulSet) and UI (Deployment) components with independent scaling
- Provides configurable security settings, resource management, and environment variable customization
Reviewed Changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| charts/compute-node/Chart.yaml | Defines chart metadata, version, and PostgreSQL dependency |
| charts/compute-node/values.yaml | Provides comprehensive configuration options for all chart components |
| charts/compute-node/templates/_helpers.tpl | Contains helper templates for resource naming and label generation |
| charts/compute-node/templates/configmap.yaml | Creates ConfigMap with application configuration |
| charts/compute-node/templates/secret.yaml | Manages secrets for app keys and database connection |
| charts/compute-node/templates/serviceaccount.yaml | Creates ServiceAccount for pod authentication |
| charts/compute-node/templates/persistentvolumeclaim.yaml | Provisions shared storage for server and worker components |
| charts/compute-node/templates/statefulset.yaml | Deploys server component as StatefulSet with persistent storage |
| charts/compute-node/templates/ui-deployment.yaml | Deploys UI component as standard Deployment |
| charts/compute-node/templates/services.yaml | Creates services for server and UI components |
| charts/compute-node/templates/ingress.yaml | Configures ingress routing for external access |
| charts/compute-node/scripts/pre-install.sh | Test preparation script using dummy images |
| charts/compute-node/README.md | Documentation with architecture diagrams and usage instructions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| metadata: | ||
| name: {{ include "compute-node.ingressName" . }} | ||
| labels: | ||
| {{- include "compute-node.labels" . | nindent 4 }} |
There was a problem hiding this comment.
The ingress uses 'compute-node.labels' but should use 'compute-node.nodeLabels' to be consistent with other resources in the chart that use node-specific labels.
| {{- include "compute-node.labels" . | nindent 4 }} | |
| {{- include "compute-node.nodeLabels" . | nindent 4 }} |
There was a problem hiding this comment.
Was there some reason we did this? @shuning-auki
| @@ -0,0 +1,72 @@ | |||
| {{- if .Values.server.enabled }} | |||
| apiVersion: apps/v1 | |||
| kind: StatefulSet | |||
There was a problem hiding this comment.
Why does the the compute node need to be a statefulset?
There was a problem hiding this comment.
server and worker need to access file system for job input
| {{- range $key, $value := .Values.server.env }} | ||
| - name: {{ $key }} | ||
| value: {{ $value | quote }} | ||
| {{- end }} |
There was a problem hiding this comment.
Can we support setting environment variable from secrets ? similar to how we do it in domain-server?
There was a problem hiding this comment.
|
I guess PR needs to be updated from https://github.com/aukilabs/vlm-node/tree/main/charts/vlm-node when that's stable. Actually it should be moved from there to here and only be referenced as a sub-chart. |
To be used by VLM nodes, VPS nodes and potentially other nodes in the future.