Use dagger for build and test pipeline #4186

kpenfound · 2023-07-13T15:12:49Z

This PR changes the test CI workflow to use a dagger pipeline. The net result is that dependencies required to run the full test suite locally are reduced to just Go and Docker. Runtimes locally should be significantly reduced because of Dagger's caching abilities, and the same can be applied in CI with some infrastructure changes.

This work was previously seen in a demo of this from equinix demo day

TODO:

tidy up builder.go
feedback on structure
tests for the ci pipeline

caarlos0

I don't know enough about dagger (its on my to study list, appreciate if you have any good material to point me to), but besides a couple of comments, looks good to me.

the lint issues can be solved by running gofumpt

caarlos0 · 2023-07-13T17:14:19Z

go.mod

+// DO NOT UPDATE THIS!! needs to be v0.0.0-20210512092938-c05353c2d58c
+require github.com/ProtonMail/go-crypto v0.0.0-20230626094100-7e9e0395ebec // indirect


can ignore the // DO NOT UPDATE thing, its outdated...

PS: I'll fix this on main later to not conflict with this pr :)

caarlos0 · 2023-07-13T17:15:42Z

ci/main.go

+
+	builder = builder.WithExec([]string{"go", "mod", "tidy"}).
+		WithExec([]string{"go", "build"}).
+		WithExec([]string{"go", "test", "./..."})


in the original pipeline, this would call ./script/test.sh, which also appends to $GITHUB_STEP_SUMMARY... not sure if that's possible with dagger too..

Yeah that should be possible, I'll try it out

caarlos0 · 2023-07-13T17:16:12Z

Taskfile.yml

-    env:
-      LC_ALL: C
    vars:
-      TEST_OPTIONS: '{{default "" .TEST_OPTIONS}}'
-      SOURCE_FILES: '{{default "./..." .SOURCE_FILES}}'
-      TEST_PATTERN: '{{default "." .TEST_PATTERN}}'
    cmds:
-      - go test {{.TEST_OPTIONS}} -failfast -race -coverpkg=./... -covermode=atomic -coverprofile=coverage.txt {{.SOURCE_FILES}} -run {{.TEST_PATTERN}} -timeout=5m


I would leave this as is, maybe add a ci task, that calls go run ./ci

caarlos0 · 2023-07-13T17:17:10Z

go.mod

@@ -4,6 +4,7 @@ go 1.20

 require (
 	code.gitea.io/sdk/gitea v0.15.1
+	dagger.io/dagger v0.7.2


can we make ./ci its own module? so whoever go install goreleaser don't need to download dagger and all this other deps too...

Yeah good call

gerhard · 2023-07-13T17:47:47Z

Thank you for reviewing this @caarlos0!

We gave you a shout-out in today's Dagger Community Call (https://dagger.io/events fwiw). Recording will be uploaded to YouTube in a day (or a few at most). Will drop a link when it's public. cc @jpadams @mircubed @d3rp3tt3

Thank you @marcosnils for helping kick this off 💪

marcosnils · 2023-07-13T19:10:27Z

ci/builder.go

+		WithExec([]string{"chown", "-R", "nonroot", "/src"}).
+		WithNewFile("/usr/local/sbin/docker", dagger.ContainerWithNewFileOpts{
+			Contents: `#!/bin/sh
+DOCKER_HOST=tcp://localhost:2375 /usr/bin/docker $@`,


@kpenfound given that @caarlos0 seems to be quite responsive to merge this maybe we can take the opportunity to make this change in goreleaser so this hack is not necessary?

@caarlos0 FWIW we had to do this beacuse goreleaser tests seem to overriding env variables when calling docker

Agreed. Originally this code ran outside of the goreleaser repo so it was running the tests without modifications. A few workarounds were necessary to accommodate that. In this case, there's a few tests where localhost is hardcoded and overriding any DOCKER_HOST env

hmmm, totally, feel free to make the necessary changes! <3

@caarlos0 basically goreleaser tests are overriding go test envs since they're being set here https://github.com/goreleaser/goreleaser/blob/7c6201f5f588fa1a8504f5cffa6264646fdee1eb/internal/pipe/docker/docker_test.go#LL992C17-L992C17 and then used here

goreleaser/internal/pipe/docker/api.go

Line 51 in 7c6201f

cmd.Env = ctx.Env.Strings()

.

The change that would help us to remove this hack is if goreleaser appended ctx.Env envs to the current existing variables here

goreleaser/internal/pipe/docker/api.go

Line 51 in 7c6201f

cmd.Env = ctx.Env.Strings()

instead of overriding them altogether. WDYT?

I think it's probably the right thing to do, and what most people would expect to happen.

Honestly kinda surprised no one reported that as a bug yet 🤔

I think it's probably the right thing to do, and what most people would expect to happen.

Honestly kinda surprised no one reported that as a bug yet thinking

#4187

This is causing issues when trying to run tests in Dagger. ref: goreleaser#4186 (comment) Signed-off-by: Marcos Lilljedahl <marcosnils@gmail.com>

This is causing issues when trying to run tests in Dagger. ref: #4186 (comment) Signed-off-by: Marcos Lilljedahl <marcosnils@gmail.com> Signed-off-by: Marcos Lilljedahl <marcosnils@gmail.com>

mircubed · 2023-07-17T23:36:39Z

Thanks again @caarlos0 ! You can check out the recording for the shout out here (13:20 timestamp) :) https://www.youtube.com/watch?v=dRfcSIIAvG4

caarlos0 · 2023-07-18T00:30:51Z

Thanks again @caarlos0 ! You can check out the recording for the shout out here (13:20 timestamp) :) https://www.youtube.com/watch?v=dRfcSIIAvG4

Pretty cool! Thanks for the mention and the PR 💜

caarlos0 · 2023-07-18T12:26:28Z

one Q about this: can we automate updates, like we do with dependabot?

if not, is there any plans to do so?

caarlos0 · 2024-01-14T18:38:38Z

hey, thanks for all the effort on this, but I think I'll leave it as-is for now.

marcosnils · 2024-01-14T18:50:25Z

:( sent from mobile Em dom., 14 de jan. de 2024 15:38, Carlos Alexandro Becker < ***@***.***> escreveu:

…

Closed #4186 <#4186>. — Reply to this email directly, view it on GitHub <#4186 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAMBLWQA4FLOVKEKGZINNNLYOQQ3VAVCNFSM6AAAAAA2JDDME6VHI2DSMVQWIX3LMV45UABCJFZXG5LFIV3GK3TUJZXXI2LGNFRWC5DJN5XDWMJRGQ4DCOJQGU4DKMA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

kpenfound · 2024-01-18T17:16:54Z

hey, thanks for all the effort on this, but I think I'll leave it as-is for now.

sounds good @caarlos0 ! Maybe we can take another run at this with a future version of Dagger :)

kpenfound added 4 commits July 11, 2023 14:54

merge main

616a22a

update contributing docs

a81affc

update build workflow yaml

d56f174

go mod tidy

cb59e63

pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Jul 13, 2023

caarlos0 reviewed Jul 13, 2023

View reviewed changes

marcosnils reviewed Jul 13, 2023

View reviewed changes

marcosnils mentioned this pull request Jul 13, 2023

ci: append envs when calling docker instead of overriding #4187

Merged

kpenfound added 3 commits July 14, 2023 16:00

merge main

957d670

Merge branch 'main' into dagger_ci

4cfed24

new task for daggerci

4bddbdb

caarlos0 added the enhancement New feature or request label Jul 18, 2023

kpenfound added 2 commits July 20, 2023 13:08

update dagger sdk

3720e93

formatting changes

49eacef

caarlos0 closed this Jan 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use dagger for build and test pipeline #4186

Use dagger for build and test pipeline #4186

kpenfound commented Jul 13, 2023

caarlos0 left a comment

caarlos0 Jul 13, 2023

caarlos0 Jul 13, 2023

caarlos0 Jul 13, 2023

kpenfound Jul 13, 2023

caarlos0 Jul 13, 2023

caarlos0 Jul 13, 2023

kpenfound Jul 13, 2023

gerhard commented Jul 13, 2023

marcosnils Jul 13, 2023

kpenfound Jul 13, 2023

caarlos0 Jul 13, 2023

marcosnils Jul 13, 2023 •

edited

caarlos0 Jul 13, 2023

marcosnils Jul 13, 2023

caarlos0 Jul 14, 2023

mircubed commented Jul 17, 2023

caarlos0 commented Jul 18, 2023

caarlos0 commented Jul 18, 2023

caarlos0 commented Jan 14, 2024

marcosnils commented Jan 14, 2024 via email

kpenfound commented Jan 18, 2024

		// DO NOT UPDATE THIS!! needs to be v0.0.0-20210512092938-c05353c2d58c
		require github.com/ProtonMail/go-crypto v0.0.0-20230626094100-7e9e0395ebec // indirect

Use dagger for build and test pipeline #4186

Use dagger for build and test pipeline #4186

Conversation

kpenfound commented Jul 13, 2023

caarlos0 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gerhard commented Jul 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcosnils Jul 13, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mircubed commented Jul 17, 2023

caarlos0 commented Jul 18, 2023

caarlos0 commented Jul 18, 2023

caarlos0 commented Jan 14, 2024

marcosnils commented Jan 14, 2024 via email

kpenfound commented Jan 18, 2024

marcosnils Jul 13, 2023 •

edited