Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
cb1371e
feat: makefile + partial helm chart for chat-service
NotYuSheng Jul 2, 2025
371d562
feat: add Helm chart and Makefile support for chat-service
NotYuSheng Jul 9, 2025
0f3846b
fix(security): move api key to Kubernetes Secrets
NotYuSheng Jul 9, 2025
157e0ef
docs: change default port to 8000 as most services local port are on …
NotYuSheng Jul 9, 2025
a421562
docs: Remove error supression on "all" commands
NotYuSheng Jul 9, 2025
76bbc6f
Merge pull request #129 from NotYuSheng/dev
NotYuSheng Aug 5, 2025
59269a8
feat: introduce production-ready Helm chart for chat-service with dep…
Aug 5, 2025
0fe818d
fix: address code review feedback and improve chart reliability
Aug 5, 2025
f7bc8d8
security: implement Kubernetes Secrets for sensitive environment vari…
Aug 5, 2025
08849af
fix: correct port examples in Makefile help text
Aug 5, 2025
b67d611
fix: correct port-forward error handling in Makefile
Aug 5, 2025
c97c108
fix: make health probes use named ports and improve Makefile idempotency
Aug 6, 2025
5700cae
improve: enhance Helm test to query /health endpoint instead of basic…
Aug 6, 2025
63f8ac7
feat: implement comprehensive Helm deployment system with automated i…
Aug 11, 2025
3848d76
feat: Add Helm charts for chromadb, embedder-service, and minio services
Aug 11, 2025
77d4fdd
feat: Implement enterprise-grade Helm standardization with shared values
NotYuSheng Aug 11, 2025
3dbcd22
Merge branch 'fix/embedder-model-preload' into feat/helm-setup
Aug 12, 2025
55389d2
feat: enhance Helm deployment with local CRC registry integration
Aug 13, 2025
5c9b978
fix: resolve Helm template parsing errors in NOTES.txt files
Aug 13, 2025
edd9257
fix: exclude assets directory from Helm chart operations
Aug 13, 2025
5ee6aa7
feat: add lint-all target for comprehensive Helm chart validation
Aug 13, 2025
4f835df
fix: address comprehensive PR feedback for Helm chart standardization
Aug 13, 2025
079cc75
fix: address critical security and configuration issues from PR review
Aug 13, 2025
1279a79
feat: standardize image configurations for CRC prestaging environment
Aug 14, 2025
7ae7a81
fix: optimize image configurations for CRC/GHCR hybrid deployment
Aug 14, 2025
78db552
fix: helm lint workflow failing on non-chart directories
Aug 14, 2025
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions .github/workflows/helm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,17 @@ jobs:
- name: Lint all Helm charts in ./helm/
run: |
for chart in helm/*/; do
echo "Linting chart: $chart"
helm lint "$chart" || exit 1
# Skip directories that aren't Helm charts
if [[ "$chart" == "helm/assets/" || "$chart" == "helm/shared-values/" ]]; then
echo "Skipping non-chart directory: $chart"
continue
fi

# Only lint if Chart.yaml exists
if [[ -f "$chart/Chart.yaml" ]]; then
echo "Linting chart: $chart"
helm lint "$chart" || exit 1
else
echo "Skipping directory without Chart.yaml: $chart"
fi
done
174 changes: 174 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,174 @@
# CLAUDE.md

This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.

## Project Overview

OmniPDF is a microservices-based PDF analyzer with translation, summarization, captioning and RAG capabilities. The system consists of 8 main services orchestrated via Docker Compose and deployable with Helm/Kubernetes.

## Core Architecture

### Service Structure
- **pdf_processor_service** (port 8000): Main coordinator for processing and routing
- **chat_service** (port 8001): RAG-based conversational interface with LLM integration
- **pdf_extraction_service** (port 8002): Extracts tables and images from PDFs
- **docling_translation_service** (port 8003): Translates docling-style JSON content
- **embedder_service** (port 8005): Text chunking and embedding with ChromaDB storage
- **nginx** (port 8080): API gateway proxying file uploads
- **cleaner**: Background cleanup service

### Dependencies
- **Redis** (port 6379): Session management
- **ChromaDB** (port 5100): Vector storage for embeddings
- **MinIO** (ports 9000/9001): S3-compatible object storage
- **vLLM** (port 1234): LLM backend server

### Project Structure
```
├── {service_name}/ # Each service follows this pattern:
│ ├── main.py # FastAPI app with router includes
│ ├── routers/ # API endpoints (health.py always present)
│ ├── models/ # Pydantic models and business logic
│ ├── utils/ # Service-specific utilities
│ ├── Dockerfile # Container definition
│ └── example.env # Environment template
├── shared_utils/ # Common utilities across services
│ ├── openai_client.py # OpenAI/vLLM client wrapper
│ ├── chroma_client.py # ChromaDB async client
│ ├── redis.py # Redis connection utilities
│ └── s3_utils.py # MinIO/S3 operations
└── helm/ # Kubernetes deployment
└── chat-service/ # Example Helm chart structure
```

All services use FastAPI with consistent structure: main.py imports routers, routers handle endpoints, models contain business logic.

## Common Commands

### Development with Docker Compose
```bash
# Start all services
docker-compose up -d

# Start with GPU support (for vLLM backend)
docker-compose -f docker-compose.gpu.yml up -d

# View logs for specific service
docker-compose logs -f chat_service

# Rebuild and restart service
docker-compose up -d --build chat_service

# Stop all services
docker-compose down

# Stop and remove volumes (full cleanup)
docker-compose down -v
```

### Helm/Kubernetes Operations
```bash
# Get help with available commands
make help

# Install single service (defaults to dev environment)
make install CHART_NAME=chat-service ENV=staging

# Install all services
make install-all ENV=prod

# Upgrade service
make upgrade CHART_NAME=chat-service ENV=prod

# Check service status
make status CHART_NAME=chat-service

# Port forward to local machine
make port-forward CHART_NAME=chat-service LOCAL_PORT=8001 REMOTE_PORT=8000

# Lint Helm chart
make lint CHART_NAME=chat-service

# Uninstall service
make uninstall CHART_NAME=chat-service
```

### Environment Configuration
- Services use `.env` files (see `example.env` in each service directory)
- Helm uses environment-specific values: `values-{dev,staging,prestaging,prod}.yaml`
- Default namespace: `omnipdf`
- Use hyphens (not underscores) in chart names for Kubernetes compliance
- Shared values system provides consistent configuration across environments:
- Values applied in order: `common-base.yaml` → `common-{env}.yaml` → `{service}/values-{env}.yaml` → `{service}/values.yaml`

## Key Implementation Details

### Service Communication
- Services communicate via HTTP APIs using shared utilities
- All services expose `/health` endpoint for monitoring
- nginx serves as API gateway for external requests
- Internal service discovery uses container names in Docker Compose

### Data Flow
1. Files uploaded via nginx (8080) → pdf_processor_service (8000)
2. pdf_processor_service orchestrates other services as needed:
- pdf_extraction_service for tables/images
- docling_translation_service for content translation
- embedder_service for text processing
3. embedder_service chunks text → ChromaDB for vector storage
4. chat_service queries ChromaDB + LLM for RAG responses
5. Session data managed via Redis, file storage via MinIO

### Shared Utilities
- `openai_client.py`: Configurable OpenAI-compatible client (works with vLLM)
- `chroma_client.py`: Async ChromaDB client with environment-based config
- `redis.py`: Redis connection utilities for session management
- `s3_utils.py`: MinIO/S3 operations for file storage
- All shared utilities use environment variables for configuration

## Code Standards

### File Organization
- Follow the established `models/`, `routers/`, `utils/` structure within services
- Place cross-service utilities in `shared_utils/`
- Each service must have a health router

### Code Review Process
- All changes require peer review before merge
- Focus on correctness, clarity, structure, and security
- Use comment labels: `nit:`, `suggest:`, `blocking:`
- Final review by designated senior reviewer before merge

### Kubernetes Naming
- Use hyphens (not underscores) in resource names
- Follow RFC 1123 naming conventions
- Chart names should match service names with hyphens

## Testing and Quality

### Service Testing
- Each service should be tested independently
- Use the `/health` endpoints for basic connectivity testing
- Helm charts include test connections via `test-connection.yaml`
- E2E testing setup available in `cypress/` directory

### Development Dependencies
Each service uses minimal dependencies:
- **FastAPI** + **uvicorn** for web framework
- **OpenAI client** for LLM integration (compatible with vLLM)
- Service-specific requirements in each `{service}/requirements.txt`

### Port Assignments (Development Only)
Development ports are documented in README.md. Production deployments should use proper routing layers (Ingress, Service Mesh, etc.).

### Shared Values Management
- Use `scripts/update-shared-values.sh` to update configuration across environments
- Example: `./scripts/update-shared-values.sh "networkPolicy.enabled=true" staging`
- Helm shared values directory: `helm/shared-values/`
- Each service can override shared configuration in its own values files

### Branch and Environment Strategy
- Main development branch: `dev`
- Current feature branch: `feat/helm-setup`
- Environments: staging (default), prestaging, prod
- Use `make install ENV=prod` to specify non-default environment
179 changes: 179 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,179 @@
# Makefile for Helm chart management in OmniPDF

# Default namespace and configurable chart
NAMESPACE ?= omnipdf
CHART_NAME ?= example-service
CHART_DIR ?= helm/$(CHART_NAME)
ENV ?= staging

# Shared values configuration
SHARED_VALUES_DIR ?= helm/shared-values
SHARED_BASE_VALUES ?= $(SHARED_VALUES_DIR)/common-base.yaml
SHARED_ENV_VALUES ?= $(SHARED_VALUES_DIR)/common-$(ENV).yaml

# Service-specific values files
SERVICE_VALUES_FILE ?= $(CHART_DIR)/values.yaml
SERVICE_ENV_VALUES_FILE ?= $(CHART_DIR)/values-$(ENV).yaml

# Build values file list (in order of precedence)
VALUES_FILES = -f $(SHARED_BASE_VALUES)
ifneq ($(wildcard $(SHARED_ENV_VALUES)),)
VALUES_FILES += -f $(SHARED_ENV_VALUES)
endif
ifneq ($(wildcard $(SERVICE_ENV_VALUES_FILE)),)
VALUES_FILES += -f $(SERVICE_ENV_VALUES_FILE)
endif
VALUES_FILES += -f $(SERVICE_VALUES_FILE)
Comment on lines +19 to +26

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.

high

The order of precedence for Helm values files is counter-intuitive and error-prone. Currently, the base service values.yaml overrides all environment-specific values because it's the last file in the list. This means environment-specific configurations can be accidentally overridden by base configurations. The most specific values file (e.g., values-prod.yaml) should have the highest precedence (be last in the list).

VALUES_FILES = -f $(SERVICE_VALUES_FILE)
ifneq ($(wildcard $(SHARED_BASE_VALUES)),)
    VALUES_FILES += -f $(SHARED_BASE_VALUES)
endif
ifneq ($(wildcard $(SHARED_ENV_VALUES)),)
    VALUES_FILES += -f $(SHARED_ENV_VALUES)
endif
ifneq ($(wildcard $(SERVICE_ENV_VALUES_FILE)),)
    VALUES_FILES += -f $(SERVICE_ENV_VALUES_FILE)
endif


# Default port for port-forwarding (override as needed)
LOCAL_PORT ?= 8000
REMOTE_PORT ?= 8000

.PHONY: help install install-all upgrade upgrade-all uninstall uninstall-all lint status port-forward

help:
@echo "Makefile commands for Helm chart management with shared values:"
@echo ""
@echo "Single-service commands:"
@echo " make install Install chart with shared values (CHART_NAME, ENV)"
@echo " e.g. make install CHART_NAME=chat-service ENV=staging"
@echo " make upgrade Upgrade chart with shared values (CHART_NAME, ENV)"
@echo " e.g. make upgrade CHART_NAME=embedder-service ENV=prod"
@echo " make uninstall Uninstall chart (CHART_NAME)"
@echo " e.g. make uninstall CHART_NAME=pdf-processor"
@echo " make lint Run helm lint on chart (CHART_NAME)"
@echo " e.g. make lint CHART_NAME=embedder-service"
@echo " make lint-all Lint all charts under ./helm/"
@echo " make status Show status of Helm release (CHART_NAME)"
@echo " e.g. make status CHART_NAME=chat-service"
@echo " make port-forward Port-forward a pod to local machine"
@echo " e.g. make port-forward CHART_NAME=chat-service LOCAL_PORT=8000 REMOTE_PORT=8000"
@echo ""
@echo "Multi-service commands:"
@echo " make install-all Install all charts with shared values (ENV)"
@echo " e.g. make install-all ENV=prod"
@echo " make upgrade-all Upgrade all charts with shared values (ENV)"
@echo " e.g. make upgrade-all ENV=staging"
@echo " make uninstall-all Uninstall all charts under ./helm/"
@echo ""
@echo "Shared Values System:"
@echo " Values are applied in order (later values override earlier ones):"
@echo " 1. helm/shared-values/common-base.yaml - Base configuration for all services"
@echo " 2. helm/shared-values/common-{ENV}.yaml - Environment-specific shared config"
@echo " 3. helm/{SERVICE}/values-{ENV}.yaml - Service-specific environment config"
@echo " 4. helm/{SERVICE}/values.yaml - Service-specific base config"
@echo ""
@echo "Environment Variables:"
@echo " ENV Environment (staging, prestaging, prod) - defaults to 'staging'"
@echo ""
@echo "Development Environment:"
@echo " Use docker-compose for local development:"
@echo " docker-compose up -d # Start all services locally"
@echo " docker-compose logs -f chat_service # View service logs"
@echo ""
@echo "⚠️ IMPORTANT:"
@echo " Avoid underscores (_) in CHART_NAME or release names."
@echo " Use hyphens (-) instead to follow Kubernetes naming conventions (RFC 1123)."
@echo " Example: use chat-service ✅, not chat_service ❌"

## Install a single Helm chart with shared values
install:
@echo "Installing $(CHART_NAME) with shared values for environment: $(ENV)"
@echo "Values files (in order): $(VALUES_FILES)"
helm upgrade --install $(CHART_NAME) $(CHART_DIR) \
--namespace $(NAMESPACE) \
--create-namespace \
$(VALUES_FILES)

## Upgrade a single Helm chart with shared values
upgrade:
@echo "Upgrading $(CHART_NAME) with shared values for environment: $(ENV)"
@echo "Values files (in order): $(VALUES_FILES)"
helm upgrade $(CHART_NAME) $(CHART_DIR) \
--namespace $(NAMESPACE) \
$(VALUES_FILES)

## Uninstall a single Helm chart
uninstall:
helm uninstall $(CHART_NAME) \
--namespace $(NAMESPACE)

## Lint all Helm charts under ./helm/
lint-all:
@echo "Linting all Helm charts under ./helm/..."
@for dir in helm/*/; do \
CHART=$$(basename $$dir); \
if [ "$$CHART" != "shared-values" ] && [ "$$CHART" != "assets" ]; then \
echo "Linting chart: $$CHART"; \
helm lint $$dir || exit 1; \
else \
echo "Skipping non-chart directory: $$dir"; \
fi; \
done

## Run lint check on a chart
lint:
helm lint $(CHART_DIR)

## Show release status and pod info
status:
@echo "=== Helm Release Status ==="
helm status $(CHART_NAME) -n $(NAMESPACE)
@echo ""
@echo "=== Pod Status ==="
kubectl get pods -n $(NAMESPACE) -l "app.kubernetes.io/name=$(CHART_NAME),app.kubernetes.io/instance=$(CHART_NAME)"

# Helper function to deploy all charts (used by both install-all and upgrade-all)
define deploy-all-charts
@echo "$(1) all Helm charts under ./helm/ with shared values for environment: $(ENV)"
@for dir in helm/*/; do \
CHART=$$(basename $$dir); \
if [ "$$CHART" != "shared-values" ] && [ "$$CHART" != "assets" ]; then \
echo "$(1) chart: $$CHART"; \
CHART_VALUES="-f $(SHARED_BASE_VALUES)"; \
if [ -f "$(SHARED_VALUES_DIR)/common-$(ENV).yaml" ]; then \
CHART_VALUES="$$CHART_VALUES -f $(SHARED_VALUES_DIR)/common-$(ENV).yaml"; \
fi; \
if [ -f "helm/$$CHART/values-$(ENV).yaml" ]; then \
CHART_VALUES="$$CHART_VALUES -f helm/$$CHART/values-$(ENV).yaml"; \
fi; \
CHART_VALUES="$$CHART_VALUES -f helm/$$CHART/values.yaml"; \
helm upgrade --install $$CHART helm/$$CHART \
--namespace $(NAMESPACE) \
--create-namespace \
$$CHART_VALUES; \
fi; \
done
endef
Comment on lines +127 to +147

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.

medium

The deploy-all-charts function duplicates the logic for constructing the list of values files from the install target. This makes the Makefile harder to maintain, as any change to the values file logic needs to be updated in two places. This could be refactored to call the install target within the loop.


## Install all Helm charts in ./helm/ with shared values
install-all:
$(call deploy-all-charts,Installing)

## Upgrade all Helm charts in ./helm/ with shared values
upgrade-all:
$(call deploy-all-charts,Upgrading)

## Uninstall all Helm charts in ./helm/
uninstall-all:
@echo "Uninstalling all Helm charts under ./helm/..."
@for dir in helm/*/; do \
CHART=$$(basename $$dir); \
if [ "$$CHART" != "shared-values" ] && [ "$$CHART" != "assets" ]; then \
echo "Uninstalling chart: $$CHART"; \
helm uninstall $$CHART \
--namespace $(NAMESPACE); \
fi; \
done

## Port-forward a running pod (default 8000:8000)
port-forward:
ifeq ($(CHART_NAME),example-service)
@echo "ERROR: CHART_NAME must be specified. Example usage:"
@echo " make port-forward CHART_NAME=chat-service LOCAL_PORT=3000 REMOTE_PORT=8000"
@exit 1
else
kubectl --namespace $(NAMESPACE) port-forward \
$$(kubectl get pod -n $(NAMESPACE) -l "app.kubernetes.io/name=$(CHART_NAME),app.kubernetes.io/instance=$(CHART_NAME)" -o jsonpath="{.items[0].metadata.name}") \
$(LOCAL_PORT):$(REMOTE_PORT)
endif
Loading