Add redis-proxy Dockerfile, GHCR publishing workflow, and deployment …#367
Add redis-proxy Dockerfile, GHCR publishing workflow, and deployment …#367
Conversation
…guide for automated container deployment
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the deployability and usability of the redis-proxy by introducing a dedicated Dockerfile for containerization and a detailed deployment guide. These additions streamline the process of packaging, deploying, and operating the redis-proxy in various environments, facilitating its role in gradual migrations from Redis to ElasticKV. Highlights
Ignored Files
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request introduces a Dockerfile for the redis-proxy and a comprehensive deployment guide. The changes are a great addition. My review includes suggestions to improve the Dockerfile for better reproducibility and build performance by pinning base image versions and optimizing layer caching. For the deployment guide, I've pointed out that the Kubernetes example uses a command not available in the container and recommended using specific image tags in examples instead of latest to promote best practices.
| @@ -0,0 +1,11 @@ | |||
| FROM golang:latest AS build | |||
Dockerfile.redis-proxy
Outdated
|
|
||
| RUN CGO_ENABLED=0 go build -o /redis-proxy ./cmd/redis-proxy/ | ||
|
|
||
| FROM gcr.io/distroless/static:latest |
There was a problem hiding this comment.
It's a best practice to pin the base image version for the final stage instead of using latest to ensure your deployments are deterministic. Additionally, consider using the nonroot variant for enhanced security, as it runs the container as a non-privileged user.
FROM gcr.io/distroless/static:nonroot
| FROM golang:latest AS build | ||
|
|
||
| WORKDIR $GOPATH/src/app | ||
| COPY . . |
There was a problem hiding this comment.
To improve Docker build performance and leverage layer caching more effectively, you should copy go.mod and go.sum first, download dependencies, and then copy the rest of the source code. This prevents re-downloading dependencies every time any file in the repository changes. The structure should be:
COPY go.mod go.sum ./
RUN go mod download
COPY . .There was a problem hiding this comment.
Pull request overview
Adds end-to-end container packaging and deployment documentation for redis-proxy, enabling automated builds/publishing to GHCR and providing run examples for Docker/Compose/Kubernetes.
Changes:
- Added a deployment guide covering proxy modes, flags, metrics, alerts, and example manifests.
- Added a dedicated
Dockerfile.redis-proxyto build a distroless redis-proxy image. - Added a GitHub Actions workflow to build (and on
main, publish) the redis-proxy image to GHCR.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
docs/redis-proxy-deployment.md |
New deployment guide covering modes, configuration, observability, and deployment examples. |
Dockerfile.redis-proxy |
New multi-stage build for a distroless redis-proxy container image. |
.github/workflows/redis-proxy-docker.yml |
New workflow to build/push the redis-proxy image to GHCR on relevant changes. |
Dockerfile.redis-proxy
Outdated
|
|
||
| RUN CGO_ENABLED=0 go build -o /redis-proxy ./cmd/redis-proxy/ | ||
|
|
||
| FROM gcr.io/distroless/static:latest |
There was a problem hiding this comment.
The runtime image runs as root by default. For a network-facing proxy, consider switching to a non-root distroless variant (or setting an explicit unprivileged USER) to reduce impact if the process is compromised.
| FROM gcr.io/distroless/static:latest | |
| FROM gcr.io/distroless/static:nonroot |
docs/redis-proxy-deployment.md
Outdated
| Pre-built images are published to GitHub Container Registry on every push to `main`: | ||
|
|
||
| ``` | ||
| ghcr.io/bootjp/elastickv/redis-proxy:latest | ||
| ghcr.io/bootjp/elastickv/redis-proxy:sha-<commit> | ||
| ``` | ||
|
|
||
| The CI workflow (`.github/workflows/redis-proxy-docker.yml`) builds the image automatically when files under `cmd/redis-proxy/`, `proxy/`, or `Dockerfile.redis-proxy` change. | ||
|
|
There was a problem hiding this comment.
The guide says images are published “on every push to main”, but the workflow is path-filtered and will only run when specific files change. Please update this sentence to match the workflow behavior (or remove the path filters if the intent really is every push).
| ```bash | ||
| docker run --rm \ | ||
| ghcr.io/bootjp/elastickv/redis-proxy:latest \ | ||
| -listen :6379 \ | ||
| -primary redis.internal:6379 \ | ||
| -mode redis-only | ||
| ``` |
There was a problem hiding this comment.
In the “Minimal (redis-only)” docker run example, the container listens on :6379 but no -p/--publish is set, so the proxy won’t be reachable from the host in a typical local run. Consider adding the appropriate -p 6379:6379 mapping (or clarify that this example assumes a user-defined Docker network / --network host).
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
@copilot open a new pull request to apply changes based on the comments in this thread |
Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
Co-authored-by: bootjp <1306365+bootjp@users.noreply.github.com>
docs: add missing port mapping to minimal redis-proxy docker run example
Fix redis-proxy deployment docs: workflow trigger accuracy and docker run port mapping
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| | Mode | Reads from | Writes to | Use case | | ||
| |------|-----------|-----------|----------| | ||
| | `redis-only` | Redis | Redis only | Transparent proxy. Route traffic through the proxy first | | ||
| | `dual-write` | Redis | Redis + ElasticKV | Begin data sync. Populate ElasticKV | |
There was a problem hiding this comment.
The proxy modes table also starts with a double leading pipe (||), which will render an extra empty column. Use a single leading pipe for proper markdown table formatting.
| | Metric | Type | Description | | ||
| |--------|------|-------------| | ||
| | `proxy_command_total` | Counter | Commands processed (labels: command, backend, status) | | ||
| | `proxy_command_duration_seconds` | Histogram | Backend command latency | |
There was a problem hiding this comment.
The Key Metrics markdown table uses a double leading pipe (||), which typically renders as an extra empty column. Switch to a single leading pipe for the header and separator rows.
| | Parameter | Value | Description | | ||
| |-----------|-------|-------------| | ||
| | Connection pool size | 128 | go-redis pool size per backend | | ||
| | Dial timeout | 5s | Backend connection timeout | | ||
| | Read timeout | 3s | Backend read timeout | |
There was a problem hiding this comment.
The Internal Parameters markdown table also starts with a double leading pipe (||), which usually creates an empty first column. Use a single leading pipe for correct table formatting.
| images: ghcr.io/${{ github.repository }}/redis-proxy | ||
| tags: | | ||
| type=sha | ||
| type=ref,event=branch |
There was a problem hiding this comment.
This workflow publishes a branch tag (e.g., :main) via type=ref,event=branch. The deployment guide currently documents only :latest and :sha-...; either drop the branch tag here or update the docs so users know all published tags.
| type=ref,event=branch |
| | Flag | Default | Description | | ||
| |------|---------|-------------| | ||
| | `-listen` | `:6479` | Proxy listen address | | ||
| | `-primary` | `localhost:6379` | Primary (Redis) address | |
There was a problem hiding this comment.
The markdown table header uses a double leading pipe (|| ... |), which renders as an extra empty column in most Markdown viewers. Use a single leading pipe for the header and separator row (and apply the same fix to the other tables in this doc).
| Pre-built images are published to GitHub Container Registry when relevant files change on `main` (see path filters in the workflow): | ||
|
|
||
| ``` | ||
| ghcr.io/bootjp/elastickv/redis-proxy:latest | ||
| ghcr.io/bootjp/elastickv/redis-proxy:sha-<commit> | ||
| ``` |
There was a problem hiding this comment.
The docs list only latest and sha-... image tags, but the workflow configuration also publishes a branch tag (e.g., :main) via type=ref,event=branch. Either document the additional tag here or remove the branch tag from the workflow to match the docs.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…guide for automated container deployment