Skip to content

Conversation

@Sg312
Copy link
Contributor

@Sg312 Sg312 commented Nov 7, 2025

Summary

  • added copilot image to helm charts

Type of Change

  • New feature

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@Sg312 Sg312 requested a review from waleedlatif1 November 7, 2025 01:25
@vercel
Copy link

vercel bot commented Nov 7, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
docs Ready Ready Preview Comment Nov 9, 2025 1:32am

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR adds comprehensive Helm chart support for deploying the Copilot microservice alongside the main Sim platform. The implementation introduces a complete Kubernetes stack including deployment, StatefulSet for PostgreSQL, migration jobs, secrets management, and optional ingress routing.

Key Changes:

  • New Kubernetes Resources: Deployment for copilot server, StatefulSet for dedicated PostgreSQL database, pre-install migration Job with database readiness checks
  • Configuration Management: Extensive values.yaml configuration with validation logic, JSON schema for type safety, and example values file for reference
  • Secret Handling: Separate secrets for environment variables and database credentials, with support for both internal PostgreSQL and external managed databases
  • Integration: Extends existing ingress to route copilot traffic, uses shared labels and naming conventions from existing helpers
  • Flexibility: Supports both internal PostgreSQL (for dev/small deployments) and external managed databases (for production), configurable resource limits and node scheduling

Issues Found:

  • Repository name inconsistency: sidstudio/copilot used in 2 locations vs simstudioai/copilot (the correct namespace used elsewhere in the chart)

Confidence Score: 4/5

  • This PR is mostly safe to merge with one syntax fix required
  • The Helm chart implementation is well-structured with proper validation, health checks, and secret management. However, the repository name inconsistency must be fixed before deployment to ensure the correct container image is pulled. The infrastructure code follows Kubernetes best practices with StatefulSets for stateful workloads, proper secret handling, and migration job hooks.
  • Fix repository names in helm/sim/values.yaml (line 925) and helm/sim/examples/values-copilot.yaml (line 8) before merging

Important Files Changed

File Analysis

Filename Score Overview
helm/sim/templates/deployment-copilot.yaml 5/5 Adds Kubernetes Deployment and Service for copilot microservice with proper configuration, health checks, and secret management
helm/sim/templates/statefulset-copilot-postgres.yaml 5/5 Adds PostgreSQL StatefulSet with persistence, secrets, and health probes for copilot's database
helm/sim/templates/_helpers.tpl 5/5 Adds helper functions for copilot secret names and comprehensive validation logic for required configuration
helm/sim/values.yaml 3/5 Adds comprehensive copilot configuration with server, PostgreSQL, and migration settings; contains repository name inconsistency (line 925)
helm/sim/examples/values-copilot.yaml 3/5 Provides example configuration for copilot deployment with documentation; contains repository name inconsistency (line 8)

Sequence Diagram

sequenceDiagram
    participant User
    participant Helm
    participant K8s as Kubernetes
    participant PG as PostgreSQL StatefulSet
    participant MigJob as Migration Job
    participant Copilot as Copilot Deployment
    participant Ingress

    User->>Helm: helm install/upgrade with copilot.enabled=true
    Helm->>Helm: Validate required env vars (AGENT_API_DB_ENCRYPTION_KEY, etc)
    
    alt Internal PostgreSQL (copilot.postgresql.enabled=true)
        Helm->>K8s: Create copilot-postgresql-secret
        Helm->>K8s: Create PostgreSQL Service
        Helm->>K8s: Create PostgreSQL StatefulSet with PVC
        K8s->>PG: Start PostgreSQL pod
        PG->>PG: Initialize database
    else External Database
        Helm->>K8s: Create database secret from copilot.database.url
    end
    
    Helm->>K8s: Create copilot-env secret (API keys, config)
    Helm->>K8s: Create Migration Job (post-install hook)
    K8s->>MigJob: Start migration job
    
    alt Internal PostgreSQL
        MigJob->>PG: Wait for PostgreSQL (pg_isready check)
        PG-->>MigJob: Ready
    end
    
    MigJob->>PG: Run database migrations
    MigJob-->>K8s: Job complete
    
    Helm->>K8s: Create Copilot Service
    Helm->>K8s: Create Copilot Deployment
    K8s->>Copilot: Start copilot pods (inject secrets)
    Copilot->>Copilot: Load env vars from secrets
    Copilot->>PG: Connect to database
    Copilot->>Copilot: Health check /healthz
    Copilot-->>K8s: Ready
    
    opt Ingress configured
        Helm->>K8s: Update Ingress with copilot host
        K8s->>Ingress: Configure routing
        Ingress-->>User: Copilot accessible at copilot.domain.com
    end
Loading

10 files reviewed, 2 comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 force-pushed the feat/copilot-image-helm branch from fbf957e to feae33e Compare November 9, 2025 01:19
waleedlatif1
waleedlatif1 previously approved these changes Nov 9, 2025
Copy link
Collaborator

@waleedlatif1 waleedlatif1 left a comment

Choose a reason for hiding this comment

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

lgtm, made some slight modifications

@waleedlatif1
Copy link
Collaborator

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

Adds comprehensive Helm chart support for the copilot microservice with deployment, PostgreSQL database, migrations, and optional ingress configuration.

Key Changes

  • Deployment: Creates copilot deployment with configurable replicas, resources, health checks, and security contexts
  • Database: Provides internal PostgreSQL StatefulSet or external database connection options
  • Migrations: Implements Helm hook-based migration job that waits for PostgreSQL and runs schema updates
  • Secrets: Manages environment variables and database credentials through Kubernetes secrets
  • Validation: Adds comprehensive validation helpers ensuring required configuration is provided
  • Ingress & HA: Optional ingress routing and PodDisruptionBudget for high availability
  • Schema: JSON schema validation for all copilot configuration properties

Issues Found

  • Critical: PostgreSQL StatefulSet missing subPath: pgdata in volume mount (line 104-105 of statefulset-copilot-postgres.yaml), which will cause initialization failures on storage systems with lost+found directories
  • Noted: Two previous comments about inconsistent repository names have already been flagged

Confidence Score: 3/5

  • This PR has a critical PostgreSQL initialization bug that will cause deployment failures
  • The implementation is comprehensive and well-structured with proper validation, security contexts, and Helm best practices. However, the missing subPath: pgdata in the PostgreSQL volume mount is a critical issue that will prevent the database from initializing correctly on most storage systems. This is a deployment-blocking bug that must be fixed before merge.
  • Pay immediate attention to helm/sim/templates/statefulset-copilot-postgres.yaml - the volume mount configuration needs the subPath fix to prevent PostgreSQL initialization failures

Important Files Changed

File Analysis

Filename Score Overview
helm/sim/values.yaml 5/5 adds comprehensive copilot configuration section with server, PostgreSQL, migrations, and validation settings
helm/sim/templates/_helpers.tpl 5/5 adds helper functions for copilot secret names, database configuration, and required validation logic
helm/sim/templates/deployment-copilot.yaml 5/5 creates deployment and service for copilot with proper health checks, security contexts, and environment injection
helm/sim/templates/statefulset-copilot-postgres.yaml 3/5 creates PostgreSQL statefulset for copilot, but missing subPath for data volume which will cause initialization failures
helm/sim/templates/job-copilot-migrations.yaml 5/5 creates migration job with postgres wait initContainer and proper helm hooks for post-install/upgrade execution

Sequence Diagram

sequenceDiagram
    participant Helm as Helm Install/Upgrade
    participant Validate as Validation Helper
    participant PG as PostgreSQL StatefulSet
    participant Secrets as Secrets (Env & DB)
    participant Migration as Migration Job
    participant Copilot as Copilot Deployment
    participant Ingress as Ingress (Optional)
    
    Helm->>Validate: Check copilot.enabled
    Validate->>Validate: Validate required env vars
    Validate->>Validate: Check API keys (OpenAI or Anthropic)
    Validate->>Validate: Validate database config
    
    alt PostgreSQL Enabled
        Helm->>Secrets: Create PostgreSQL secret
        Helm->>PG: Create StatefulSet
        PG->>PG: Initialize database
        Note over PG: Port 5432
    else External Database
        Helm->>Secrets: Create external DB secret
    end
    
    Helm->>Secrets: Create environment secret
    
    Helm->>Migration: Create migration job (hook)
    Note over Migration: helm.sh/hook: post-install,post-upgrade
    Migration->>PG: Wait for PostgreSQL ready
    PG-->>Migration: pg_isready success
    Migration->>Migration: Run /usr/local/bin/migrate
    Migration->>PG: Apply schema migrations
    
    Helm->>Copilot: Create Deployment
    Copilot->>Secrets: Load environment variables
    Copilot->>PG: Connect to database
    Copilot->>Copilot: Start server on port 8080
    Note over Copilot: /healthz endpoint
    
    Helm->>Copilot: Create Service (ClusterIP)
    
    alt Ingress Configured
        Helm->>Ingress: Add copilot routing rules
        Note over Ingress: External access enabled
    end
    
    alt PodDisruptionBudget Enabled
        Helm->>Copilot: Create PDB for HA
    end
Loading

11 files reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

@waleedlatif1
Copy link
Collaborator

@greptile

Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

Greptile Overview

Greptile Summary

This PR adds comprehensive Helm chart support for the copilot service, addressing the previously identified subPath: pgdata issue for PostgreSQL volume mounts.

Key additions:

  • Secret management for copilot environment variables and database credentials with flexible configuration options
  • PostgreSQL StatefulSet for copilot with proper volume mounting using subPath: pgdata to prevent initialization failures
  • Copilot server deployment with validation logic ensuring required API keys and encryption keys are provided
  • Database migration job with PostgreSQL readiness checks
  • Support for both internal PostgreSQL and external database configurations

Implementation highlights:

  • Secret reference logic properly handles three scenarios: internal PostgreSQL, external database with existing secret, and external database with provided URL
  • Comprehensive validation in _helpers.tpl ensures required environment variables are set before deployment
  • Migration job uses init container to wait for PostgreSQL readiness when using internal database
  • Follows existing Helm chart patterns from the main application

Confidence Score: 5/5

  • This PR is safe to merge - it adds well-structured copilot infrastructure following established patterns
  • The implementation follows Kubernetes and Helm best practices with proper secret management, validation logic, and volume mounting. The previous critical issue with PostgreSQL volume mounting (missing subPath) has been resolved. All conditional logic for secret creation and referencing is correct, and the migration job properly waits for database readiness.
  • No files require special attention - all implementations follow established patterns correctly

Important Files Changed

File Analysis

Filename Score Overview
helm/sim/templates/secrets-copilot.yaml 5/5 creates copilot environment and database secrets with proper conditional logic for different deployment scenarios
helm/sim/templates/statefulset-copilot-postgres.yaml 5/5 adds PostgreSQL StatefulSet for copilot with proper subPath configuration, secret management, and service definition
helm/sim/templates/deployment-copilot.yaml 5/5 creates copilot server deployment with validation, proper secret references, and configurable replicas

Sequence Diagram

sequenceDiagram
    participant Helm as Helm Chart
    participant Validation as Validation Logic
    participant Secrets as Kubernetes Secrets
    participant PostgreSQL as PostgreSQL StatefulSet
    participant Migration as Migration Job
    participant Copilot as Copilot Deployment
    
    Helm->>Validation: Validate copilot configuration
    Validation->>Validation: Check required env vars (API keys, encryption keys)
    Validation->>Validation: Verify database config (internal or external)
    
    alt PostgreSQL Enabled
        Helm->>Secrets: Create copilot-postgresql-secret
        Note over Secrets: Contains POSTGRES_USER, POSTGRES_PASSWORD, DATABASE_URL
        Helm->>PostgreSQL: Deploy StatefulSet with PVC
        Note over PostgreSQL: Mounts volume at /var/lib/postgresql/data with subPath: pgdata
        PostgreSQL->>PostgreSQL: Initialize database
    else External Database
        alt Existing Secret Provided
            Note over Secrets: Use existing secret
        else Database URL Provided
            Helm->>Secrets: Create copilot-database-secret
            Note over Secrets: Contains DATABASE_URL
        end
    end
    
    Helm->>Secrets: Create copilot-env secret
    Note over Secrets: Contains server env vars (API keys, encryption keys)
    
    alt Migrations Enabled
        Helm->>Migration: Create migration Job (post-install/post-upgrade hook)
        Migration->>PostgreSQL: Wait for PostgreSQL ready (if internal)
        Migration->>Migration: Run database migrations
        Migration->>Copilot: Migration complete
    end
    
    Helm->>Copilot: Deploy copilot server
    Copilot->>Secrets: Load env and database secrets
    Copilot->>Copilot: Start copilot application

Loading

3 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@waleedlatif1 waleedlatif1 merged commit 142d3aa into staging Nov 9, 2025
9 checks passed
@waleedlatif1 waleedlatif1 deleted the feat/copilot-image-helm branch November 9, 2025 01:36
waleedlatif1 pushed a commit that referenced this pull request Nov 9, 2025
* Add helm for copilot

* Remove otel and log level

* Change repo name

* improvement(helm): enhance copilot chart with HA support and validation

* refactor(helm): consolidate copilot secrets and fix postgres volume mount
waleedlatif1 pushed a commit that referenced this pull request Nov 12, 2025
* Add helm for copilot

* Remove otel and log level

* Change repo name

* improvement(helm): enhance copilot chart with HA support and validation

* refactor(helm): consolidate copilot secrets and fix postgres volume mount
@waleedlatif1 waleedlatif1 mentioned this pull request Nov 12, 2025
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants