Skip to content

feat: e2e deploy with docker works#3567

Merged
chronark merged 12 commits intomainfrom
07-15-feat_e2e_deploy_with_docker_works
Jul 16, 2025
Merged

feat: e2e deploy with docker works#3567
chronark merged 12 commits intomainfrom
07-15-feat_e2e_deploy_with_docker_works

Conversation

@chronark
Copy link
Collaborator

@chronark chronark commented Jul 15, 2025

What does this PR do?

Implements real VM provisioning for deployments using the Docker backend. This PR replaces the mock VM creation and management with actual calls to the metald service, enabling proper container deployment.

Key changes:

  • Replaced mock VM creation with real metald CreateVm calls
  • Updated VM configuration for Docker backend with appropriate resources (512MB RAM, 1 CPU)
  • Added port mapping support for deployed containers
  • Enhanced hostname assignment to include localhost:port for local development
  • Improved VM status handling and error reporting
  • Added VM info retrieval to get container port mappings

Fixes #123

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Chore (refactoring code, technical debt, workflow improvements)
  • Enhancement (small improvements)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How should this be tested?

Follow the instructions in QUICKSTART-DEPLOY.md

Checklist

Required

  • Filled out the "How to test" section in this PR
  • Read Contributing Guide
  • Self-reviewed my own code
  • Commented on my code in hard-to-understand areas
  • Ran pnpm build
  • Ran pnpm fmt
  • Checked for warnings, there are none
  • Removed all console.logs
  • Merged the latest changes from main onto my branch with git pull origin main
  • My changes don't cause any responsiveness issues

Appreciated

  • If a UI change was made: Added a screen recording or screenshots to this PR
  • Updated the Unkey Docs if changes were necessary

Summary by CodeRabbit

  • New Features

    • Deployment now uses Docker containers instead of VMs, with improved startup speed and simplified process.
    • CLI deployment output now displays all assigned hostnames and domains, including localhost ports.
    • Troubleshooting instructions and code samples for handling the PORT environment variable added to documentation.
  • Bug Fixes

    • Enhanced Docker backend with robust image handling, environment variable support, and improved port allocation to avoid conflicts.
  • Refactor

    • Dockerfile for development refactored for faster, parallel builds and reduced redundancy.
    • Deployment workflow updated to interact with real VM provisioning and Docker backend.
  • Documentation

    • Quickstart deployment guide updated to reflect container-based workflow, new commands, and troubleshooting steps.
  • Chores

    • Removed unused default Docker image configuration.

@changeset-bot
Copy link

changeset-bot bot commented Jul 15, 2025

⚠️ No Changeset found

Latest commit: d165e14

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jul 15, 2025

📝 Walkthrough

Walkthrough

The changes update the deployment workflow and documentation to use a Docker container-based backend instead of VM-based provisioning. Modifications include robust Docker image handling, improved port allocation, enhanced environment variable support, updated CLI deployment reporting, and revised quickstart instructions to reflect the new container-based approach.

Changes

File(s) Change Summary
QUICKSTART-DEPLOY.md Updated quickstart guide for Docker-based deployment, revised steps, added troubleshooting, and code examples.
go/apps/ctrl/services/version/deploy_workflow.go Switched from mock to real metald VM provisioning, updated for Docker backend, enhanced domain assignment.
go/cmd/version/main.go Added --force-build CLI flag, always builds Docker image, improved deployment reporting, updated function signatures.
go/demo_api/Dockerfile Replaced EXPOSE 8080 with ENV PORT 8080.
go/deploy/Dockerfile.dev Refactored to multi-stage build, parallel service compilation, streamlined runtime stage.
go/deploy/metald/internal/backend/docker/client.go Improved Docker backend: robust image/port handling, env var support, container checks, logging enhancements.
go/deploy/metald/internal/backend/docker/types.go Removed DefaultImage field from config struct and default config function.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant CLI
    participant ctrl (API)
    participant metald (Docker backend)
    participant Docker

    User->>CLI: Run deploy command
    CLI->>ctrl (API): Create deployment (always build Docker image)
    ctrl (API)->>metald: CreateVm (with Docker image & env vars)
    metald->>Docker: Create container (pull image if needed, allocate random port)
    Docker-->>metald: Container ID, port mapping
    metald->>Docker: Boot container (verify existence, start)
    Docker-->>metald: Container running
    metald->>ctrl (API): GetVmInfo (with port mappings)
    ctrl (API)->>ctrl (API): Assign domains (including localhost:port)
    ctrl (API)-->>CLI: Deployment status and domains
    CLI-->>User: Print deployment complete with hostnames
Loading

Assessment against linked issues

Objective Addressed Explanation
[#123] Update overview.mdx for typo fix No changes to overview.mdx or typo corrections were made in this PR.

Possibly related PRs

  • feat: docker backend #3565: Updates the deployment workflow and CLI to use the Docker backend for VM provisioning and deployment, directly building on and modifying the Docker backend implementation introduced in this PR.
  • feat: workflow calls metald to create and boot a vm #3495: Extends and refines the deployment workflow’s VM provisioning by replacing mocks with real metald client RPC calls to create, boot, and manage Docker-backed VMs, directly building on this PR’s initial integration.

Suggested reviewers

  • imeyer
  • perkinsjr
  • Flo4604
✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@chronark chronark mentioned this pull request Jul 15, 2025
18 tasks
@vercel
Copy link

vercel bot commented Jul 15, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
dashboard ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2025 1:31pm
engineering ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2025 1:31pm

@chronark chronark mentioned this pull request Jul 15, 2025
18 tasks
Copy link
Collaborator Author

chronark commented Jul 15, 2025

@vercel vercel bot temporarily deployed to Preview – engineering July 15, 2025 13:34 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard July 15, 2025 13:34 Inactive
@chronark chronark marked this pull request as ready for review July 15, 2025 13:34
Copy link
Collaborator Author

🔍 Review requested from @imeyer, @perkinsjr and @Flo4604

@github-actions
Copy link
Contributor

github-actions bot commented Jul 15, 2025

Thank you for following the naming conventions for pull request titles! 🙏

@chronark chronark mentioned this pull request Jul 15, 2025
18 tasks
@chronark chronark force-pushed the 07-15-feat_docker_seems_to_run branch from 57d9cf4 to a86c954 Compare July 16, 2025 08:34
@chronark chronark force-pushed the 07-15-feat_e2e_deploy_with_docker_works branch from 44bda99 to 2e47e5a Compare July 16, 2025 08:34
@vercel vercel bot temporarily deployed to Preview – dashboard July 16, 2025 08:37 Inactive
@vercel vercel bot temporarily deployed to Preview – engineering July 16, 2025 08:37 Inactive
Base automatically changed from 07-15-feat_docker_seems_to_run to main July 16, 2025 09:33
@vercel vercel bot temporarily deployed to Preview – engineering July 16, 2025 09:38 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard July 16, 2025 09:38 Inactive
@vercel vercel bot temporarily deployed to Preview – engineering July 16, 2025 13:29 Inactive
@vercel vercel bot temporarily deployed to Preview – dashboard July 16, 2025 13:31 Inactive
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 10

📜 Review details

Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d54f3db and d165e14.

📒 Files selected for processing (7)
  • QUICKSTART-DEPLOY.md (3 hunks)
  • go/apps/ctrl/services/version/deploy_workflow.go (7 hunks)
  • go/cmd/version/main.go (5 hunks)
  • go/demo_api/Dockerfile (1 hunks)
  • go/deploy/Dockerfile.dev (3 hunks)
  • go/deploy/metald/internal/backend/docker/client.go (14 hunks)
  • go/deploy/metald/internal/backend/docker/types.go (0 hunks)
💤 Files with no reviewable changes (1)
  • go/deploy/metald/internal/backend/docker/types.go
🧰 Additional context used
📓 Path-based instructions (3)
go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,env,conf,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml}

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • go/deploy/CLAUDE.md
go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml}

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • go/deploy/CLAUDE.md
go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go

Instructions used from:

Sources:
📄 CodeRabbit Inference Engine

  • go/deploy/CLAUDE.md
🧠 Learnings (7)
📓 Common learnings
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.898Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:153-158
Timestamp: 2025-07-16T09:18:45.348Z
Learning: In the go/cmd/cli/commands/deploy/ CLI codebase, ogzhanolguncu prefers to allow deployment to continue even when Docker push fails (around lines 153-158 in deploy.go) because the team is working locally and needs this behavior for local development workflows where registry access might not be available.
go/demo_api/Dockerfile (1)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.898Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.
go/deploy/Dockerfile.dev (13)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make install` to build and install the binary with systemd unit from `$SERVICE/contrib/systemd`.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Use `make build` to test that the binary builds.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.898Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/Makefile : Service-level makefile should be `<service>/Makefile`.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/build/** : Build artifact directory should be `<service>/build`.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Do not remove `AIDEV-*`s without explicit human instruction.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:153-158
Timestamp: 2025-07-16T09:18:45.348Z
Learning: In the go/cmd/cli/commands/deploy/ CLI codebase, ogzhanolguncu prefers to allow deployment to continue even when Docker push fails (around lines 153-158 in deploy.go) because the team is working locally and needs this behavior for local development workflows where registry access might not be available.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Use `AIDEV-NOTE:`, `AIDEV-TODO:`, `AIDEV-BUSINESS_RULE:`, or `AIDEV-QUESTION:` (all-caps prefix) as anchor comments aimed at AI and developers.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{env,yaml,yml,json,ini,conf,sh} : All environment variables MUST follow the format UNKEY_<SERVICE_NAME>_VARNAME.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/cmd/*/** : Service binary code should be located at `<service>/cmd/<service | command>`.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/contrib/systemd/** : Systemd unit files should be located at `<service>/contrib/systemd`.
go/apps/ctrl/services/version/deploy_workflow.go (4)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:153-158
Timestamp: 2025-07-16T09:18:45.348Z
Learning: In the go/cmd/cli/commands/deploy/ CLI codebase, ogzhanolguncu prefers to allow deployment to continue even when Docker push fails (around lines 153-158 in deploy.go) because the team is working locally and needs this behavior for local development workflows where registry access might not be available.
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.173Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.898Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.
QUICKSTART-DEPLOY.md (10)
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.898Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Use `AIDEV-NOTE:`, `AIDEV-TODO:`, `AIDEV-BUSINESS_RULE:`, or `AIDEV-QUESTION:` (all-caps prefix) as anchor comments aimed at AI and developers.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Make sure to add relevant anchor comments whenever a file or piece of code is too complex, very important, confusing, or could have a bug.
Learnt from: Devansh-Baghel
PR: unkeyed/unkey#2452
File: oss.gg/7_create_a_template.md:42-42
Timestamp: 2024-10-15T19:57:16.520Z
Learning: In Hono & Cloudflare Workers templates, the 'Setup and Installation' section in the README serves as the quickstart guide.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Update relevant anchors when modifying associated code.
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.173Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/*/docs/** : Documentation should be located at `<service>/docs`.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/**/*.{go,js,ts,tsx,py,sh,md,txt,json,yaml,yml,ini,rb,java,c,cpp,h,cs,rs,php,html,css,scss,xml} : Do not remove `AIDEV-*`s without explicit human instruction.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:153-158
Timestamp: 2025-07-16T09:18:45.348Z
Learning: In the go/cmd/cli/commands/deploy/ CLI codebase, ogzhanolguncu prefers to allow deployment to continue even when Docker push fails (around lines 153-158 in deploy.go) because the team is working locally and needs this behavior for local development workflows where registry access might not be available.
go/deploy/metald/internal/backend/docker/client.go (1)
Learnt from: chronark
PR: unkeyed/unkey#3560
File: go/deploy/metald/internal/database/repository.go:0-0
Timestamp: 2025-07-15T14:59:30.173Z
Learning: go/deploy/metald cannot currently import helpers from go/pkg/db because it is not yet part of the main Go module; avoid suggesting such imports until the modules are unified.
go/cmd/version/main.go (6)
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/*.go : When a service's `*.go` code changes significantly, increase the patch-level version number.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/flags.go:17-20
Timestamp: 2025-07-15T14:45:18.898Z
Learning: In the go/cmd/cli/commands/deploy/ directory, ogzhanolguncu prefers to keep potentially temporary features (like UNKEY_DOCKER_REGISTRY environment variable) undocumented in help text if they might be deleted in the future, to avoid documentation churn.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:153-158
Timestamp: 2025-07-16T09:18:45.348Z
Learning: In the go/cmd/cli/commands/deploy/ CLI codebase, ogzhanolguncu prefers to allow deployment to continue even when Docker push fails (around lines 153-158 in deploy.go) because the team is working locally and needs this behavior for local development workflows where registry access might not be available.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:14-14
Timestamp: 2025-07-15T14:47:43.695Z
Learning: In go/cmd/cli/commands/deploy/deploy.go, the DEBUG_DELAY constant is temporary code that ogzhanolguncu plans to remove in a future PR, rather than making it configurable.
Learnt from: CR
PR: unkeyed/unkey#0
File: go/deploy/CLAUDE.md:0-0
Timestamp: 2025-07-09T08:42:29.316Z
Learning: Applies to go/deploy/{assetmanagerd,billaged,builderd,metald}/**/Makefile : Never use `go build` for any of the `assetmanagerd`, `billaged`, `builderd`, `metald` binaries.
Learnt from: ogzhanolguncu
PR: unkeyed/unkey#3564
File: go/cmd/cli/commands/deploy/deploy.go:71-73
Timestamp: 2025-07-15T15:11:30.732Z
Learning: In the go/cmd/cli/commands/deploy/ CLI codebase, ogzhanolguncu uses a UI-layer error handling pattern where errors are displayed through ui.PrintError() and ui.PrintErrorDetails(), followed by returning nil to indicate the error has been handled at the UI level, rather than propagating errors up the call stack.
🪛 LanguageTool
QUICKSTART-DEPLOY.md

[grammar] ~9-~9: Use correct spacing
Context: ...- Go 1.24 or later - A terminal/command line ## Step 1: Start the Platform 1. Start all...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~11-~11: Use correct spacing
Context: ...inal/command line ## Step 1: Start the Platform 1. Start all services using Docker Compose:...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~13-~13: Use correct spacing
Context: ...orm 1. Start all services using Docker Compose: bash docker-compose up metald-aio dashboard ctrl -d 2. Wait for all services to be healthy The...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~19-~19: There might be a mistake here.
Context: ... -d ``` 2. Wait for all services to be healthy The platform now uses a Docker backend t...

(QB_NEW_EN_OTHER)


[grammar] ~21-~21: Use correct spacing
Context: ...making it much faster and easier to run locally. ## Step 2: Set Up Your Workspace 1. Open y...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~31-~31: There might be a mistake here.
Context: ...``` 2. Create a workspace and copy its id 3. Create a new project by filling ou...

(QB_NEW_EN_OTHER)


[grammar] ~33-~33: Use correct spacing
Context: ...Create a new project by filling out the form: Go to http://localhost:3000/projects - ...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~35-~35: There might be a mistake here.
Context: ... the form: Go to http://localhost:3000/projects - Name: Choose any name (e.g., "My Test ...

(QB_NEW_EN_OTHER)


[grammar] ~37-~37: There might be a mistake here.
Context: ...Name**: Choose any name (e.g., "My Test App") - Slug: This will auto-generate based on...

(QB_NEW_EN_OTHER)


[grammar] ~38-~38: There might be a mistake here.
Context: ...*: This will auto-generate based on the name - Git URL: Optional, leave blank for tes...

(QB_NEW_EN_OTHER)


[grammar] ~39-~39: There might be a mistake here.
Context: ...nerate based on the name - Git URL: Optional, leave blank for testing 4. After creat...

(QB_NEW_EN_OTHER)


[grammar] ~39-~39: There might be a mistake here.
Context: ... Git URL: Optional, leave blank for testing 4. After creating the project, **copy the P...

(QB_NEW_EN_OTHER)


[grammar] ~41-~41: Use correct spacing
Context: ... from the project details. It will look like: ## Step 3: Deploy a Version 1. Navigate to...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~64-~64: There might be a mistake here.
Context: ...ys build a fresh Docker image from your code - Set the PORT environment variable to 808...

(QB_NEW_EN_OTHER)


[grammar] ~65-~65: There might be a mistake here.
Context: ...ORT environment variable to 8080 in the container - Use the Docker backend to create a conta...

(QB_NEW_EN_OTHER)


[grammar] ~66-~66: Use proper capitalization
Context: ...kend to create a container instead of a VM - Automatically allocate a random host por...

(QB_NEW_EN_OTHER_ERROR_IDS_6)


[grammar] ~67-~67: There might be a mistake here.
Context: ...random host port (e.g., 35432) to avoid conflicts - Show real-time progress as your deployme...

(QB_NEW_EN_OTHER)


[grammar] ~68-~68: There might be a mistake here.
Context: ...ess as your deployment goes through the stages ## Step 4: View Your Deployment 1. Once th...

(QB_NEW_EN_OTHER)


[grammar] ~70-~70: Use correct spacing
Context: ...hrough the stages ## Step 4: View Your Deployment 1. Once the deployment completes, the CLI w...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~72-~72: Use correct spacing
Context: ...es, the CLI will show you the available domains: Deployment Complete Version ID: v_xxxxxxxxxxxxxxxxxx Status: Ready Environment: Production Domains https://main-commit-workspace.unkey.app http://localhost:35432 2. If you're using the demo_api you can c...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~85-~85: There might be a mistake here.
Context: ...mo_apiyou can curl the/v1/liveness` endpoint 3. Return to the dashboard and navigate to:...

(QB_NEW_EN_OTHER)


[grammar] ~86-~86: Use correct spacing
Context: ...3. Return to the dashboard and navigate to: http://localhost:3000/versions http://localhost:3000/deployments ### Important: Your Application Must Listen ...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~93-~93: There might be a mistake here.
Context: ...our Application Must Listen on the PORT Environment Variable **Your deployed application MUST read the ...

(QB_NEW_EN_OTHER)


[grammar] ~95-~95: Use correct spacing
Context: ...tainer, and your code needs to use this value. Example for different languages: ```j...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~97-~97: Use correct spacing
Context: ...se this value. Example for different languages: javascript // Node.js const port = process.env.PORT || 3000; app.listen(port, () => { console.log(`Server running on port ${port}`); }); python # Python import os port = int(os.environ.get('PORT', 3000)) app.run(host='0.0.0.0', port=port) go // Go port := os.Getenv("PORT") if port == "" { port = "3000" } http.ListenAndServe(":"+port, handler) The demo_api already follows this patter...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~123-~123: Use correct spacing
Context: ...ern and listens on the PORT environment variable. ## Troubleshooting - If you see "port is a...

(QB_NEW_EN_OTHER_ERROR_IDS_5)


[grammar] ~127-~127: There might be a mistake here.
Context: ...l automatically retry with a new random port - Check container logs: `docker logs <cont...

(QB_NEW_EN_OTHER)


[grammar] ~129-~129: There might be a mistake here.
Context: ...he PORT environment variable (should be 8080) - Make sure your Dockerfile exposes the co...

(QB_NEW_EN_OTHER)


[grammar] ~130-~130: There might be a mistake here.
Context: ... the correct port (8080 in the demo_api example)

(QB_NEW_EN_OTHER)

🪛 markdownlint-cli2 (0.17.2)
QUICKSTART-DEPLOY.md

19-19: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)


31-31: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/2/3

(MD029, ol-prefix)


33-33: Ordered list item prefix
Expected: 2; Actual: 3; Style: 1/2/3

(MD029, ol-prefix)


35-35: Bare URL used

(MD034, no-bare-urls)


41-41: Ordered list item prefix
Expected: 1; Actual: 4; Style: 1/1/1

(MD029, ol-prefix)


63-63: Ordered list item prefix
Expected: 1; Actual: 3; Style: 1/1/1

(MD029, ol-prefix)


74-74: Fenced code blocks should have a language specified

(MD040, fenced-code-language)


85-85: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/2/3

(MD029, ol-prefix)


86-86: Ordered list item prefix
Expected: 2; Actual: 3; Style: 1/2/3

(MD029, ol-prefix)


88-88: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
  • GitHub Check: Test Agent Local / test_agent_local
  • GitHub Check: Build / Build
  • GitHub Check: Test API / API Test Local
  • GitHub Check: Test Packages / Test
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: Analyze (actions)
🔇 Additional comments (7)
go/deploy/Dockerfile.dev (1)

4-51: Excellent refactoring to multi-stage build pattern!

The separation of build stages for each service enables parallel compilation and better layer caching. Reusing the install stage for runtime is a smart optimization that avoids redundant package installations.

QUICKSTART-DEPLOY.md (1)

97-123: LGTM! Excellent documentation of the PORT requirement.

The examples clearly demonstrate how applications should read the PORT environment variable across different languages. This is crucial for successful deployments.

go/deploy/metald/internal/backend/docker/client.go (5)

228-238: Good defensive programming practice!

Adding the container existence check before starting prevents obscure runtime errors. This aligns well with the real RPC integration mentioned in the PR objectives.


627-631: Explicit configuration requirement improves clarity.

Removing the fallback to a default image and requiring explicit specification in metadata is a good design decision. This prevents unexpected behavior and makes deployments more predictable.


683-703: Robust image handling implementation.

The image pull logic correctly checks for local existence first and ensures the pull operation completes by reading the response. The logging at each step aids debugging.


828-868: Comprehensive port mapping extraction.

The implementation correctly parses Docker's port binding format and converts it to the protobuf structure. Good handling of both TCP and UDP protocols and multiple host bindings.


879-890: Improved port allocation strategy.

The random port selection reduces conflicts in concurrent deployment scenarios compared to linear scanning. The 100-attempt limit with informative error messaging is appropriate.

COPY --from=builder /app/main .

EXPOSE 8080
ENV PORT 8080
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Keep the EXPOSE instruction alongside ENV PORT for proper documentation.

The EXPOSE instruction serves as important metadata documentation for which ports the container listens on. While your application reads from the PORT environment variable, you should keep both instructions.

Apply this diff to maintain proper port documentation:

 ENV PORT 8080
+EXPOSE 8080
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
ENV PORT 8080
ENV PORT 8080
EXPOSE 8080
🤖 Prompt for AI Agents
In go/demo_api/Dockerfile at line 20, add an EXPOSE instruction for port 8080
alongside the existing ENV PORT 8080 line. This will document the port the
container listens on, improving clarity and container metadata without affecting
runtime behavior.

Comment on lines +192 to 218
// Create VM configuration for Docker backend
vmConfig := &vmprovisionerv1.VmConfig{
Cpu: &vmprovisionerv1.CpuConfig{
VcpuCount: 2,
MaxVcpuCount: 4,
Topology: nil,
Features: nil,
VcpuCount: 1,
},
Memory: &vmprovisionerv1.MemoryConfig{
SizeBytes: 2 * 1024 * 1024 * 1024, // 2GB
MaxSizeBytes: 8 * 1024 * 1024 * 1024, // 8GB
HotplugEnabled: true,
Backing: nil,
SizeBytes: 536870912, // 512MB
},
Boot: &vmprovisionerv1.BootConfig{
KernelPath: "/opt/vm-assets/vmlinux",
KernelArgs: "console=ttyS0 reboot=k panic=1 pci=off",
InitrdPath: "",
BootOptions: nil,
KernelPath: "/boot/vmlinux",
InitrdPath: "/boot/initrd",
KernelArgs: "console=ttyS0 quiet",
},
Storage: []*vmprovisionerv1.StorageDevice{{
Id: "rootfs",
Path: "/opt/vm-assets/rootfs.ext4",
ReadOnly: false,
IsRootDevice: true,
InterfaceType: "virtio-blk",
Options: map[string]string{
"docker_image": req.DockerImage,
"auto_build": "true",
},
Id: "root",
Path: "/dev/vda",
}},
Network: []*vmprovisionerv1.NetworkInterface{{
Id: "eth0",
InterfaceType: "virtio-net",
Mode: vmprovisionerv1.NetworkMode_NETWORK_MODE_DUAL_STACK,
Ipv4Config: &vmprovisionerv1.IPv4Config{
Dhcp: true,
Address: "",
Netmask: "",
Gateway: "",
DnsServers: nil,
},
Ipv6Config: &vmprovisionerv1.IPv6Config{
Slaac: true,
PrivacyExtensions: true,
Address: "",
PrefixLength: 0,
Gateway: "",
DnsServers: nil,
LinkLocal: "",
},
}},
Console: &vmprovisionerv1.ConsoleConfig{
Enabled: true,
Output: "/tmp/standard-vm-console.log",
Input: "",
ConsoleType: "serial",
},
Metadata: map[string]string{
"template": "standard",
"purpose": "general",
"docker_image": req.DockerImage,
"runtime": "docker",
"version_id": req.VersionID,
"workspace_id": req.WorkspaceID,
"project_id": req.ProjectID,
"created_by": "deploy-workflow",
"docker_image": req.DockerImage,
"exposed_ports": "8080/tcp",
"env_vars": "PORT=8080",
"version_id": req.VersionID,
"workspace_id": req.WorkspaceID,
"project_id": req.ProjectID,
"created_by": "deploy-workflow",
},
}
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove or update kernel/initrd paths for Docker backend.

The boot configuration specifies kernel and initrd paths that are not applicable to Docker containers. These paths (/boot/vmlinux, /boot/initrd) are for traditional VM booting, not container execution.

Since the metadata clearly indicates this is for Docker backend, consider:

 		// Create VM configuration for Docker backend
 		vmConfig := &vmprovisionerv1.VmConfig{
 			Cpu: &vmprovisionerv1.CpuConfig{
 				VcpuCount: 1,
 			},
 			Memory: &vmprovisionerv1.MemoryConfig{
 				SizeBytes: 536870912, // 512MB
 			},
-			Boot: &vmprovisionerv1.BootConfig{
-				KernelPath: "/boot/vmlinux",
-				InitrdPath: "/boot/initrd",
-				KernelArgs: "console=ttyS0 quiet",
-			},
 			Storage: []*vmprovisionerv1.StorageDevice{{
 				Id:   "root",
 				Path: "/dev/vda",
 			}},

Or if the Boot field is required by the API, use placeholder values that clearly indicate Docker usage.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
// Create VM configuration for Docker backend
vmConfig := &vmprovisionerv1.VmConfig{
Cpu: &vmprovisionerv1.CpuConfig{
VcpuCount: 2,
MaxVcpuCount: 4,
Topology: nil,
Features: nil,
VcpuCount: 1,
},
Memory: &vmprovisionerv1.MemoryConfig{
SizeBytes: 2 * 1024 * 1024 * 1024, // 2GB
MaxSizeBytes: 8 * 1024 * 1024 * 1024, // 8GB
HotplugEnabled: true,
Backing: nil,
SizeBytes: 536870912, // 512MB
},
Boot: &vmprovisionerv1.BootConfig{
KernelPath: "/opt/vm-assets/vmlinux",
KernelArgs: "console=ttyS0 reboot=k panic=1 pci=off",
InitrdPath: "",
BootOptions: nil,
KernelPath: "/boot/vmlinux",
InitrdPath: "/boot/initrd",
KernelArgs: "console=ttyS0 quiet",
},
Storage: []*vmprovisionerv1.StorageDevice{{
Id: "rootfs",
Path: "/opt/vm-assets/rootfs.ext4",
ReadOnly: false,
IsRootDevice: true,
InterfaceType: "virtio-blk",
Options: map[string]string{
"docker_image": req.DockerImage,
"auto_build": "true",
},
Id: "root",
Path: "/dev/vda",
}},
Network: []*vmprovisionerv1.NetworkInterface{{
Id: "eth0",
InterfaceType: "virtio-net",
Mode: vmprovisionerv1.NetworkMode_NETWORK_MODE_DUAL_STACK,
Ipv4Config: &vmprovisionerv1.IPv4Config{
Dhcp: true,
Address: "",
Netmask: "",
Gateway: "",
DnsServers: nil,
},
Ipv6Config: &vmprovisionerv1.IPv6Config{
Slaac: true,
PrivacyExtensions: true,
Address: "",
PrefixLength: 0,
Gateway: "",
DnsServers: nil,
LinkLocal: "",
},
}},
Console: &vmprovisionerv1.ConsoleConfig{
Enabled: true,
Output: "/tmp/standard-vm-console.log",
Input: "",
ConsoleType: "serial",
},
Metadata: map[string]string{
"template": "standard",
"purpose": "general",
"docker_image": req.DockerImage,
"runtime": "docker",
"version_id": req.VersionID,
"workspace_id": req.WorkspaceID,
"project_id": req.ProjectID,
"created_by": "deploy-workflow",
"docker_image": req.DockerImage,
"exposed_ports": "8080/tcp",
"env_vars": "PORT=8080",
"version_id": req.VersionID,
"workspace_id": req.WorkspaceID,
"project_id": req.ProjectID,
"created_by": "deploy-workflow",
},
}
// Create VM configuration for Docker backend
vmConfig := &vmprovisionerv1.VmConfig{
Cpu: &vmprovisionerv1.CpuConfig{
VcpuCount: 1,
},
Memory: &vmprovisionerv1.MemoryConfig{
SizeBytes: 536870912, // 512MB
},
Storage: []*vmprovisionerv1.StorageDevice{{
Id: "root",
Path: "/dev/vda",
}},
Metadata: map[string]string{
"docker_image": req.DockerImage,
"exposed_ports": "8080/tcp",
"env_vars": "PORT=8080",
"version_id": req.VersionID,
"workspace_id": req.WorkspaceID,
"project_id": req.ProjectID,
"created_by": "deploy-workflow",
},
}
🤖 Prompt for AI Agents
In go/apps/ctrl/services/version/deploy_workflow.go between lines 192 and 218,
the BootConfig includes kernel and initrd paths meant for traditional VMs, which
are not applicable for Docker containers. To fix this, either remove the Boot
field entirely if the API allows, or replace the KernelPath and InitrdPath with
placeholder values that clearly indicate Docker usage, such as empty strings or
descriptive placeholders, to avoid confusion and ensure compatibility with the
Docker backend.

Comment on lines +58 to +61
&cli.BoolFlag{
Name: "force-build",
Usage: "Force build Docker image even if --docker-image is provided",
},
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Remove unused --force-build flag.

The --force-build flag is defined but never used in the logic. This creates confusion for users.

Either implement the flag's functionality or remove it:

-		&cli.BoolFlag{
-			Name:  "force-build",
-			Usage: "Force build Docker image even if --docker-image is provided",
-		},
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
&cli.BoolFlag{
Name: "force-build",
Usage: "Force build Docker image even if --docker-image is provided",
},
🤖 Prompt for AI Agents
In go/cmd/version/main.go around lines 58 to 61, the --force-build flag is
defined but not used anywhere in the code, causing confusion. Remove the entire
definition of the --force-build flag from the CLI flags list to eliminate this
unused and misleading option.

Comment on lines +130 to +132
// Always build the image, ignoring any provided docker-image
dockerImage = ""

Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Don't silently ignore the --docker-image flag.

The code unconditionally clears the dockerImage variable, ignoring any value provided via the --docker-image flag. This contradicts the flag's documented purpose and creates a poor user experience.

Implement proper logic to respect the flag or update the flag's documentation:

-	// Always build the image, ignoring any provided docker-image
-	dockerImage = ""
+	// Use force-build flag to determine behavior
+	if cmd.Bool("force-build") {
+		dockerImage = ""
+	}

Alternatively, if the intention is to always build, remove the --docker-image flag entirely to avoid confusion.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In go/cmd/version/main.go around lines 130 to 132, the code unconditionally
clears the dockerImage variable, ignoring any value passed via the
--docker-image flag. To fix this, either remove the line that sets dockerImage
to an empty string so the flag value is respected, or if the intention is to
always build the image, remove the --docker-image flag entirely from the code
and documentation to avoid confusion.

Comment on lines 88 to 91
```
http://localhost:3000/versions
http://localhost:3000/deployments
```
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add language specifier to code block.

-```
+```text
 http://localhost:3000/versions
 http://localhost:3000/deployments

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>

88-88: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

In QUICKSTART-DEPLOY.md around lines 88 to 91, the code block showing URLs lacks
a language specifier, which affects syntax highlighting and readability. Add the
language specifier "text" immediately after the opening triple backticks to mark
the block as plain text, like ```text, and ensure the closing triple backticks
remain unchanged.


</details>

<!-- fingerprinting:phantom:medusa:lion -->

<!-- This is an auto-generated comment by CodeRabbit -->

Comment on lines +74 to +84
```
Deployment Complete
Version ID: v_xxxxxxxxxxxxxxxxxx
Status: Ready
Environment: Production

Domains
https://main-commit-workspace.unkey.app
http://localhost:35432
```

Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Add language specifier to code block.

-```
+```text
 Deployment Complete
   Version ID: v_xxxxxxxxxxxxxxxxxx
   Status: Ready
   Environment: Production
 
 Domains
   https://main-commit-workspace.unkey.app
   http://localhost:35432

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>

74-74: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

</details>

</details>

<details>
<summary>🤖 Prompt for AI Agents</summary>

In QUICKSTART-DEPLOY.md around lines 74 to 84, the code block showing deployment
details lacks a language specifier, which can affect syntax highlighting and
readability. Add the language specifier "text" immediately after the opening
triple backticks to indicate the content type, changing totext.


</details>

<!-- fingerprinting:phantom:medusa:lion -->

<!-- This is an auto-generated comment by CodeRabbit -->

```
http://localhost:3000/projects
```
Go to http://localhost:3000/projects
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Use proper markdown link syntax instead of bare URL.

-Go to http://localhost:3000/projects
+Go to [http://localhost:3000/projects](http://localhost:3000/projects)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Go to http://localhost:3000/projects
Go to [http://localhost:3000/projects](http://localhost:3000/projects)
🧰 Tools
🪛 LanguageTool

[grammar] ~35-~35: There might be a mistake here.
Context: ... the form: Go to http://localhost:3000/projects - Name: Choose any name (e.g., "My Test ...

(QB_NEW_EN_OTHER)

🪛 markdownlint-cli2 (0.17.2)

35-35: Bare URL used

(MD034, no-bare-urls)

🤖 Prompt for AI Agents
In QUICKSTART-DEPLOY.md at line 35, replace the bare URL
"http://localhost:3000/projects" with proper markdown link syntax by enclosing
the link text in square brackets and the URL in parentheses, for example:
[http://localhost:3000/projects](http://localhost:3000/projects).

Comment on lines +734 to 797
// Set up port bindings with retry logic
maxRetries := 5
for retry := 0; retry < maxRetries; retry++ {
// Clear previous port bindings
hostConfig.PortBindings = make(nat.PortMap)

// Allocate ports for this attempt
var allocatedPorts []int
portAllocationFailed := false

for i, mapping := range spec.PortMappings {
if mapping.HostPort == 0 {
// Allocate a new port
hostPort, err := d.portAllocator.allocatePort(spec.Labels["unkey.vm.id"])
if err != nil {
// Release any ports allocated in this attempt
for _, port := range allocatedPorts {
d.portAllocator.releasePort(port, spec.Labels["unkey.vm.id"])
}
portAllocationFailed = true
break
}
spec.PortMappings[i].HostPort = hostPort
allocatedPorts = append(allocatedPorts, hostPort)
}

containerPort := nat.Port(fmt.Sprintf("%d/%s", mapping.ContainerPort, mapping.Protocol))
hostConfig.PortBindings[containerPort] = []nat.PortBinding{
{
HostIP: "0.0.0.0",
HostPort: strconv.Itoa(spec.PortMappings[i].HostPort),
},
}
}

if portAllocationFailed {
continue // Try again with new ports
}
}

// Create container
containerName := d.config.ContainerPrefix + spec.Labels["unkey.vm.id"]
resp, err := d.dockerClient.ContainerCreate(ctx, config, hostConfig, nil, nil, containerName)
if err != nil {
span.RecordError(err)
return "", fmt.Errorf("failed to create container: %w", err)
// Create container
containerName := d.config.ContainerPrefix + spec.Labels["unkey.vm.id"]
resp, err := d.dockerClient.ContainerCreate(ctx, config, hostConfig, nil, nil, containerName)
if err != nil {
// If it's a port binding error, release ports and try again
if strings.Contains(err.Error(), "port is already allocated") || strings.Contains(err.Error(), "bind") {
for _, port := range allocatedPorts {
d.portAllocator.releasePort(port, spec.Labels["unkey.vm.id"])
}
d.logger.Warn("port binding failed, retrying with new ports", "error", err, "retry", retry+1)
continue
}
// Other errors are not retryable
span.RecordError(err)
return "", fmt.Errorf("failed to create container: %w", err)
}

// Success!
span.SetAttributes(attribute.String("container_id", resp.ID))
return resp.ID, nil
}

span.SetAttributes(attribute.String("container_id", resp.ID))
return resp.ID, nil
// If we get here, all retries failed
return "", fmt.Errorf("failed to create container after %d retries due to port conflicts", maxRetries)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Well-implemented retry logic for port conflicts.

The port binding retry mechanism properly handles conflicts by reallocating ports and cleaning up on failure. The implementation is robust with good error handling and logging.

Consider making the retry count configurable through DockerBackendConfig to allow tuning based on deployment environment port congestion.

🤖 Prompt for AI Agents
In go/deploy/metald/internal/backend/docker/client.go around lines 734 to 797,
the retry count for port binding conflicts is currently hardcoded as 5. Modify
the code to make this retry count configurable via a new field in
DockerBackendConfig, allowing different retry limits based on deployment
environment. Update the loop to use this configurable value instead of the fixed
maxRetries constant.

Comment on lines +566 to +569
DiskReadBytes: 0, // TODO: Calculate from BlkioStats
DiskWriteBytes: 0, // TODO: Calculate from BlkioStats
NetworkRxBytes: 0, // TODO: Calculate from NetworkStats
NetworkTxBytes: 0, // TODO: Calculate from NetworkStats
Copy link
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick (assertive)

Track the missing metrics implementation.

The TODO comments indicate that disk and network metrics calculations are not yet implemented. While returning 0 is acceptable for the initial implementation, these should be tracked for completion.

Would you like me to create an issue to track implementing these metrics calculations from Docker's BlkioStats and NetworkStats?

🤖 Prompt for AI Agents
In go/deploy/metald/internal/backend/docker/client.go around lines 566 to 569,
the DiskReadBytes, DiskWriteBytes, NetworkRxBytes, and NetworkTxBytes fields are
currently set to 0 with TODO comments indicating they should be calculated from
BlkioStats and NetworkStats. To fix this, implement the logic to extract and
calculate these metrics from the respective Docker stats structures instead of
returning zero. If immediate implementation is not possible, create a tracking
issue to ensure these metrics calculations are completed later.

@chronark chronark merged commit 252498e into main Jul 16, 2025
18 of 19 checks passed
@chronark chronark deleted the 07-15-feat_e2e_deploy_with_docker_works branch July 16, 2025 14:08
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.

2 participants