ci: daggerize test and lint pipelines

[marcosnils]

👋 coming from #1436

TL;DR:

1. Install the dagger CLI (https://docs.dagger.io/quickstart/cli)
2. Check out this PR and run:

dagger --src . call make-all --args "check_license test"

you can alternatively tell dagger to run the tests against this PR's branch without having to check out your code in your machine as follows:

dagger call -m github.com/prometheus/client_golang@pull/1534/merge --src https://github.com/prometheus/client_golang#pull/1534/merge \
 make-all --args "check_license test"

This runs the same tests the current GHA pipeline runs here https://github.com/prometheus/client_golang/actions/runs/9419021570/workflow#L60 for all the supported_go_versions.txt file.

---

Hopefully this serves as a starting point to make this project's CI pipelines more reproducible and consistent across different environments 🙏 .

Some considerations I took while daggerizing this pipeline:

• I've decided to "wrap" as much as I could in order to make the change the least disruptive. As you can see from the PR, I kept the same make UX the current repository has
• I mostly focused on the go.yml (https://github.com/prometheus/client_golang/actions/runs/9419021570/workflow) and golangci-lint.yml (https://github.com/prometheus/client_golang/actions/runs/9419021582/workflow) workflows as those are mostly the ones that get executed for each PR and commit to main.
• I've noticed that the GHA workflow files seem to updated from the prometheus/prometheus repository. Given that I needed to modify them, I just did it in this repo as I'm unsure which would be the proper way to proceed here.

cc @kakkoyun

[shykes]

you can alternatively tell dagger to run the tests against this PR's branch without having to check out your code in your machine as follows:

dagger -m github.com/prometheus/client_golang@refs/pull/1534/merge call --src git@github.com:prometheus/client_golang#refs/pull/1534/merge make-all --args "check_license test"

For some reason that git URL failed for me, but this worked:

dagger call -m github.com/prometheus/client_golang@pull/1534/merge --src https://github.com/prometheus/client_golang#pull/1534/merge \
 make-all --args "check_license test"

I get some test failures, is that expected @marcosnils ?

[...]
FAIL    github.com/prometheus/client_golang/prometheus  16.423s
ok      github.com/prometheus/client_golang/prometheus/collectors       0.022s
ok      github.com/prometheus/client_golang/prometheus/graphite 0.026s
ok      github.com/prometheus/client_golang/prometheus/internal 0.026s
ok      github.com/prometheus/client_golang/prometheus/promauto 0.026s
ok      github.com/prometheus/client_golang/prometheus/promhttp 0.386s
ok      github.com/prometheus/client_golang/prometheus/push     0.023s
ok      github.com/prometheus/client_golang/prometheus/testutil 0.026s
ok      github.com/prometheus/client_golang/prometheus/testutil/diff    0.023s
ok      github.com/prometheus/client_golang/prometheus/testutil/promlint        0.023s
FAIL
Stderr:
make: *** [Makefile.common:153: common-test] Error

[marcosnils]

For some reason that git URL failed for me, but this worked:

ok, strange.. both seem to work for me. I'll update the original URL to use yours in the original description 🙏

[github-actions]

PR body does not contain a valid type of change. Please refer to CONTRIBUTING.md for more information.

[marcosnils]

I get some test failures, is that expected @marcosnils ?

just 👀 at this failures and I have the theory that they're related to some flukes due to timing nits in tests: https://github.com/marcosnils/client_golang/blob/e56689d0f2d0bbff8be277c2e4a8dd71c89ce47a/prometheus/summary_test.go?plain=1#L389. Since make-all runs all go tests in parallel, looks like it's causing that tests to match incorrect assertions.

cc @kakkoyun WDYT?

[kakkoyun]

@marcosnils Awesome! Thanks for the kickstart. I'll check this PR in-depth in the upcoming days. And see what we can improve.

[marcosnils]

hey @kakkoyun gentle ping here. LMK if you want me to rebase this PR if you have the time to 👀 🙏

[kakkoyun]

This is looking good. 🗡️ 🗡️ 🗡️

Sorry for the extremely late review, and you had to ping us a couple of times.

This is super close to done. If you can address the comments we have put in here.

We review this together with @ArthurSens (hence the "we"s 😜)

[kakkoyun]

I think make all is a bit excessive to run in this repo. We can live with

• make test
• make fmt
  for starters.

This could also solve the issue with the license checker that you had to edit above.

[kakkoyun]

And maybe make lint

[marcosnils]

make all is primarily because it loops through all the supported go versions here: https://github.com/prometheus/client_golang/pull/1534/files#diff-f18b612d33c74a552af8f75da09a36af695ef16d51ef7537bd1e7ef2d3d09685R58-R66 and runs whatever make target against them.

This it not running all make targets, it's only running the ones in the github action workflow which currently are lint, and check_license test

[bwplotka]

Let's call it MakeLintTest then?

[marcosnils]

Let's call it MakeLintTest then?

Sorry, I haven't expressed myself correctly. MakeAll runs whatever make target you need to run (https://github.com/prometheus/client_golang/pull/1534/files#diff-f18b612d33c74a552af8f75da09a36af695ef16d51ef7537bd1e7ef2d3d09685R67-R77) with all the supported_go_versions https://github.com/prometheus/client_golang/pull/1534/files#diff-f18b612d33c74a552af8f75da09a36af695ef16d51ef7537bd1e7ef2d3d09685R67-R77.

I realize now that the name is a bit confusion. How about MakeWithSupportedVersions?

[kakkoyun]

Ok, yes, the name is confusing, then. MakeRun or MakeRunWithSupportedVersions would make more sense to me.

[kakkoyun]

@marcosnils, @ArthurSens, and I finally had some time to spare on this. Thanks again for the contribution.

[marcosnils]

@kakkoyun @ArthurSens I've addressed all the comments above. Please let me know if there's anything else that you'd prefer to be changed 🙏

I've also bumped the dagger version to the latest v0.12 and also rebased against main to resolve conflicts.

[bwplotka]

Thanks! I think it's bit adventurous, but given we have little CI scaffolding let's experiment and see how it goes 💪🏽 However, I would try to actively remove old paths to not have two ways of doing things (that means slowly migrating to dagger/main from Make right?

[marcosnils]

things (that means slowly migrating to dagger/main from Make right?

This would be the ideal. At the same time, since Dagger is still calling the same underlying make targets, IMO it's a "good enough" trade-off as an initial approach to see how Dagger feel ☺️

[kakkoyun]

LGTM.

If we can fix the naming and add a comment to explain the intention, we are golden.

We can merge it after a rebase to make sure we capture all the sync files from upstream.

Thanks again @marcosnils

[marcosnils]

If we can fix the naming and add a comment to explain the intention, we are golden.

done! the comment was already there 👐 (https://github.com/prometheus/client_golang/pull/1534/files#diff-f18b612d33c74a552af8f75da09a36af695ef16d51ef7537bd1e7ef2d3d09685R53)