feat: Implement enterprise-grade Kubernetes deployment with Helm charts#191
Merged
Conversation
…loyment automation Introduces Helm chart support for the chat-service microservice and a centralized Makefile for managing deployments across all services, with comprehensive production hardening. Address #98 Changes Made: - Created Helm chart for chat-service under helm/chat-service/ - Defined Kubernetes manifests for Deployment, Service, and Helm NOTES.txt - Introduced values.yaml with configurable ports, replica count, image name, and probes - Added centralized Makefile with install, upgrade, uninstall, status, and lint for individual charts - Included install-all, upgrade-all, uninstall-all for batch operations - Added configurable port-forward target - Included examples and usage notes via make help - Added production-ready resource limits (200m CPU, 256Mi memory) - Configured security hardening (non-root user, read-only filesystem, dropped capabilities) - Fixed environment variable injection in deployment template - Enabled autoscaling with realistic limits (2-10 replicas) - Added comprehensive health checks (startup, liveness, readiness probes) - Added local image repository setup documentation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Use helm upgrade --install for robust deployment in Makefile - Fix image repository name (chat_service -> chat-service) following K8s conventions - Remove hardcoded image tag, use Chart.yaml appVersion instead - Fix hardcoded ports in NOTES.txt to use dynamic service port values - Set pullPolicy to Never for offline production environments - Add missing startupProbe to deployment template - Improve chart portability and offline deployment readiness 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…ables - Remove OPENAI_API_KEY from values.yaml to prevent secrets in git - Add Secret template for secure credential management - Update deployment to use secretKeyRef for API key injection - Add comprehensive README with security best practices - Maintain backward compatibility with optional secret creation - Eliminate security vulnerability of plaintext secrets in version control 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix inconsistent port examples (8080 -> 8000) in port-forward help - Update comment to reflect correct default port (8000:8000) - Align examples with actual chat-service port configuration 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix ifndef CHART_NAME check that never triggered due to default value - Use ifeq to properly detect when CHART_NAME is still set to default - Remove trailing spaces from CHART_NAME default value - Prevent accidental port-forward attempts to non-existent example-service - Ensure users get clear error message when CHART_NAME not specified 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Change all health probes to use 'port: http' instead of hardcoded 8000 - Makes chart robust to service.port configuration changes - Update install-all and upgrade-all to use 'helm upgrade --install' - Ensures batch operations are idempotent and won't fail on re-runs - Add --create-namespace to upgrade-all for consistency - Improve overall chart reliability and user experience 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
… TCP check - Replace basic TCP connectivity test with HTTP health endpoint verification - Add --spider flag to check URL without downloading content - Add 10-second timeout to prevent test hanging - Provides more robust service health validation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…mage loading - Add CLAUDE.md with development guidance and architectural overview - Enhance Makefile with multi-environment support (dev/staging/prestaging/prod) - Create automated image loading script (load-images.sh) for CRC registry - Add comprehensive chat-service Helm chart with production-grade features: * Multi-environment values files (prestaging, staging, prod) * Enterprise templates: NetworkPolicy, PodDisruptionBudget, ResourceQuota, ServiceMonitor * Enhanced deployment with LLM configuration and security contexts * Proper secret management and environment variable handling - Update LOCAL_IMAGE_REPOSITORY.md with automated workflow documentation - Configure prestaging environment for external vLLM service connectivity - Add sample images.txt for batch image loading Successfully tested prestaging deployment with chat-service connecting to external vLLM. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
### Summary Standardize all Helm charts with identical structure, add production-ready templates, and implement shared values system for centralized configuration management across staging, pre-staging, and production environments. ### Major Changes - ✅ Standardize all 4 service charts with identical template coverage - ✅ Add production templates: NetworkPolicy, PodDisruptionBudget, ServiceMonitor, NOTES.txt - ✅ Create shared values system (helm/shared-values/) for centralized config management - ✅ Remove dev environment (development uses docker-compose, not Helm) - ✅ Update Makefile with shared values support and improved documentation - ✅ Add template coverage: ingress, hpa, tests for all services - ✅ Standardize naming: persistentvolumeclaim.yaml → pvc.yaml ### Benefits - 🏢 Enterprise-grade configuration management (Fortune 500 standards) - 🔄 Single point of control for cross-service configuration changes - 🔒 Production-ready security, monitoring, and high availability - 📊 Environment-specific scaling and feature management 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update images.txt with Docker Hub standard services (nginx, redis, minio, chromadb) - Fix load-images.sh to handle Docker Hub URL formats (3 and 4-component paths) - Configure staging values to use local CRC registry for service images - Add comprehensive Helm charts for all OmniPDF services - Update CLAUDE.md with enhanced deployment documentation and shared values system - Successfully deploy and test chat-service health endpoint 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix malformed template escaping in LoadBalancer service IP extraction - Update all affected charts: nginx, pdf-extraction-service, pdf-renderer-service, redis - Ensure proper Helm template syntax for nested kubectl commands - All charts now pass helm lint validation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update Makefile to exclude 'assets' directory from install-all and uninstall-all targets - Prevent treating non-chart directories as Helm charts - Assets directory contains shared resources (logos, etc.) not chart definitions - Maintains existing exclusion for 'shared-values' directory - Fixes CI lint failures for helm/assets/ directory 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add 'make lint-all' command to lint all charts with proper exclusions - Automatically skip non-chart directories (assets, shared-values) - Update help documentation with new command - Provides CI-friendly solution for bulk chart linting - Ensures consistency with install-all and uninstall-all exclusion logic 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove incorrect API key environment variable from nginx deployment - Clean up inappropriate LLM configurations from nginx and pdf-renderer services - Remove all resourceQuota configurations to prevent namespace conflicts - Fix chart icon paths to use correct relative path (file://../assets/logo.svg) - Refactor Makefile to eliminate duplicated logic in install-all/upgrade-all targets - Enhance update-shared-values.sh script with better yq fallback warnings - Update deprecated OpenShift CLI commands (oc whoami -t → oc whoami --show-token) - Verify image pull policy configuration is correct for different environments 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix cleaner service test to check deployment status instead of invalid HTTP endpoint - Remove copy-pasted chat-service configurations from multiple charts (headers, secrets, affinity) - **SECURITY**: Remove hardcoded MinIO credentials from Redis staging/prestaging values - **SECURITY**: Fix Redis charts with inappropriate MinIO environment variables - **SECURITY**: Convert staging/prestaging secrets to external management (create: false) - Improve update-shared-values.sh with robust YAML handling and type detection - Add proper validation for nested keys and missing keys in sed fallback 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update all service image repositories from GHCR to CRC internal registry - Set pullPolicy to Always across all Helm charts for latest image fetching - Add comprehensive CRC setup documentation and configuration scripts - Create config/crc/ directory with automated setup scripts and documentation - Update README.md with detailed CRC configuration instructions This change aligns the prestaging environment with offline production deployment patterns, ensuring all services pull from the internal registry as intended for airgapped environments. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Configure services with available CRC images to use internal registry with specific tags - chat-service: CRC registry with dev-v0.0.1-860e67e tag ✅ - embedder-service: CRC registry with dev-v0.0.1-860e67e tag ✅ - pdf-processor-service: CRC registry with dev-v0.0.1-860e67e tag ✅ - Revert services missing from CRC registry back to GHCR for now - pdf-extraction-service: back to GHCR (image not in CRC registry) - pdf-renderer-service: back to GHCR (image not in CRC registry) - docling-translation-service: back to GHCR (image not in CRC registry) This creates a working hybrid approach where services pull from CRC internal registry when images are available (prestaging simulation) and fall back to GHCR for missing images. Chat-service successfully demonstrates CRC registry integration is working. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update Helm lint workflow to skip non-chart directories (assets/, shared-values/) - Add proper Chart.yaml existence check before linting - Fix workflow that was failing due to attempting to lint directories without Chart.yaml The workflow now properly: - Skips helm/assets/ (contains logo and docs) - Skips helm/shared-values/ (contains shared YAML configs, not charts) - Only lints actual Helm charts with Chart.yaml files All charts now pass linting successfully. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Add CLAUDE.md to .gitignore to prevent accidental commits - Remove CLAUDE.md from git tracking while preserving local file - This ensures project instructions remain local to development environment 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Update docker-compose.yml service configurations - Refresh image references in helm/images.txt - Add services build configuration for CI/CD pipeline - Add OmniPDF monitoring dashboard for OpenShift console 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Redis health probes: Add conditional authentication support for redis-cli ping commands - Makefile port-forward: Replace fragile pod selection with robust deployment targeting - YAML type preservation: Fix yq command to preserve boolean/numeric types using strenv These fixes improve security, reliability, and configuration correctness across the Helm deployment system. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Fix deployment to use minio.pvcName helper instead of minio.fullname
- Ensures consistency between PVC creation (minio-data) and deployment reference
- Prevents pod startup failures due to PersistentVolumeClaim not found errors
PVC template creates: {release-name}-data
Deployment now references: {release-name}-data (was: {release-name})
🤖 Generated with [Claude Code](https://claude.ai/code)
Co-Authored-By: Claude <noreply@anthropic.com>
…arify istio-gateway RBAC status - Fix nginx configmap to use proper service routing matching nginx.conf.template - Remove invalid /api/ endpoint from helm nginx configuration - Update istio-gateway RBAC status to N/A (doesn't need RBAC as networking config) - Use helm template functions for dynamic service name resolution 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…uration - Remove localhost:8080 from Gateway hosts field (invalid for Istio) - Remove localhost:8080 from VirtualService hosts field - Keep only valid DNS name: omnipdf-prestaging.apps-crc.testing 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove redundant /api/ location block that duplicated /pdf_processor/ functionality - Simplify routing to use only /pdf_processor/ prefix for API requests - Maintains consistent routing pattern with existing configuration 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
/gemini review |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request is a significant and impressive infrastructure overhaul, migrating the OmniPDF application to a production-ready Kubernetes deployment using Helm. The changes are extensive, introducing comprehensive Helm charts, security hardening with non-root users and fine-grained RBAC, observability via Prometheus metrics, and automated deployment scripts. The documentation is also greatly improved. My review focuses on ensuring the correctness and maintainability of this new infrastructure. I've identified a critical RBAC misconfiguration that would lead to runtime failures, a bug in the Makefile, and opportunities to improve script robustness and documentation accuracy. Overall, this is excellent work that lays a solid foundation for the project's scalability and reliability.
- Resolve merge conflicts in pdf_processor_service - Keep Prometheus instrumentation from helm branch - Add new routers from dev branch (chat, metadata, wordcloud, extractor) - Fix corrupted requirements.txt and add openai dependency - Maintain nginx proxy timeout configurations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This is an impressive and comprehensive pull request that lays a solid foundation for deploying OmniPDF on Kubernetes. The introduction of Helm charts for all services, detailed documentation, security hardening with non-root users and NetworkPolicies, and the addition of enterprise-grade features like HPA and ServiceMonitors are all excellent improvements. The Makefile and helper scripts also significantly improve the developer experience.
However, there are a few critical issues that need to be addressed before this can be merged. The RBAC configuration in helm/rbac/values.yaml is incomplete and inconsistent with the architecture, which will lead to runtime permission errors. The Nginx Helm chart has a template error that will prevent it from deploying. There are also some smaller issues in the Makefile and documentation that should be corrected for consistency and correctness.
My review comments provide specific details and suggestions for fixing these issues. Once they are addressed, this will be a fantastic enhancement to the project.
- Create standardized values.yaml files for all 15 services based on prestaging configurations - Fix template reference errors in nginx configmap (use configurable service names) - Add missing service, ingress, podDisruptionBudget, and service-specific configurations - Ensure all charts pass helm lint validation Services updated: - API services (8000): chat, docling-translation, embedder, pdf-extraction, metadata, pdf-processor, pdf-renderer, image-captioner - Infrastructure: nginx (8080), redis (6379), minio (9000), chromadb (8000), frontend (8501), cleaner - Gateway: rbac configuration All 16 Helm charts now lint successfully with proper default values. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
Owner
Author
|
/gemini review |
Contributor
There was a problem hiding this comment.
Code Review
This is an impressive pull request that introduces a comprehensive, enterprise-grade Kubernetes deployment system using Helm. The changes are extensive and well-structured, covering multi-environment support, security hardening, monitoring, and automation. The use of Helm charts for each service, detailed documentation, and automation scripts like the Makefile and load-images.sh demonstrate a thorough approach to infrastructure as code. My review focuses on improving consistency, maintainability, and correctness in a few key areas.
…ractions - Fix C4 diagram Redis connections: embedder job polling, pdf_renderer file management - Add missing RBAC permissions for Redis/MinIO access in 5 services - Align architecture documentation with actual service implementations 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- nginx: Remove direct service calls, routes to istio-gateway (different namespace) - frontend: Call pdf-processor-service directly instead of nginx - Prevents runtime service discovery permission errors 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Ensures consistency with lint-all and uninstall-all functions - Prevents deployment failures if shared-values directory is added - Maintains proper Helm chart directory filtering 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove outdated claims about missing permissions - All services already have complete RBAC data store access - Update service coverage count from 15/15 to 14/14 - Replace "Required Fixes" with validation commands - Clarify that istio-gateway doesn't need RBAC (is configuration, not service) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove namespace specification from Gateway template to use release namespace - Remove namespace key from values-prestaging.yaml - Follows best practice of deploying Gateway resources in application namespace - Eliminates need for elevated permissions in istio-system namespace 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…ared values references - Add default values.yaml for istio-gateway chart to fix helm lint error - Remove unused shared values logic from deploy-helm-charts.sh script - Clean up references to non-existent helm/shared-values directory 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
…tion - Change example from chat_service to chat-service to match project convention - Maintain consistency with hyphenated service names used throughout documentation 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
… Gateway duplication - Fix misleading 'persistentvolumes' key in RBAC values.yaml - rename to 'persistentvolumeclaims' to match actual Kubernetes resource type being accessed (PVCs not PVs) - Update RBAC template condition and variable naming for consistency - Modify Istio Gateway values.yaml to follow same duplication pattern as other services (prestaging config as default with minimal additions) 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
- Remove shared-values references from Makefile (3 locations), GitHub workflow, and scripts - Delete obsolete update-shared-values.sh script (shared-values directory no longer exists) - Fix helm lint command in README to use proper find syntax excluding only assets - Update chart version script to only skip assets directory The shared-values directory was removed in previous commits but references remained. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR implements a comprehensive Kubernetes deployment system for OmniPDF using Helm charts, enabling production-ready deployment across multiple environments (prestaging, staging, production). The implementation includes enterprise-grade features like security hardening, monitoring, and automated deployment workflows.
Changes Made
Context / Rationale
This is a major infrastructure enhancement that transforms OmniPDF from a Docker Compose-only system to a production-ready Kubernetes application. The changes enable:
The implementation follows Kubernetes best practices and enterprise deployment patterns, making OmniPDF suitable for production environments.
Related Docs or References
helm/SECRET-MANAGEMENT.md: Comprehensive guide for Kubernetes Secretshelm/LOCAL_IMAGE_REPOSITORY.md: Local development and CRC setup guideconfig-crc/README.md: Red Hat CodeReady Containers configurationMakefile: Automated deployment and management commands.github/workflows/helm.yml: CI/CD integration for Helm chartsFastAPI Application Checklist (Delete if PR is not relevant)
This PR is infrastructure-focused and does not modify FastAPI applications directly.
General Checklist
helm lintvalidation🎉 Generated with Claude Code