-
Notifications
You must be signed in to change notification settings - Fork 950
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
bugfix: can't copy data with dr mode #2554
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2554 +/- ##
==========================================
- Coverage 69.15% 69.01% -0.14%
==========================================
Files 278 278
Lines 18574 18581 +7
==========================================
- Hits 12845 12824 -21
- Misses 4260 4279 +19
- Partials 1469 1478 +9
|
link to #2200 |
daemon/mgr/container_storage_test.go
Outdated
} | ||
|
||
mounts = sortMountPoint(mounts) | ||
if mounts[0].Destination != "/opt" || mounts[0].Source != "/pouch/volume5" { |
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 can prepare two arrays here. one is out of order, other one is in order. After sortMountPoint
, use loop to check each item. WDYT?
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.
Agree.
test/cli_run_volume_test.go
Outdated
res = command.PouchRun("exec", cname, "ls", "/var/spool") | ||
res.Assert(c, icmd.Success) | ||
if !strings.Contains(res.Stdout(), "mail") { | ||
c.Fatal() |
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.
fatal for what?
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 forgot.
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.
need to update test case.
@fuweid I have modified test case, PTAL again. 🍵 ☕️ |
use volume with dr mode, can't copy image's data to parent directory when sub-directory has copied. It causes the volume directory is not empty, so before copy image data, it need to sort mountpoint by the string length of destination directory. Signed-off-by: Rudy Zhang <rudyflyzhang@gmail.com>
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.
LGTM
Ⅰ. Describe what this PR did
use volume with dr mode, can't copy image's data to parent directory
when sub-directory has copied. It causes the volume directory is not
empty, so before copy image data, it need to sort mountpoint by the
string length of destination directory.
Ⅱ. Does this pull request fix one issue?
NONE
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)
add ut:
TestSortMountPoint
add integration test:
TestRunCopyDataWithDR
Ⅳ. Describe how to verify it
run test
TestRunCopyDataWithDR
passⅤ. Special notes for reviews
Signed-off-by: Rudy Zhang rudyflyzhang@gmail.com