-
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: fix failed to run a container with specifying a non-exist workdir #2127
bugfix: fix failed to run a container with specifying a non-exist workdir #2127
Conversation
918b54e
to
1349d95
Compare
Codecov Report
@@ Coverage Diff @@
## master #2127 +/- ##
==========================================
+ Coverage 59.63% 61.71% +2.07%
==========================================
Files 208 208
Lines 16482 16498 +16
==========================================
+ Hits 9829 10181 +352
+ Misses 5453 5055 -398
- Partials 1200 1262 +62
|
|
||
c.Config.WorkingDir = filepath.Clean(c.Config.WorkingDir) | ||
|
||
path := filepath.Join(c.MountFS, c.Config.WorkingDir) |
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.
does runc
change user mode of the working dir?
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.
@fuweid , both pouch and runc do not change owner about workdir, since we do not have id map in pouch, create it as a root user with filemode 0755 is enough.
1349d95
to
68a64fa
Compare
68a64fa
to
514c2cf
Compare
514c2cf
to
9cfd5ef
Compare
@HusterWan , please make sure the following case is good, also a test is need
container 6b4914ad9e38 has workdir /home/tmp, when exec into it, workdir should be same, like
|
@Ace-Tang Good advices test below is ok :
but now, we not support set working directory when execute |
…ing directory Signed-off-by: Michael Wan <zirenwan@gmail.com>
9cfd5ef
to
52a7620
Compare
@Ace-Tang Updated, all test cases added |
LGTM |
Signed-off-by: Michael Wan zirenwan@gmail.com
Ⅰ. Describe what this PR did
Fix a problem that failed to run a container when specifying a not-exist working directory
Ⅱ. Does this pull request fix one issue?
Ⅲ. Why don't you add test cases (unit test/integration test)? (你真的觉得不需要加测试吗?)
add
TestRunWithExistWorkingDir
TestRunWithNotExistWorkingDir
Ⅳ. Describe how to verify it
Creating a container specifying a workdir not exist should success
Ⅴ. Special notes for reviews