Skip to content
This repository has been archived by the owner on Oct 4, 2022. It is now read-only.

Copy-paste dagger-buildkitd setup code from dagger. #110

Merged
merged 1 commit into from
Sep 2, 2022

Conversation

sipsma
Copy link
Contributor

@sipsma sipsma commented Aug 25, 2022

This is 95% copy-paste, just some small adjustments. Let me know if there's any technical baggage I'm copying over from the previous implementation that we don't want. The only significant difference is that I am not starting with --net=host here since that seemed like a compromise earlier that we don't need to rush into making again.

Signed-off-by: Erik Sipsma erik@sipsma.dev

Also, there are log statements from before that don't work currently, but I think the fix there should be to add zerolog config to cloak rather than to remove the log statements. Also need to update docs w/ BUILDKIT_HOST info. I will update w/ both of those tomorrow.

@dolanor
Copy link
Contributor

dolanor commented Aug 26, 2022

The lint seems to be a false positive.
Though, it would cause a problem if somehow the debug.ReadBuildInfo() would fail to give the buildkit module version info and the version would be empty.
Main problem is debug.ReadBuildInfo() during go test, so that's already something to deal with. Maybe put some hardcoded value by default? (not that great, I agree): golang/go#33976

@dolanor
Copy link
Contributor

dolanor commented Aug 26, 2022

Hit this error while running go test ./...

?   	github.com/dagger/cloak/core/filesystem	[no test files]
#1 docker-image://docker.io/library/alpine:3.16.2
#1 resolve docker.io/library/alpine:3.16.2
--- FAIL: TestCoreImage (14.15s)
    core.schema_test.go:29: 
        	Error Trace:	/home/dolanor/src/cloak/core/integration/core.schema_test.go:29
        	Error:      	Received unexpected error:
        	            	rpc error: code = Unavailable desc = error reading from server: command [docker exec -i dagger-buildkitd buildctl dial-stdio] has exited with exit status 137, please make sure the URL is valid, and Docker 18.09 or later is installed on the remote host: stderr=
        	            	failed to receive status
        	            	github.com/moby/buildkit/client.(*Client).solve.func4
        	            		/home/dolanor/go/pkg/mod/github.com/moby/buildkit@v0.10.3/client/solve.go:264
        	            	golang.org/x/sync/errgroup.(*Group).Go.func1
        	            		/home/dolanor/go/pkg/mod/golang.org/x/sync@v0.0.0-20220722155255-886fb9371eb4/errgroup/errgroup.go:75
        	            	runtime.goexit
        	            		/home/dolanor/sdk/go1.18/src/runtime/asm_amd64.s:1571
        	Test:       	TestCoreImage
#1 git://github.com/dagger/dagger
#1 0.890 ref: refs/heads/main	HEAD
#1 0.896 db05c3d8abbd37964856a0eed25069079f9aa913	HEAD
#1 1.762 db05c3d8abbd37964856a0eed25069079f9aa913	refs/heads/main
#1 CACHED
FAIL
FAIL	github.com/dagger/cloak/core/integration	42.555s
?   	github.com/dagger/cloak/core/shim	[no test files]
?   	github.com/dagger/cloak/core/shim/cmd	[no test files]
?   	github.com/dagger/cloak/engine	[no test files]
?   	github.com/dagger/cloak/example/foo	[no test files]
?   	github.com/dagger/cloak/example/foo/gen/core	[no test files]
?   	github.com/dagger/cloak/example/leftpad	[no test files]
?   	github.com/dagger/cloak/example/leftpad/gen/core	[no test files]
?   	github.com/dagger/cloak/examples/alpine	[no test files]
?   	github.com/dagger/cloak/examples/alpine/gen/alpine	[no test files]
?   	github.com/dagger/cloak/examples/alpine/gen/core	[no test files]
?   	github.com/dagger/cloak/examples/netlify/go	[no test files]
?   	github.com/dagger/cloak/examples/netlify/go/gen/core	[no test files]
?   	github.com/dagger/cloak/examples/netlify/go/gen/netlify	[no test files]
#1 docker-image://docker.io/library/alpine:latest
#1 resolve docker.io/library/alpine:latest
--- FAIL: TestQueries (27.30s)
    queries_test.go:49: 
        	Error Trace:	/home/dolanor/src/cloak/examples/queries/queries_test.go:49
        	Error:      	Received unexpected error:
        	            	rpc error: code = Unavailable desc = error reading from server: command [docker exec -i dagger-buildkitd buildctl dial-stdio] has exited with exit status 137, please make sure the URL is valid, and Docker 18.09 or later is installed on the remote host: stderr=
        	            	failed to receive status
        	            	github.com/moby/buildkit/client.(*Client).solve.func4
        	            		/home/dolanor/go/pkg/mod/github.com/moby/buildkit@v0.10.3/client/solve.go:264
        	            	golang.org/x/sync/errgroup.(*Group).Go.func1
        	            		/home/dolanor/go/pkg/mod/golang.org/x/sync@v0.0.0-20220722155255-886fb9371eb4/errgroup/errgroup.go:75
        	            	runtime.goexit
        	            		/home/dolanor/sdk/go1.18/src/runtime/asm_amd64.s:1571
        	Test:       	TestQueries
        	Messages:   	file: multi.graphql
FAIL
FAIL	github.com/dagger/cloak/examples/queries	27.318s

@sipsma sipsma force-pushed the dagger-buildkitd branch 4 times, most recently from 8e302cb to d3fa877 Compare September 2, 2022 04:10
This is 95% copy-paste, just some small adjustments. Let me know if
there's any technical baggage I'm copying over from the previous
implementation that we don't want. The only significant difference is
that I am not starting with `--net=host` here since that seemed like a
compromise earlier that we don't need to rush into making again.

Signed-off-by: Erik Sipsma <erik@sipsma.dev>
@sipsma
Copy link
Contributor Author

sipsma commented Sep 2, 2022

The lint seems to be a false positive. Though, it would cause a problem if somehow the debug.ReadBuildInfo() would fail to give the buildkit module version info and the version would be empty. Main problem is debug.ReadBuildInfo() during go test, so that's already something to deal with. Maybe put some hardcoded value by default? (not that great, I agree): golang/go#33976

Thanks for the link! I worked around this for now by adding a testutil that gets the version information by shelling out to go list. Shelling out is ugly but it's okay-enough here because it's only done in tests and if you are running tests you almost certainly have access to go (technically not if you compiled a test binary, but I'm okay with not handling that situation for now). It's worth it to get test coverage against the actual buildkitd we setup for users (and coverage of the codepath that auto starts buildkitd).

@sipsma
Copy link
Contributor Author

sipsma commented Sep 2, 2022

cc @aluzzardi in case I'm accidentally copying anything over from dagger that we don't want here that I didn't notice. Also so you're aware of why there's some weird stuff going on with putting the package in internal and shelling out to go list when running in tests (golang/go#33976)

@aluzzardi
Copy link
Member

@sipsma Looks good.

Just to be aware: there's a flaw in dagger's buildkitd code; it adds start latency. That's because no matter what, it will docker inspect buildkitd and check if it's running the correct version (same as the vendored one). IIRC it'll add about half a second or so of extra start time to cloak.

I think it's probably fine to leave it so but we should log an issue

A possible workaround which we can do later would be, since we're already fiddling with ~/.config, to store in there the last buildkit version we've checked. If it's the same as the one in go.mod, then we skip version checking altogether.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants