-
Notifications
You must be signed in to change notification settings - Fork 639
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
fix dockerfile local dir #1192
fix dockerfile local dir #1192
Conversation
70cd2cd
to
0bce666
Compare
func TestBuildWithDockerfile(t *testing.T) { | ||
testutil.RequiresBuild(t) | ||
base := testutil.NewBase(t) | ||
defer base.Cmd("builder", "prune").Run() |
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.
Q: Why do we need builder prune? Won't these slow our tests?
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.
cc @ktock
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.
@manugupt1 @junnplus This was to ensure all content (possibly leased by BuildKit) in the content store gets pruned after each test. Tests may check the behaviour of pulling blobs and contents remaining in the content store unexpectedly skip these operations.But yes, we may improve this by removing defer base.Cmd("builder", "prune").Run()
and instead adding rmiAll()
(defined in image_encrypt_linux_test.go
) to tests (if exist) requiring clean content store. Thanks for pointing this out :)
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.
thanks for the changes!
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
assert.NilError(t, err) | ||
defer os.Chdir(pwd) | ||
|
||
// hack os.Getwd return "(unreachable)" on rootless |
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.
How does this happen?
Can't find "unreachable" error in the code
https://cs.opensource.google/go/go/+/refs/tags/go1.18.3:src/os/getwd.go;l=22
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 actually don't know what happened, Getwd
returns a strange path during rootless testing.
(unreachable)/tmp/TestBuildWithDockerfile2096791215/001/text/Dockerfile
https://github.com/containerd/nerdctl/runs/7196804919?check_suite_focus=true#step:5:1004
Signed-off-by: Ye Sijun <junnplus@gmail.com>
0bce666
to
eb3715a
Compare
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.
Thanks
Signed-off-by: Ye Sijun junnplus@gmail.com
Fixes: #1191
In addition, check in advance whether the dockerfile file exists.
before:
after: