Repository navigation
serve docker: fix volume path being lost when the plugin restarts - #9870
Merged
Merged
Conversation
applyOptions consumes the "path" option into vol.Path rather than leaving it in vol.Options, but restoreState rebuilt the options with only fs and type. The explicit path was therefore dropped when the plugin restarted, and since fsString is rebuilt from those options the volume was remounted at the root of the remote instead of at its subpath. Before this change a volume created with type + path lost its path completely, and one created with remote + path silently fell back to the path of the connection string. With a backend whose credentials are scoped to the subpath the restored mount then failed every operation rather than serving the wrong directory. Feed the persisted path back like fs and type, so applyOptions applies the same precedence on restore that it applies when the volume is first created. Fixes rclone#9853
1 task
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this change do?
rclone serve dockerpersists each volume'spathoption indocker-plugin.state, but never fed it back whenreloading.
applyOptionsconsumes"path"intovol.Pathrather than leaving it invol.Options, andrestoreStaterebuilt the options with onlyfsandtype, so the rebuiltfsStringlost the on-remote pathand the volume was remounted at the root of the remote.
Two volume shapes were affected:
fsStringafter a restart, beforetype+path:local:/tmp/path→:local:remote+path:local:/tmp/path→:local:/tmp/remoteThe second one is not in the issue. An explicit
pathoverrides the connection string's path when the volume iscreated, but on restore it silently reverted to the connection string's path. Feeding the persisted path back
unconditionally lets
applyOptionsapply the same precedence on restore that it applies at create time, and is ano-op for volumes that have no path.
For a backend whose credentials are scoped to the subpath — the reporter's case, S3 on Cloudflare R2 with a
bucket-scoped token — the restored mount does not merely serve the wrong directory: every operation at the new
root needs
ListBuckets, which is denied, so all I/O through the mount returnsEIOafter each reboot.The issue also reports that stale
mountsentries can block plugin start. That one is already fixed on master bythe deferred
pendingMountsrestore, so this PR only addresses the path.Testing
cmd/serve/docker/volume_test.goadds:TestRestoreStatePath— a table test over the four option combinations that can carry a path (type+path,remoteonly,remote+path,typeonly), round-tripping the volume through its JSON persisted form andasserting
fsStringcomes back unchanged.TestDriverRestoreStatePath— the same through a realdocker-plugin.statefile andDriver.restoreState.I checked the tests actually pin the behaviour by mutating the fix:
type_and_path,remote_and_pathandTestDriverRestoreStatePathfail, while the twoshapes that were never broken still pass;
vol.Path != "" && vol.Fs == "": onlyremote_and_pathfails, which is the case that wouldotherwise have been missed.
go build ./...,make quicktestandgolangci-lint run ./cmd/serve/docker/...all pass.No documentation change: the "Restarting or upgrading the plugin" section of
docker.mdalready describes volumesbeing restored across a restart, which this fixes rather than changes.
Linked issue
Fixes #9853
Checklist
test_allpasses for this backend — not a backend change.