Skip to content
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

API: Sandbox routing: Use Redis for client-proxy DNS instead of local map. #245

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jaytaylor
Copy link
Collaborator

@jaytaylor jaytaylor commented Jan 17, 2025

API: Sandbox routing:

Use Redis for client-proxy DNS instead of local map.

It also cleans up logging a bit to be more consistent and informative about activity.

Description by Callstackai

This PR refactors the DNS handling in the API to use Redis for client-proxy DNS instead of a local map. It also improves logging consistency and adds a Redis job specification.

Diagrams of code changes
sequenceDiagram
    participant Client
    participant DNS Server
    participant Redis
    participant Orchestrator
    participant Node

    Note over DNS Server: New DNS service with Redis integration

    alt Add DNS Entry
        Orchestrator->>DNS Server: Add(sandboxID, ip)
        DNS Server->>Redis: Set key-value with 24h expiration
    end

    alt DNS Query Flow
        Client->>DNS Server: Query(sandboxID)
        DNS Server->>Redis: Get(sandboxID)
        alt Found in Redis
            Redis-->>DNS Server: Return IP
        else Not Found in Redis
            DNS Server->>Orchestrator: FallbackResolver(sandboxID)
            Orchestrator-->>DNS Server: Return IP from Node
            DNS Server->>Redis: Cache IP (async)
        end
        DNS Server-->>Client: Return IP Address
    end

    alt Remove DNS Entry
        Orchestrator->>DNS Server: Remove(sandboxID)
        DNS Server->>Redis: Delete key
    end
Loading
Files Changed
FileSummary
.terraform.lock.hclUpdated provider constraints and added new hashes.
packages/api/internal/dns/server.goRefactored DNS handling to use Redis for storage and improved logging.
packages/api/internal/orchestrator/cache.goUpdated DNS removal and addition to handle errors properly.
packages/api/internal/orchestrator/orchestrator.goInitialized Redis client and fallback resolver function for DNS.
packages/nomad/main.tfAdded a new Nomad job for Redis.
packages/nomad/redis.hclCreated a new configuration file for the Redis job.

This PR includes files in programming languages that we currently do not support. We have not reviewed files with the extensions .hcl, .tf. See list of supported languages.

callstackai[bot]

This comment was marked as outdated.

@jaytaylor jaytaylor force-pushed the jay/redis-dns branch 3 times, most recently from 7c35766 to 39ce0fb Compare January 17, 2025 01:12
callstackai[bot]

This comment was marked as outdated.

callstackai[bot]

This comment was marked as outdated.

callstackai[bot]

This comment was marked as outdated.

packages/api/internal/dns/server.go Outdated Show resolved Hide resolved
packages/api/internal/orchestrator/cache.go Show resolved Hide resolved
packages/api/internal/dns/server.go Outdated Show resolved Hide resolved
callstackai[bot]

This comment was marked as outdated.

@jaytaylor jaytaylor force-pushed the jay/redis-dns branch 2 times, most recently from c942f6b to ea157fd Compare January 17, 2025 01:28
callstackai[bot]

This comment was marked as outdated.

callstackai[bot]

This comment was marked as outdated.

callstackai[bot]

This comment was marked as outdated.

callstackai[bot]

This comment was marked as outdated.

@ValentaTomas ValentaTomas added the improvement Improvement for current functionality label Jan 17, 2025
@ValentaTomas
Copy link
Member

The PR looks good!
I'm changing it to draft for now—we will return to it when we start changing the other parts of the API to use Redis.

@ValentaTomas ValentaTomas marked this pull request as draft January 17, 2025 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
improvement Improvement for current functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants