✨ Rclone indirect transfer engine - #152
Conversation
|
Warning Review limit reached
Next review available in: 10 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughAdds an indirect transfer package that validates options, builds rclone pods with PVC and Secret mounts, performs cloud-storage uploads and downloads, checks pod completion, and cleans up labeled pods. ChangesIndirect transfer engine
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant PVC
participant IndirectTransfer
participant KubernetesClient
participant TransferPod
PVC->>IndirectTransfer: provide PVC metadata
IndirectTransfer->>KubernetesClient: create upload or download pod
KubernetesClient->>TransferPod: submit rclone pod
IndirectTransfer->>KubernetesClient: check pod status
KubernetesClient-->>IndirectTransfer: return completion status
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ 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.
Actionable comments posted: 5
🤖 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.
Inline comments:
In `@state_transfer/transfer/indirect/cleanup.go`:
- Around line 12-16: Prevent Cleanup from listing or deleting all namespace pods
when no labels are configured. Update New to normalize an empty Options.Labels
map as unset, or have IndirectTransfer.Cleanup reject empty labels before
calling client.List; preserve labeled cleanup behavior.
In `@state_transfer/transfer/indirect/indirect.go`:
- Around line 51-56: Update the label initialization in the transfer options
flow to apply defaults when Options.Labels is nil or empty, then add a unique
transfer-specific label before pod creation. Ensure Cleanup uses the resulting
labels with client.MatchingLabels(...) so deletion is restricted to Pods from
the current transfer rather than all namespace Pods.
- Around line 87-91: Update truncatePodName to preserve Kubernetes-valid, unique
names by reserving space for a deterministic hash suffix, trimming the retained
prefix so the result ends alphanumerically, and keeping the final length within
maxPodNameLen. Add tests covering a truncation boundary ending in “-” and
distinct long names sharing the same retained prefix.
- Line 13: Update the defaultImage constant to use the approved immutable image
digest instead of the mutable latest tag, preserving the existing transfer image
repository and default-image behavior.
In `@state_transfer/transfer/indirect/upload.go`:
- Around line 10-12: Validate required options at the start of
IndirectTransfer.Upload before constructing remotePath or the rclone command,
and return validation errors without calling Create; apply the same change at
state_transfer/transfer/indirect/upload.go lines 10-12 and
state_transfer/transfer/indirect/download.go lines 10-12 in
IndirectTransfer.Download, with tests asserting invalid options prevent Create.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 64bbf448-dede-4130-8f16-ea88b8dc6c5b
📒 Files selected for processing (5)
state_transfer/transfer/indirect/cleanup.gostate_transfer/transfer/indirect/download.gostate_transfer/transfer/indirect/indirect.gostate_transfer/transfer/indirect/indirect_test.gostate_transfer/transfer/indirect/upload.go
|
👍 |
Summary
Add indirect transfer engine for PVC data migration via S3-compatible cloud storage. This enables
transfer-pvcto migrate data between clusters without direct network connectivity, each cluster only needs outbound access to the object store.New package:
state_transfer/transfer/indirect/indirect.goupload.goUpload()creates rclone upload pod on source,IsUploadComplete()checks pod statusdownload.goDownload()creates rclone download pod on target,IsDownloadComplete()checks pod statuscleanup.goCleanup()deletes pods by label, tolerates already-deleted podsindirect_test.goDesign points
Transferinterface — the existing interface assumes server-client pairs with transport and endpoint, which don't apply herersync-transfercontainer image — rclone is compiled into it (separate PR on migtools/rsync-transfer)/dataand rclone config Secret at/etc/rclone--progress,--links(symlink handling),-vrclone.conf), not CLI flags — crane generates the config when--encryptis setCloudStorageandConfigSecretTests
TestBuildRcloneCommand(2 cases)TestBuildPodTestBuildPodLabelsAreCopiedTestTruncatePodNameTestDefaultImagequay.io/konveyor/rsync-transfer:latestTestDefaultLabelsTestOptionsValidation(4 cases)All existing crane-lib tests pass unchanged.
Summary by CodeRabbit
Summary by CodeRabbit