Repository navigation
Add rclone to rsync-transfer image - #75
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe module adds an rclone executable with crypt, local, S3, and sync support. The Docker build uses Go 1.25, disables CGO for existing builds, packages rclone, and validates the binaries with smoke tests. ChangesRclone integration
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related issues
Sequence Diagram(s)sequenceDiagram
participant DockerBuilder
participant GoToolchain
participant FinalImage
DockerBuilder->>GoToolchain: Build blockrsync, proxy, and rclone
GoToolchain->>FinalImage: Copy rclone to /usr/local/bin/rclone
DockerBuilder->>FinalImage: Run blockrsync, rclone, and rsync smoke tests
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
🧹 Nitpick comments (2)
Dockerfile (1)
30-30: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winUse a builder that satisfies the module’s Go version.
go.modrequires Go 1.25.0, but the builder at Line 3 remainsgolang:1.21. The new build at Line 30 relies on automatic toolchain switching. WithGOTOOLCHAIN=local, or without access to download the required toolchain, the build can stop before compiling rclone. Go treats thegodirective as a minimum requirement; automatic switching depends onGOTOOLCHAIN. (go.dev)Pin the builder to Go 1.25 or explicitly provision the required toolchain.
Proposed fix
-FROM docker.io/golang:1.21 as golang-builder +FROM docker.io/golang:1.25 as golang-builder🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` at line 30, Update the Dockerfile builder image used by the rclone build so it provides Go 1.25, matching the minimum version declared in go.mod. Change the existing builder configuration rather than relying on automatic toolchain downloads, and keep the RUN go build command unchanged.cmd/rclone/main.go (1)
1-1: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winPreserve rclone’s TLS compatibility setting in this executable.
This module is built on Go 1.25+ and does not inherit
github.com/rclone/rclone v1.75.0’s dependencygodebugdirectives. If compatibility with S3-compatible servers using negative certificate serial numbers is still required, add the directive to the rclone main package.Proposed fix
+//go:debug x509negativeserial=1 package main🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@cmd/rclone/main.go` at line 1, Update the rclone main package to declare the required godebug TLS compatibility setting for negative certificate serial numbers, matching the setting expected by rclone v1.75.0. Add it at package scope in cmd/rclone/main.go so the Go 1.25+ executable preserves this behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@cmd/rclone/main.go`:
- Line 1: Update the rclone main package to declare the required godebug TLS
compatibility setting for negative certificate serial numbers, matching the
setting expected by rclone v1.75.0. Add it at package scope in
cmd/rclone/main.go so the Go 1.25+ executable preserves this behavior.
In `@Dockerfile`:
- Line 30: Update the Dockerfile builder image used by the rclone build so it
provides Go 1.25, matching the minimum version declared in go.mod. Change the
existing builder configuration rather than relying on automatic toolchain
downloads, and keep the RUN go build command unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 09d1392c-2b28-49ce-ba0a-e83a2c80c105
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (3)
Dockerfilecmd/rclone/main.gogo.mod
There was a problem hiding this comment.
🧹 Nitpick comments (1)
Dockerfile (1)
28-30: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winKeep the
rcloneartifact statically linked.Lines 28-29 set
CGO_ENABLED=0, but line 30 does not. Go usesCGO_ENABLEDto control cgo-dependent build selection. (github.com)If the selected rclone dependencies produce a cgo-linked binary, the final minimal image can fail to execute. Set
CGO_ENABLED=0for the rclone build, or verify that the artifact has no dynamic dependencies for every supported architecture.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Dockerfile` around lines 28 - 30, Update the rclone build command in the Dockerfile to set CGO_ENABLED=0, matching the blockrsync and proxy builds, so the rclone artifact remains statically linked across supported architectures.Source: MCP tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@Dockerfile`:
- Around line 28-30: Update the rclone build command in the Dockerfile to set
CGO_ENABLED=0, matching the blockrsync and proxy builds, so the rclone artifact
remains statically linked across supported architectures.
|
Approved. Few minor claude suggestions:
|
|
Looks good, thanks. nitpick: |
Summary
Add rclone as a selectively-compiled Go binary to the rsync-transfer container image, enabling indirect PVC data migration via S3-compatible cloud storage.
Changes
New file:
cmd/rclone/main.goMinimal rclone binary with only the backends needed for indirect migration:
s3— S3-compatible storage (AWS, MinIO, Ceph, Noobaa)local— local filesystem (PVC mount)crypt— client-side encryption overlayOnly the
syncsubcommand is included.Modified:
DockerfileAdded one
go buildline and oneCOPYline following the existing pattern for blockrsync and proxy.Modified:
go.mod/go.sumAdded
github.com/rclone/rclonedependency.Binary sizes (Linux amd64)
Selective backend imports reduce the binary from ~80MB (full rclone) to 37MB.
What this enables
The rclone binary in the image allows future crane
transfer-pvc --cloud-storageto create pods that runrclone syncinstead ofrsync. No crane code changes in this PR — this only adds the tool to the image.Backward compatibility
Existing rsync and stunnel functionality is completely unaffected. The rclone binary is an addition, not a replacement.
Summary by CodeRabbit
New Features
rclonecommand-line utility to the Docker image.Improvements
rcloneexecutable is available at/usr/local/bin/rclonewithout additional installation.