-
Notifications
You must be signed in to change notification settings - Fork 113
feat(docker): add containerization support with multi-stage build #120
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
base: main
Are you sure you want to change the base?
Conversation
Summary by CodeRabbit
WalkthroughThis pull request introduces Docker containerization support for Rogue. It adds a multi-stage Dockerfile that bundles Python dependencies and the TUI component, a Docker Compose configuration orchestrating server and UI services with health checks and persistent storage, comprehensive Docker documentation, and updates the README with Docker installation instructions. Changes
Sequence DiagramsequenceDiagram
actor User
participant rogue-server
participant rogue-ui
participant rogue-network
participant rogue-data
User->>rogue-server: docker-compose up
activate rogue-server
rogue-server->>rogue-network: connect
rogue-server->>rogue-data: mount /app/.rogue
rogue-server->>rogue-server: run healthcheck
note over rogue-server: waiting for healthy status
rogue-server-->>rogue-ui: service_healthy condition met
activate rogue-ui
rogue-ui->>rogue-network: connect
rogue-ui->>rogue-data: mount /app/.rogue
rogue-ui->>rogue-server: http://rogue-server:8000
note over rogue-ui: initialized and running
User->>rogue-server: access :8000
User->>rogue-ui: access :7860
deactivate rogue-ui
deactivate rogue-server
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
.dockerignore (1)
36-37: Broad .md exclusion warrants consideration if docs are runtime-needed.The pattern
*.mdon line 36 with the exception!README.mdis appropriate for most builds. However, verify that no other root-level markdown files (e.g.,DOCKER_USAGE.md,ARCHITECTURE.md) are referenced or needed within the container at runtime. If they are embedded in the application or needed for runtime operations, consider explicitly preserving them or reconsidering the scope of the exclusion.README.md (2)
118-179: Consolidate duplicated Docker sections.The README contains two Docker-focused sections with substantial overlap:
- Lines 118–179: "Option 3: Docker Installation" (procedural: how to install & run)
- Lines 479–537: "Docker Deployment" (architectural: deployment options)
Both cover similar ground (build command, quick start, component table, docker-compose reference), creating maintenance burden and potential for divergence. Consider consolidating these into a single, comprehensive section that covers both the procedural steps (from Option 3) and the architectural overview (from Deployment), keeping the distinction clear with subsections.
Alternatively, if both perspectives serve distinct audiences, clearly differentiate their purpose in introductory text to avoid reader confusion.
Also applies to: 479-537
533-534: Wrap bare URLs in inline code formatting.Per Markdown best practices (markdownlint MD034), bare URLs should be wrapped in backticks or angle brackets to improve rendering and accessibility. This also applies to lines 625–627 in the Quick Reference table.
Apply this diff:
-**Access:** -- API Docs: http://localhost:8000/docs -- Web UI: http://localhost:7860 +**Access:** +- API Docs: `http://localhost:8000/docs` +- Web UI: `http://localhost:7860`Similarly, update the Quick Reference table (lines 625–627):
| Server | 8000 | `http://localhost:8000/docs` | | Web UI | 7860 | `http://localhost:7860` | | Example Agent | 10001 | `http://localhost:10001` |docker-compose.yml (1)
35-35: Clarify purpose ofGRADIO_SERVER_NAME=0.0.0.0.Line 35 sets
GRADIO_SERVER_NAME=0.0.0.0for the rogue-ui container. This tells Gradio to listen on all network interfaces within the container, which is necessary for Docker networking but may conflict with or complement the port mapping on line 37.Document why this is needed (e.g., "Required for Gradio to accept external connections in Docker") or verify it doesn't cause unexpected behavior with Gradio's security model.
Add an inline comment to clarify:
environment: - OPENAI_API_KEY=${OPENAI_API_KEY:-} - - GRADIO_SERVER_NAME=0.0.0.0 + - GRADIO_SERVER_NAME=0.0.0.0 # Required for Gradio to listen on all interfaces
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.dockerignore(1 hunks)DOCKER_USAGE.md(1 hunks)Dockerfile(1 hunks)README.md(2 hunks)docker-compose.yml(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
DOCKER_USAGE.md
89-89: Bare URL used
(MD034, no-bare-urls)
146-146: Bare URL used
(MD034, no-bare-urls)
237-237: Bare URL used
(MD034, no-bare-urls)
238-238: Bare URL used
(MD034, no-bare-urls)
625-625: Bare URL used
(MD034, no-bare-urls)
626-626: Bare URL used
(MD034, no-bare-urls)
627-627: Bare URL used
(MD034, no-bare-urls)
README.md
533-533: Bare URL used
(MD034, no-bare-urls)
534-534: Bare URL used
(MD034, no-bare-urls)
🔇 Additional comments (3)
DOCKER_USAGE.md (1)
1-627: Comprehensive and well-structured Docker documentation.DOCKER_USAGE.md is thorough and well-organized, covering all major use cases from basic runs to advanced troubleshooting. The inclusion of Windows-specific guidance (PowerShell syntax), real-world pain points (port conflicts, permissions, memory), and practical examples (host.docker.internal, named volumes) demonstrates consideration for diverse user environments.
Strengths:
- Clear progression from basic to advanced scenarios
- Practical troubleshooting section with concrete solutions
- Good networking and storage guidance
- Well-explained CLI flags and options
Optional linting note:
Bare URLs throughout (lines 89, 146, 237–238, 625–627) are flagged by markdownlint (MD034). While acceptable for documentation, wrapping them in backticks would improve consistency:- **Server API:** `http://localhost:8000/docs` - **Web UI:** `http://localhost:7860`This is a minor stylistic preference and not required for functionality.
docker-compose.yml (1)
21-26: No issues found—healthcheck endpoint is properly implemented and configured.The
/api/v1/healthendpoint exists inrogue/server/api/health.pyand returns HTTP 200 with a reliable response. The docker-compose healthcheck will function as expected.Dockerfile (1)
90-90: All verification concerns are valid and properly implemented.
- Entry point availability:
pyproject.tomldefinesrogue-ai = "rogue.__main__:main", andpip install -e .in the Dockerfile installs it correctly into PATH.- Default behavior verified:
rogue/__main__.pyshows that runningrogue-aiwithout arguments (whenargs.mode is None) installs TUI, starts server in background, and launches the TUI—matching the documented default.- Non-interactive Docker requirement documented:
DOCKER_USAGE.mdexplicitly states "-it: Interactive terminal (required for TUI)" and provides examples requiring-itflags for default mode, plus separate instructions for server-only mode without TUI.The
CMD ["rogue-ai"]is correct and all documented prerequisites are in place.
| build: . | ||
| image: rogue-app:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add build context to rogue-ui service for image consistency.
The rogue-server service (line 4) has build: ., but rogue-ui (line 30) only specifies image: rogue-app:latest without a build context. This creates an inconsistency:
- If rogue-app:latest doesn't exist locally,
docker-compose upwill fail for rogue-ui with an image-not-found error. - Build-time updates (e.g., source changes) will only rebuild for rogue-server, leaving rogue-ui stale.
- First-time users may be confused why rogue-server builds but rogue-ui doesn't.
Both services use the same image, so both should trigger the build process in a compose workflow.
Apply this diff:
# Rogue Web UI - Gradio interface
rogue-ui:
+ build: .
image: rogue-app:latest
command: rogue-ai ui --port 7860 --rogue-server-url http://rogue-server:8000Alternatively, if rogue-ui should intentionally use a pre-built image from a registry, document this clearly and remove the image tag to avoid confusion.
Also applies to: 30-30
🤖 Prompt for AI Agents
In docker-compose.yml around lines 4-5 and line 30, rogue-server is configured
with build: . while rogue-ui only has image: rogue-app:latest, causing
inconsistent behavior and possible image-not-found or stale UI builds; fix by
adding build: . to the rogue-ui service so both services build from the same
local context (you may keep image: rogue-app:latest to tag the built image), or
if rogue-ui should intentionally come from a registry, remove the local build
and add a clear comment documenting that it must be pulled from the registry.
| COPY . . | ||
|
|
||
| # Install the local packages in editable mode (same as builder stage) | ||
| RUN pip install -e ./sdks/python | ||
| RUN pip install -e . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove redundant package reinstall in final stage.
Lines 83–84 reinstall local packages using pip, but these packages were already installed in the builder stage (lines 54–55) and their site-packages have been copied to the final image (line 70). This reinstallation is redundant, adds unnecessary build time, and introduces inconsistency by switching from uv to pip.
Since the entire source code is already present in the container (line 80), the pre-installed editable installs in site-packages should remain valid.
Apply this diff to remove the redundant reinstall:
# Copy the application source code
COPY . .
-# Install the local packages in editable mode (same as builder stage)
-RUN pip install -e ./sdks/python
-RUN pip install -e .
-
# Expose port 8000 (default server port)
EXPOSE 8000If the reinstall is genuinely needed (e.g., due to path resolution issues with editable installs), please document why and consider using uv for consistency.
📝 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.
| COPY . . | |
| # Install the local packages in editable mode (same as builder stage) | |
| RUN pip install -e ./sdks/python | |
| RUN pip install -e . | |
| COPY . . | |
🤖 Prompt for AI Agents
In Dockerfile around lines 80 to 84, remove the redundant reinstall steps:
delete the two RUN pip install -e ./sdks/python and RUN pip install -e . lines
because the editable packages were already installed in the builder stage and
their site-packages were copied into the final image; if there is a concrete
reason the reinstall is required (e.g., editable install path resolution),
document that in a comment and, for consistency with the builder stage, perform
the install using the same tool (`uv`) rather than switching to `pip`.
| | Mode | Command | Use Case | | ||
| |------|---------|----------| | ||
| | **Default** | `docker run -it --rm -p 8000:8000 rogue-app` | Interactive TUI | | ||
| | **Server** | `docker run -d -p 8000:8000 rogue-app rogue-ai server` | Backend only | | ||
| | **Web UI** | `docker run -d -p 7860:7860 -p 8000:8000 rogue-app rogue-ai ui` | Web interface | | ||
| | **CLI** | `docker run --rm rogue-app rogue-ai cli [OPTIONS]` | CI/CD pipelines | | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the tui also supported?
|
|
||
| # Build the TUI binary for Linux amd64 | ||
| WORKDIR /app/packages/tui | ||
| RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should not hard-code the architecture. We should take it from the result of uname -m.
Without this, building the docker on ARM machines (any MacBook with Apple Silicon for example) will fail since the arch of the base image (golang:1.23-alpine) will match the host's architecture (unless specified otherwise)
| RUN CGO_ENABLED=0 GOOS=linux GOARCH=amd64 go build \ | |
| RUN CGO_ENABLED=0 GOOS=linux GOARCH=$(uname -m) go build \ |
| -o dist/rogue-tui ./cmd/rogue | ||
|
|
||
| # Stage 2: Install Python dependencies using uv | ||
| FROM python:3.11-slim AS builder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| FROM python:3.11-slim AS builder | |
| FROM python:3.13-slim AS builder |
| RUN uv pip install -e . | ||
|
|
||
| # Stage 3: Create the final runtime image | ||
| FROM python:3.11-slim AS final |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| FROM python:3.11-slim AS final | |
| FROM python:3.13-slim AS final |
| RUN uv pip install -e . | ||
|
|
||
| # Stage 3: Create the final runtime image | ||
| FROM python:3.11-slim AS final |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we need this final layer. We can simply copy the tui to the previous builder layer.
| # Install uv | ||
| RUN pip install uv | ||
|
|
||
| # Set environment variables for uv | ||
| ENV UV_NO_INTERACTION=1 \ | ||
| UV_VIRTUALENVS_CREATE=false | ||
|
|
||
| # Set working directory | ||
| WORKDIR /app | ||
|
|
||
| # Copy dependency files | ||
| COPY pyproject.toml uv.lock ./ | ||
|
|
||
| # Copy the VERSION file (required for dynamic version) | ||
| COPY VERSION ./ | ||
|
|
||
| # Copy the README.md file (required for package metadata) | ||
| COPY README.md ./ | ||
|
|
||
| # Copy the SDK directory (required for the local dependency) | ||
| COPY sdks/python/ ./sdks/python/ | ||
|
|
||
| # Copy the main source code (needed for editable install) | ||
| COPY rogue/ ./rogue/ | ||
|
|
||
| # Install dependencies (excluding dev dependencies) | ||
| RUN uv sync --frozen --no-dev | ||
|
|
||
| # Install the local packages in editable mode | ||
| RUN uv pip install -e ./sdks/python | ||
| RUN uv pip install -e . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we don't build the package here, can we just do this?
| # Install uv | |
| RUN pip install uv | |
| # Set environment variables for uv | |
| ENV UV_NO_INTERACTION=1 \ | |
| UV_VIRTUALENVS_CREATE=false | |
| # Set working directory | |
| WORKDIR /app | |
| # Copy dependency files | |
| COPY pyproject.toml uv.lock ./ | |
| # Copy the VERSION file (required for dynamic version) | |
| COPY VERSION ./ | |
| # Copy the README.md file (required for package metadata) | |
| COPY README.md ./ | |
| # Copy the SDK directory (required for the local dependency) | |
| COPY sdks/python/ ./sdks/python/ | |
| # Copy the main source code (needed for editable install) | |
| COPY rogue/ ./rogue/ | |
| # Install dependencies (excluding dev dependencies) | |
| RUN uv sync --frozen --no-dev | |
| # Install the local packages in editable mode | |
| RUN uv pip install -e ./sdks/python | |
| RUN uv pip install -e . | |
| # Install the local packages in editable mode | |
| RUN pip install -e ./sdks/python | |
| RUN pip install -e . |
| docker build --no-cache -t rogue-app . | ||
|
|
||
| # Specific platform | ||
| docker build --platform linux/amd64 -t rogue-app . |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is not currently supported in the docker file (the os and arch are static in the go build)
Phoenix4201
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how it all works can sosome one help me i watch as many times as vids as I can but you guys are doinggreat
|
Hi @puwun, is there any update regarding this PR? |
Thanks @Phoenix4201, Please feel free to join our Discord community! |
|
@yuval-qf im not working on this currently, so it's up for grabs |
Description
Dockerfilewith TUI binary compilation and Python dependency installationdocker-compose.ymlfor production-ready multi-service deploymentDOCKER_USAGE.mdwith operational guide.dockerignoreto optimize build contextREADME.mdwith Docker installation option and quick startType of Change
✨ New feature (non-breaking change which adds functionality)
📝 Documentation update
🔧 Configuration/build changes
Screenshots/Examples (if applicable)
Quick Start Commands
Build and run with TUI:
Production deployment:
docker-compose up -d # Access: http://localhost:8000/docs (API) and http://localhost:7860 (Web UI)Related Issues/PRs