-
Notifications
You must be signed in to change notification settings - Fork 611
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
Support Docker 23.0 compatibility #2427
Support Docker 23.0 compatibility #2427
Conversation
75bfa8a
to
b4cd36f
Compare
cmd/nerdctl/builder_build_test.go
Outdated
t.Run("InvalidBuildArgCausesError", func(t *testing.T) { | ||
base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", "=TEST_STRING").AssertFail() | ||
t.Run("InvalidBuildArgNotCausesError", func(t *testing.T) { | ||
base.Cmd("build", buildCtx, "-t", imageName, "--build-arg", "=TEST_STRING").AssertOK() |
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.
Seems to be a bug of Docker
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.
Set Buildx and BuildKit as the default builder on Linux. moby/moby#43992 from https://docs.docker.com/engine/release-notes/23.0/.
So there are some change of the build-args.
Should the test be removed , because it may be a bug :-)
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.
0003448
to
0844a51
Compare
HI @AkihiroSuda Would you please review the PR again ? :-) |
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: Kay Yan <kay.yan@daocloud.io>
Thanks @AkihiroSuda for the PR review :-) |
base.Cmd("volume", "create", tID+"-1").AssertOK() | ||
base.Cmd("volume", "create", tID+"-2").AssertOK() | ||
|
||
base.Cmd("run", "-v", fmt.Sprintf("%s:/volume", tID+"-1"), "--name", tID, testutil.CommonImage).AssertOK() | ||
defer base.Cmd("rm", "-f", tID).Run() | ||
|
||
base.Cmd("volume", "prune", "-f").AssertOutContains(tID + "-2") | ||
base.Cmd("volume", "prune", "-f").AssertOutContains(vID) | ||
base.Cmd("volume", "prune", "-a", "-f").AssertOutContains(tID + "-2") |
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.
Curious is this a breaking change which changes the behaviour of nerdctl volume prune -f
without -a
? Or any breaking change which is for Docker compliance is not considered as breaking change in Nerdctl?
Docker bumped major version with this change so fine in Docker side. https://docs.docker.com/engine/release-notes/23.0/#bug-fixes-and-enhancements-1
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.
This seems too subtle to bump up the major version for nerdctl
Support Docker 23.0 compatibility , to fix #2421 .
nerdctl volume prune -a
nerdctl system prune --volume
behavior to skip the name volume.InvalidBuildArgCausesError