-
Notifications
You must be signed in to change notification settings - Fork 2
Add MySQL support to Helm chart #255
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
…mentation Co-authored-by: mfittko <326798+mfittko@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR implements MySQL as an optional database backend for Kubernetes deployments via Helm chart, following the existing PostgreSQL pattern. It enables both in-cluster MySQL (for development/testing) and external MySQL (for production) configurations.
Key Changes
- Adds complete MySQL support including StatefulSet, Service, and Secret templates
- Implements MySQL connection string generation with TLS support
- Adds comprehensive validation logic to prevent configuration conflicts
- Provides example values and documentation for MySQL deployments
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| deploy/helm/llm-proxy/values.yaml | Adds MySQL configuration section with auth, persistence, resources, and TLS options mirroring PostgreSQL structure |
| deploy/helm/llm-proxy/templates/mysql-statefulset.yaml | Defines single-replica MySQL StatefulSet with health probes, volume claims, and configurable resources |
| deploy/helm/llm-proxy/templates/mysql-service.yaml | Creates headless ClusterIP service on port 3306 for stable StatefulSet network identity |
| deploy/helm/llm-proxy/templates/mysql-secret.yaml | Manages MySQL credentials with conditional rendering based on existingSecret configuration |
| deploy/helm/llm-proxy/templates/deployment.yaml | Integrates MySQL password injection and DATABASE_URL construction into proxy deployment |
| deploy/helm/llm-proxy/templates/_helpers.tpl | Adds MySQL helper functions for connection URL generation and comprehensive configuration validation |
| deploy/helm/llm-proxy/examples/values-mysql.yaml | Provides annotated example configuration for in-cluster MySQL deployment |
| deploy/helm/llm-proxy/README.md | Documents both external (production) and in-cluster (development) MySQL deployment patterns |
| deploy/helm/llm-proxy/Chart.yaml | Adds MySQL support annotations indicating build tag requirements |
| livenessProbe: | ||
| exec: | ||
| command: | ||
| - mysqladmin | ||
| - ping | ||
| - -h | ||
| - localhost | ||
| initialDelaySeconds: 30 | ||
| periodSeconds: 10 | ||
| timeoutSeconds: 5 | ||
| failureThreshold: 3 |
Copilot
AI
Jan 2, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mysqladmin ping command requires authentication when a root password is set. The probe will fail because no password is provided. Add the -u root and -p${MYSQL_ROOT_PASSWORD} arguments to the probe commands to fix authentication issues.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in cb06290. Added -u root -p${MYSQL_ROOT_PASSWORD} authentication to the liveness probe command using shell execution.
|
@copilot fix the comments yourself. |
- Add authentication to mysqladmin ping probes (requires -u root -p$MYSQL_ROOT_PASSWORD) - Remove invalid MYSQL_* environment variables (only valid as args) - Fix TLS parameter in connection string (omit when disabled, not tls=false) Co-authored-by: mfittko <326798+mfittko@users.noreply.github.com>
mfittko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comprehensive Review: Add MySQL Support to Helm Chart
Summary
This PR adds comprehensive MySQL support to the llm-proxy Helm chart, enabling users to deploy with MySQL as an alternative database backend alongside SQLite and PostgreSQL. The implementation addresses issue #244 as part of the MySQL epic (#240).
✅ Strengths
1. Complete Template Set
The PR provides a full set of Kubernetes resources mirroring the PostgreSQL pattern:
mysql-statefulset.yaml(105 lines) - StatefulSet with proper volume managementmysql-service.yaml(20 lines) - Headless ClusterIP service for stable network identitymysql-secret.yaml(21 lines) - Secure password storage with base64 encoding
2. Comprehensive Helper Functions (_helpers.tpl)
126 lines of well-structured helper functions:
llm-proxy.mysql.host,llm-proxy.mysql.port,llm-proxy.mysql.databasellm-proxy.mysql.secretName,llm-proxy.mysql.secretKey,llm-proxy.mysql.rootSecretKeyllm-proxy.mysql.url- Constructs the correct MySQL DSN formatllm-proxy.validateMysqlConfig- Comprehensive validation with clear error messages
3. Configuration Validation
The validation logic in _helpers.tpl is excellent:
{{- if and $hasInCluster $hasExternal }}
{{- fail "Configuration error: Cannot use both in-cluster MySQL (mysql.enabled=true) and external MySQL (secrets.databaseUrl) at the same time." }}
{{- end }}This prevents misconfigurations and provides actionable error messages.
4. Proper TLS Handling
The TLS connection string logic is correct after the fix:
- Disabled: No
tlsparameter (nottls=falsewhich is invalid) - Enabled:
&tls=true - Skip verify:
&tls=skip-verify
5. Authenticated Health Probes
The StatefulSet uses proper authenticated probes:
command:
- sh
- -c
- mysqladmin ping -h localhost -u root -p${MYSQL_ROOT_PASSWORD}This addresses the initial review feedback and ensures probes work when MySQL is password-protected.
6. Well-Documented Example
The examples/values-mysql.yaml (128 lines) provides a complete, commented example for development deployments with clear "CHANGE-ME" placeholders.
7. MySQL Config via Args (Not Env Vars)
Correctly passes MySQL configuration as command-line arguments:
args:
{{- if .Values.mysql.extraConfig.maxConnections }}
- --max-connections={{ .Values.mysql.extraConfig.maxConnections }}
{{- end }}This addresses the feedback that MYSQL_MAX_CONNECTIONS, MYSQL_CHARACTER_SET_SERVER, etc. are not valid environment variables for the official MySQL image.
📝 Observations (Non-Blocking)
1. README TLS Documentation
The README mentions tls=skip-verify as an option:
For production, enable TLS with
tls=trueortls=skip-verify(not recommended)
While the "(not recommended)" note is present, consider being more explicit about the security implications of skipping verification.
2. Single-Replica StatefulSet
The MySQL StatefulSet is hardcoded to replicas: 1:
replicas: 1This is appropriate for simple deployments but may warrant a note that for production HA, users should use an external MySQL cluster (Aurora, RDS, etc.).
3. Storage Class Handling
The special handling for storageClass: "-" to use an empty storageClassName is good:
{{- if (eq "-" .Values.mysql.persistence.storageClass) }}
storageClassName: ""
{{- else }}
storageClassName: {{ .Values.mysql.persistence.storageClass | quote }}
{{- end }}This allows users to explicitly request no storage class.
🔍 Previous Review Comments (All Resolved)
| Issue | Status |
|---|---|
| mysqladmin ping missing authentication | ✅ Fixed (cb06290) |
TLS false invalid syntax |
✅ Fixed (cb06290) |
| Invalid MySQL environment variables | ✅ Fixed (cb06290) |
| Readiness probe auth | ✅ Fixed (cb06290) |
All 4 review threads from the automated reviewer have been addressed.
📁 Files Changed Summary
| File | Lines | Purpose |
|---|---|---|
templates/mysql-statefulset.yaml |
+105 (new) | MySQL StatefulSet with PVC |
templates/_helpers.tpl |
+126 | MySQL helper functions |
examples/values-mysql.yaml |
+128 (new) | Example configuration |
values.yaml |
+80 | MySQL configuration section |
README.md |
+61 | MySQL documentation |
templates/mysql-secret.yaml |
+21 (new) | Password secret |
templates/mysql-service.yaml |
+20 (new) | Headless service |
templates/deployment.yaml |
+11/-1 | MySQL env vars integration |
Chart.yaml |
+4 | MySQL annotations |
Total: +556 additions, -1 deletion across 9 files
🧪 Testing
The PR description indicates template rendering tests passed:
- ✅ MySQL StatefulSet renders with authenticated probes
- ✅ MySQL Service renders as headless ClusterIP
- ✅ MySQL Secret renders with base64 encoded passwords
- ✅ DATABASE_URL correctly formatted with/without TLS
- ✅ Configuration validation catches misconfigured deployments
Verdict: APPROVE ✅
This PR is ready to merge. It provides:
- Complete MySQL Helm support following the established PostgreSQL pattern
- Proper validation with helpful error messages
- Correct TLS handling (fixed after review)
- Authenticated health probes (fixed after review)
- Comprehensive documentation with examples
- All previous review comments resolved
The implementation is consistent with existing chart patterns and provides a solid foundation for MySQL deployments.
MySQL Helm Chart Support - COMPLETE ✅
This PR adds comprehensive MySQL support to the llm-proxy Helm chart, enabling users to deploy with MySQL as an alternative database backend alongside the existing SQLite and PostgreSQL options.
Implementation Summary
Phase 1: Values Configuration ✅
database.mysqlsection to values.yaml with image, auth, persistence, resources, and config optionsPhase 2: MySQL Templates ✅
Phase 3: Helper Functions ✅
Phase 4: Proxy Integration ✅
Phase 5: Documentation & Examples ✅
Phase 6: Validation & Testing ✅
Recent Fixes
Probe Authentication
-u root -p${MYSQL_ROOT_PASSWORD}to mysqladmin ping commandsTLS Connection String
tlsparameter when disabled (was incorrectly usingtls=false)&tls=true, skip-verify =&tls=skip-verifyEnvironment Variables
MYSQL_MAX_CONNECTIONS,MYSQL_CHARACTER_SET_SERVER,MYSQL_COLLATION_SERVERenv varsFiles Changed (9 files, +569/-1)
New Files (4)
deploy/helm/llm-proxy/templates/mysql-statefulset.yaml- MySQL StatefulSet with persistencedeploy/helm/llm-proxy/templates/mysql-service.yaml- Headless ClusterIP servicedeploy/helm/llm-proxy/templates/mysql-secret.yaml- Password managementdeploy/helm/llm-proxy/examples/values-mysql.yaml- Example configurationModified Files (5)
deploy/helm/llm-proxy/values.yaml- Added mysql configuration sectiondeploy/helm/llm-proxy/templates/_helpers.tpl- Added MySQL helper functionsdeploy/helm/llm-proxy/templates/deployment.yaml- Added MySQL env vars and validationdeploy/helm/llm-proxy/README.md- Added MySQL documentationdeploy/helm/llm-proxy/Chart.yaml- Added MySQL annotationsTesting Results - All Passed ✅
Template Rendering Tests
user:$(MYSQL_PASSWORD)@tcp(host:3306)/db?parseTime=trueuser:$(MYSQL_PASSWORD)@tcp(host:3306)/db?parseTime=true&tls=trueConfiguration Tests
tls=false)Key Features
Installation Examples
Development (In-Cluster MySQL)
Production (External MySQL)
kubectl create secret generic llm-proxy-db \ --from-literal=DATABASE_URL="user:pass@tcp(host:3306)/db?parseTime=true&tls=true" helm install llm-proxy deploy/helm/llm-proxy \ --set image.repository=ghcr.io/sofatutor/llm-proxy \ --set image.tag=latest \ --set env.DB_DRIVER=mysql \ --set secrets.managementToken.existingSecret.name=llm-proxy-secrets \ --set secrets.databaseUrl.existingSecret.name=llm-proxy-dbOriginal prompt
This section details on the original issue you should resolve
<issue_title>[MySQL] Helm Chart Support</issue_title>
<issue_description>## Summary
Update Helm charts to support MySQL deployment as an alternative to PostgreSQL, including configuration values, templates, and deployment documentation.
Parent Epic
Part of #240 - Add MySQL as Optional Database Backend
Kubernetes Architecture
flowchart TB subgraph HelmChart["Helm Chart: llm-proxy"] subgraph Proxy["LLM Proxy Deployment"] DEP[Deployment] SVC[Service] CM[ConfigMap] SEC[Secret] end subgraph Database["Database Layer"] subgraph SQLite["SQLite - default"] PVC_LITE[PersistentVolumeClaim] end subgraph PostgreSQL["PostgreSQL - optional"] PG_SS[StatefulSet] PG_SVC[Service] PG_SEC[Secret] PG_PVC[PVC Template] end subgraph MySQL["MySQL - NEW optional"] MY_SS["StatefulSet NEW"] MY_SVC["Service NEW"] MY_SEC["Secret NEW"] MY_PVC["PVC Template NEW"] end end end DEP -->|DB_DRIVER=sqlite| PVC_LITE DEP -->|DB_DRIVER=postgres| PG_SVC DEP -->|DB_DRIVER=mysql| MY_SVCDetailed Task Breakdown
Phase 1: Values Configuration
database.mysqlsection to valuesvalues.yamlvalues.yamlvalues.yamlvalues.yamlvalues.yamlvalues.yamlPhase 2: MySQL StatefulSet Template
templates/mysql-statefulset.yamltemplates/mysql-statefulset.yamltemplates/mysql-statefulset.yamltemplates/mysql-statefulset.yamltemplates/mysql-statefulset.yamltemplates/mysql-statefulset.yamlPhase 3: MySQL Service Template
templates/mysql-service.yamltemplates/mysql-service.yamltemplates/mysql-service.yamltemplates/mysql-service.yamlPhase 4: Secrets Management
templates/mysql-secret.yamltemplates/mysql-secret.yamltemplates/mysql-secret.yamltemplates/mysql-secret.yamlPhase 5: Proxy Integration
templates/deployment.yamltemplates/deployment.yamltemplates/deployment.yamltemplates/_helpers.tplPhase 6: Example and Documentation
examples/values-mysql.yamlREADME.mdREADME.mdValues Structure
Database Section Hierarchy
flowchart TB subgraph Values["values.yaml"] DB[database] DB --> TYPE["type: sqlite/postgres/mysql"] DB --> MYSQL[mysql] MYSQL --> ENABLED["enabled: false"] MYSQL --> IMAGE[image] MYSQL --> AUTH[auth] MYSQL --> PERSIST[persistence] MYSQL --> RES[resources] MYSQL --> EXTRA[extraConfig] IMAGE --> REPO["repository: mysql"] IMAGE --> TAG["tag: 8.0"] AUTH --> ROOT["rootPassword"] AUTH --> DBNAME["database: llmproxy"] AUTH --> USER["username: llmproxy"] AUTH --> PASS["password"] PERSIST --> P_EN["enabled: true"] PERSIST --> P_SC["storageClass"] PERSIST --> P_SZ["size: 10Gi"] endTemplate Conditional Logic
flowchart TD START[Helm Install] CHECK{database.type?} CHECK -->|mysql| MYSQL_PATH CHECK -->|other| OTHER_PATH subgraph MYSQL_PATH["MySQL Templates Rendered"] RENDER_SS[mysql-statefulset.ya... </details> <!-- START COPILOT CODING AGENT SUFFIX --> - Fixes sofatutor/llm-proxy#244 <!-- START COPILOT CODING AGENT TIPS --> --- 💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more [Copilot coding agent tips](https://gh.io/copilot-coding-agent-tips) in the docs.