[23.0 backport] builder-next: temporarily disable mergeop and diffop #45112

tonistiigi · 2023-03-06T22:40:39Z

(Partial, synthetic) backport of builder-next: disable mergeop and diffop #45153
Addresses mergeop/diffop not working properly with Moby worker #45111

Temporarily disable Merge/Diff until there is a proper solution to #45111 . Dockerfiles will detect the missing capability for COPY --link and fall back to the behavior in 20.10.

Changed to be a backport of a change to master for workflow reasons by @neersighted.

thaJeztah · 2023-03-07T09:35:31Z

Looks like changes are needed in the BuildKit integration tests to skip some of these (at least for now);

--- FAIL: TestIntegration (1.55s)
    --- FAIL: TestIntegration/TestDiffMergeOpaqueRegressionWithFileOverwrite/worker=dockerd (0.24s)
    --- FAIL: TestIntegration/TestDiffMergeOpaqueRegression/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffAsParentMultipleLayerDelete/worker=dockerd (0.26s)
    --- FAIL: TestIntegration/TestDiffAsParentMultipleLayers/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffAsParentSingleLayerDelete/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffAsParentSingleLayer/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffOfDiffsWithDeletes/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffOfDiffs/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffNestedLayeredMergeDeletes/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffNestedLayeredMerges/worker=dockerd (0.26s)
    --- FAIL: TestIntegration/TestDiffMultipleLayerOnMerge/worker=dockerd (1.26s)
    --- FAIL: TestIntegration/TestDiffSingleDeleteLayerOnMerge/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffSingleLayerOnMerge/worker=dockerd (0.26s)
    --- FAIL: TestIntegration/TestDiffOfMergesWithDeletes/worker=dockerd (3.27s)
    --- FAIL: TestIntegration/TestDiffOfMerges/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffOnlyMerge/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffExecMount/worker=dockerd (1.28s)
    --- FAIL: TestIntegration/TestDiffExecRoot/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffLazyBlobMerge/worker=dockerd (0.26s)
    --- FAIL: TestIntegration/TestDiffHardlinkChangesDoNotPropagateBetweenSnapshots/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffHardlinks/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffCircularParentDirSymlinks/worker=dockerd (1.25s)
    --- FAIL: TestIntegration/TestDiffCircularDirSymlink/worker=dockerd (1.28s)
    --- FAIL: TestIntegration/TestDiffCircularSymlinks/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffDeleteDoesNotFollowParentSymlink/worker=dockerd (1.26s)
    --- FAIL: TestIntegration/TestDiffDeleteDoesNotFollowSymlink/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffSymlinkOverridesSymlink/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffSymlinkOverridesDir/worker=dockerd (0.26s)
    --- FAIL: TestIntegration/TestDiffDirOverridesSymlink/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffCombinedMultiLayer/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffCombinedSingleLayer/worker=dockerd (1.28s)
    --- FAIL: TestIntegration/TestDiffChardevs/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffFifos/worker=dockerd (1.29s)
    --- FAIL: TestIntegration/TestDiffShuffleFilesMerge/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffShuffleFiles/worker=dockerd (1.29s)
    --- FAIL: TestIntegration/TestDiffOpaqueDirsMerge/worker=dockerd (1.28s)
    --- FAIL: TestIntegration/TestDiffOpaqueDirs/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffUnmatchedDelete/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffDeleteDirsMerge/worker=dockerd (1.28s)
    --- FAIL: TestIntegration/TestDiffDeleteDirs/worker=dockerd (1.27s)
    --- FAIL: TestIntegration/TestDiffDeleteFilesMerge/worker=dockerd (1.28s)
    --- FAIL: TestIntegration/TestDiffDeleteFiles/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestDiffOverrideFiles/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffOverrideDirs/worker=dockerd (1.28s)
    --- FAIL: TestIntegration/TestDiffModifiedDirs/worker=dockerd (0.25s)
    --- FAIL: TestIntegration/TestDiffNewDirs/worker=dockerd (0.30s)
    --- FAIL: TestIntegration/TestDiffModifiedFiles/worker=dockerd (0.29s)
    --- FAIL: TestIntegration/TestDiffNewFiles/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffLowerScratchDeletes/worker=dockerd (1.25s)
    --- FAIL: TestIntegration/TestDiffSelfDeletes/worker=dockerd (0.27s)
    --- FAIL: TestIntegration/TestDiffSelf/worker=dockerd (0.28s)
    --- FAIL: TestIntegration/TestMergeOp/worker=dockerd (0.25s)

I expect most of them are because the feature is disabled;

=== CONT  TestIntegration/TestMergeOp/worker=dockerd
    client_test.go:4736: 
        	Error Trace:	client_test.go:5194
        	            				client_test.go:4736
        	            				run.go:88
        	            				run.go:202
        	Error:      	Received unexpected error:
        	            	failed to load LLB: requested feature mergeop  has been disabled on the build server: only enabled with containerd image store backend
        	            	github.com/moby/buildkit/util/stack.Enable
        	            		/src/util/stack/stack.go:77
        	            	github.com/moby/buildkit/util/grpcerrors.FromGRPC
        	            		/src/util/grpcerrors/grpcerrors.go:188
        	            	github.com/moby/buildkit/util/grpcerrors.UnaryClientInterceptor
        	            		/src/util/grpcerrors/intercept.go:41
        	            	google.golang.org/grpc.(*ClientConn).Invoke
        	            		/src/vendor/google.golang.org/grpc/call.go:35
        	            	github.com/moby/buildkit/api/services/control.(*controlClient).Solve
        	            		/src/api/services/control/control.pb.go:1443
        	            	github.com/moby/buildkit/client.(*Client).solve.func2
        	            		/src/client/solve.go:208
        	            	golang.org/x/sync/errgroup.(*Group).Go.func1
        	            		/src/vendor/golang.org/x/sync/errgroup/errgroup.go:57
        	            	runtime.goexit
        	            		/usr/local/go/src/runtime/asm_amd64.s:1571
        	            	failed to solve
        	            	github.com/moby/buildkit/client.(*Client).solve.func2
        	            		/src/client/solve.go:221
        	            	golang.org/x/sync/errgroup.(*Group).Go.func1
        	            		/src/vendor/golang.org/x/sync/errgroup/errgroup.go:57
        	            	runtime.goexit
        	            		/usr/local/go/src/runtime/asm_amd64.s:1571
        	Test:       	TestIntegration/TestMergeOp/worker=dockerd

Also seeing a various of "broken pipe" errors;

2023-03-07T07:59:30.1724349Z === CONT  TestClientGatewayIntegration/TestClientGatewayContainerPlatformPATH/worker=dockerd
2023-03-07T07:59:30.1725215Z     client_test.go:5333: checkAllReleasable: skipping check for exported tars in non-containerd test
2023-03-07T07:59:30.1730573Z time="2023-03-07T07:59:30Z" level=warning msg="dockerd proxy error: write unix /tmp/buildkit-integration2940010673->@: write: broken pipe"

And some other errors (not sure if they are expected)

2023-03-07T08:01:24.0733870Z === CONT  TestIntegration/TestBuildHTTPSource/worker=dockerd
2023-03-07T08:01:24.3414310Z     client_test.go:5333: checkAllReleasable: skipping check for exported tars in non-containerd test
2023-03-07T08:01:24.3478855Z time="2023-03-07T08:01:24Z" level=warning msg="dockerd proxy error: read unix @->/tmp/integration1647505780/dx1c28m69anpr/docker.sock: read: connection reset by peer"
2023-03-07T08:01:24.3479569Z === CONT  TestIntegration/TestBuildMultiMount/worker=dockerd
2023-03-07T08:01:25.5113476Z     client_test.go:5333: checkAllReleasable: skipping check for exported tars in non-containerd test
2023-03-07T08:01:25.5118018Z time="2023-03-07T08:01:25Z" level=warning msg="dockerd proxy error: write unix /tmp/buildkit-integration235030185->@: use of closed network connection"
2023-03-07T08:01:25.5321868Z === CONT  TestIntegration/TestCallDiskUsage/worker=dockerd

tonistiigi · 2023-03-07T17:21:52Z

@thaJeztah Would you be ok with an internal environment variable that makes this pluggable so that tests can continue using this? I wouldn't want to regress on any of the current behavior that the tests are checking. Although I guess it could theoretically also mean that some tests could break only for users, and we don't discover it because tests use env switch.

thaJeztah · 2023-03-07T17:25:29Z

Ah, yes, some "DONT USE" env var would work for me

tonistiigi · 2023-03-11T02:03:37Z

I couldn't add an env without changing the buildkit vendor as well as dockerd is started by the testsuite (in a container, launched by script). I'll try to find something better for the master branch.

tonistiigi · 2023-03-13T15:50:22Z

This is green now.

tonistiigi · 2023-03-13T19:06:29Z

What's the difference between 52a6477 and da092c4 ?

neersighted · 2023-03-13T19:13:10Z

The commit message shows the commit on master that was cherry-picked.

tonistiigi · 2023-03-13T19:19:28Z

But it wasn't cherry-picked. The master codebase had already (about to be) diverged in this area via #44079 . The original description mentioned this.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com> Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com> (cherry picked from commit 0ac3bf8) Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>

Co-authored-by: Tonis Tiigi <tonistiigi@gmail.com> Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>

.github/workflows/buildkit.yml

neersighted · 2023-03-16T18:12:09Z

Known flake in Windows CI:

--- FAIL: TestLogs (46.53s)
    --- PASS: TestLogs/driver_local (0.02s)
        --- PASS: TestLogs/driver_local/without_tty/only_stderr (3.73s)
        --- PASS: TestLogs/driver_local/tty/stdout_and_stderr (4.06s)
        --- PASS: TestLogs/driver_local/without_tty/only_stdout (4.56s)
        --- PASS: TestLogs/driver_local/without_tty/stdout_and_stderr (4.33s)
        --- PASS: TestLogs/driver_local/tty/only_stderr (4.10s)
        --- PASS: TestLogs/driver_local/tty/only_stdout (4.37s)
    --- FAIL: TestLogs/driver_json-file (0.02s)
        --- PASS: TestLogs/driver_json-file/tty/stdout_and_stderr (4.22s)
        --- PASS: TestLogs/driver_json-file/without_tty/only_stdout (3.60s)
        --- PASS: TestLogs/driver_json-file/without_tty/stdout_and_stderr (3.70s)
        --- PASS: TestLogs/driver_json-file/tty/only_stderr (3.81s)
        --- PASS: TestLogs/driver_json-file/tty/only_stdout (3.74s)
        --- FAIL: TestLogs/driver_json-file/without_tty/only_stderr (33.73s)

tonistiigi changed the title ~~[23.0] builder-next: disable mergeop and diffop~~ [23.0] builder-next: temporarily disable mergeop and diffop Mar 6, 2023

cpuguy83 approved these changes Mar 7, 2023

View reviewed changes

thaJeztah added area/builder status/2-code-review impact/changelog area/builder/buildkit Issues affecting buildkit labels Mar 7, 2023

thaJeztah added this to the 23.0.2 milestone Mar 7, 2023

thaJeztah added the kind/bugfix PR's that fix bugs label Mar 7, 2023

thaJeztah added the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 7, 2023

AkihiroSuda linked an issue Mar 9, 2023 that may be closed by this pull request

[v23 regression] COPY --link breaks file caps (Apparently fixed in the master branch) #45128

Closed

tonistiigi force-pushed the 23.0-disable-mergeop-diffop branch from 2a5d679 to 52a6477 Compare March 11, 2023 01:34

neersighted changed the base branch from 23.0 to master March 13, 2023 18:38

neersighted requested review from thaJeztah, tianon, samuelkarp and johnstep as code owners March 13, 2023 18:38

neersighted changed the base branch from master to 23.0 March 13, 2023 18:38

neersighted mentioned this pull request Mar 13, 2023

builder-next: disable mergeop and diffop #45153

Merged

neersighted force-pushed the 23.0-disable-mergeop-diffop branch from 52a6477 to da092c4 Compare March 13, 2023 18:51

neersighted changed the title ~~[23.0] builder-next: temporarily disable mergeop and diffop~~ [23.0 backport] builder-next: temporarily disable mergeop and diffop Mar 13, 2023

neersighted removed the status/failing-ci Indicates that the PR in its current state fails the test suite label Mar 13, 2023

tonistiigi mentioned this pull request Mar 13, 2023

integration: enforce features compat through env vars moby/buildkit#3711

Merged

cpuguy83 approved these changes Mar 14, 2023

View reviewed changes

tonistiigi and others added 2 commits March 16, 2023 08:27

builder-next: disable mergeop and diffop

d43958b

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com> Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com> (cherry picked from commit 0ac3bf8) Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>

ci: disable testing mergeop and diffop with the dockerd worker

1363b3e

Co-authored-by: Tonis Tiigi <tonistiigi@gmail.com> Signed-off-by: Bjorn Neergaard <bneergaard@mirantis.com>

neersighted force-pushed the 23.0-disable-mergeop-diffop branch from da092c4 to 1363b3e Compare March 16, 2023 14:30

neersighted requested a review from crazy-max March 16, 2023 14:35

crazy-max reviewed Mar 16, 2023

View reviewed changes

.github/workflows/buildkit.yml Show resolved Hide resolved

crazy-max approved these changes Mar 16, 2023

View reviewed changes

corhere approved these changes Mar 16, 2023

View reviewed changes

neersighted merged commit 07f9061 into moby:23.0 Mar 16, 2023
85 of 86 checks passed

polarathene mentioned this pull request Apr 27, 2023

COPY --link does not rebase cached layers on top of changed layers docker/buildx#1099

Closed

tonistiigi mentioned this pull request Sep 16, 2023

Proposal: expose DiffOp in Dockerfile moby/buildkit#4239

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[23.0 backport] builder-next: temporarily disable mergeop and diffop #45112

[23.0 backport] builder-next: temporarily disable mergeop and diffop #45112

tonistiigi commented Mar 6, 2023 •

edited by neersighted

thaJeztah commented Mar 7, 2023

tonistiigi commented Mar 7, 2023 •

edited

thaJeztah commented Mar 7, 2023

tonistiigi commented Mar 11, 2023

tonistiigi commented Mar 13, 2023

tonistiigi commented Mar 13, 2023

neersighted commented Mar 13, 2023

tonistiigi commented Mar 13, 2023

neersighted commented Mar 16, 2023

[23.0 backport] builder-next: temporarily disable mergeop and diffop #45112

[23.0 backport] builder-next: temporarily disable mergeop and diffop #45112

Conversation

tonistiigi commented Mar 6, 2023 • edited by neersighted

thaJeztah commented Mar 7, 2023

tonistiigi commented Mar 7, 2023 • edited

thaJeztah commented Mar 7, 2023

tonistiigi commented Mar 11, 2023

tonistiigi commented Mar 13, 2023

tonistiigi commented Mar 13, 2023

neersighted commented Mar 13, 2023

tonistiigi commented Mar 13, 2023

neersighted commented Mar 16, 2023

tonistiigi commented Mar 6, 2023 •

edited by neersighted

tonistiigi commented Mar 7, 2023 •

edited