Skip to content
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

Improve docker github action workflow with Dagger #17547

Closed
wants to merge 11 commits into from

Conversation

marcosnils
Copy link
Contributor

TL;DR improve the docker GHA as follows:

  • Allow to run, build, test and publish locally
  • Simplify workflow definition from 4 to 5 by moving YAML repeated steps into code
  • Enable testing multiple versions concurrently locally
  • Improve consecutive build times from 6m to 20s by properly caching brew tap Dockerfile command

You can build and test brew with the latest ubuntu version by calling:

dagger call test –src .

Or you can also build and test all possible versions concurrently:

dagger call –test-all –src .

First and foremost thanks for all the effort you’re putting into making Brew awesome, many of us at Dagger use it regularly.

While troubleshooting some Homebrew <> Dagger issues related to the official Docker image 7, I observed that each docker build of such image was taking a considerable amount of time (around 6 minutes) due to the fact that the homebrew-core tap is cloned each time as well as the inability to easily run the same complete build and test suite that your CI currently performs 5.

In an attempt to overcome these pain points, I’ve taken a shot and decided to “Daggerize” part of this process so it’s easier for new users to run a complete build and test suite which runs in a reasonable amount of time. This, at the same time, has additional benefits like the ability to run pipelines locally as well as easier to maintain and more modular form as they’re expressed in code.

As you’ll see from this PR, I have decided to start with a relatively simple pipeline 3 to get a better understanding of the project, This is generally how I recommend our users to start adopting Dagger in their projects, by picking a well scoped and constrained pipeline that lays the foundations for future daggerizations. I have to say that I’m quite happy with the results and if you think this is beneficial to the project, I'm happy to keep helping daggerizing other core pipelines which you might need help with.

Happy to answer any questions that you may have.

PS:

As a fun experiment, I’ve also decided to perform a livestream series 1 for educational purposes as a real case scenario on how someone could gradually adopt Dagger in a large consolidated project. Hopefully you’ll find this as entertaining and educational as we’ve found it ourselves while recording it.

`call` is the default so it's redundant

Co-authored-by: Solomon Hykes <[email protected]>
README.md Outdated Show resolved Hide resolved
dagger/main.go Outdated Show resolved Hide resolved
dagger/main.go Outdated Show resolved Hide resolved
Co-authored-by: Solomon Hykes <[email protected]>
@shykes
Copy link

shykes commented Jun 21, 2024

Obviously I love this 😁

Small nit, I don't think you need to include go.sum and 99% of go.mod, assuming your own code has no go dependencies: it's all generated dagger stuff that is already pinned by the dagger SDK and engine version. Removing them would make the PR look less intimidating.

default="" is better expressed as optional

Co-authored-by: Solomon Hykes <[email protected]>
@woodruffw
Copy link
Member

Hey @marcosnils and @shykes -- this PR caught us by surprise, since it (1) proposes pretty significant changes to the Homebrew CI, and (2) doesn't tell us what Dagger actually is 🙂

To take a step back: could you offer some more context here? My first intuition is that these changes are major and involve technologies that the Homebrew maintainers don't touch frequently (like Go), so we'd need a really strong user story to justify adoption here.

@MikeMcQuaid
Copy link
Member

Going stronger than @woodruffw's (great) message above: passing on this. We're not interested in having Go code in this repository and, if you have Dagger issues at Dagger, I'd suggest that's on you folks to fix in your product rather than make Homebrew use Dagger to improve the integration here.

We'll review more targeted PRs or issues to make more tightly scoped Dockerfile/CI changes to improve our Docker images but, as-is, nope, sorry.

@shykes
Copy link

shykes commented Jun 24, 2024

if you have Dagger issues at Dagger, I'd suggest that's on you folks to fix in your product rather than make Homebrew use Dagger to improve the integration here

Hi Mike, I'll let @marcosnils address the rest, but to clarify, this was a genuine contribution, not an attempt to push downstream issues on you.

@MikeMcQuaid
Copy link
Member

Hi Mike, I'll let @marcosnils address the rest, but to clarify, this was a genuine contribution, not an attempt to push downstream issues on you.

@shykes I was responding to:

While troubleshooting some Homebrew <> Dagger issues related to the official Docker image

Typically I would not expect the resolution of such issues to occur in the smaller/newer project (e.g. Dagger) rather than the larger/older project (e.g. Homebrew).

I don't think you are trying to push downstream issues on us but, from the PR, it's unclear what, if any, benefits this provides to Homebrew users that are not also Dagger users/employees.

@shykes
Copy link

shykes commented Jun 24, 2024

While troubleshooting some Homebrew <> Dagger issues related to the official Docker image

Typically I would not expect the resolution of such issues to occur in the smaller/newer project (e.g. Dagger) rather than the larger/older project (e.g. Homebrew).

I understand what you're saying, and I agree. But that sentence you're replying to is misleading: this PR is not addressing any Dagger-related issue, or any issues with Dagger/Homebrew integration. It's 100% a contribution to Homebrew, with the sole intent of improving Homebrew's CI. Specifically it makes it faster and easier to run locally. Clearly we failed to communicate that.

@marcosnils
Copy link
Contributor Author

marcosnils commented Jun 24, 2024

@woodruffw @MikeMcQuaid I appreciate the feedback and apologies beforehand if this generated some unnecessary overhead to your role as maintainers. As an OSS maintainer and contributor myself, I went through Homebrew's contribution guidelines 1 before opening the PR to validate if some prior discussion was needed, and just moved forward and opened this as I saw it didn't seem to be necessary.

Regardless this PR getting merged or not, I'd like to clarify some misunderstandings and leave some constructive user feedback that might help some future contributors.

To take a step back: could you offer some more context here?

@woodruffw definitely! To be concrete, the initial frustration that lead us to this PR is (1) the excessive amount of time (~6m each time) it takes to build the brew docker image, even in the same machine repeatedly with a warm cache, and (2) the lack of being able to run the same build and test process Homebrew currently runs in CI (https://github.com/Homebrew/brew/blob/master/.github/workflows/docker.yml#L36-L53) to check that we're not breaking anything.

This PR addresses both of the pain points described above. The long build times issue is addressed primarily by the changes introduced in the Dockerfile. For the second issue, instead of creating a Makefile or using some other equivalent tool which all have their shortcomings, I've opted for giving Dagger a try given that it's specifically designed to solve the problem.

Dagger is a programmable engine that runs pipelines in containers which solves the problem of transforming messy CI scripts and YAML into actual code. I wish I could have used Ruby in this PR but unfortunately, that's still not supported in Dagger as a core SDK.

if you have Dagger issues at Dagger, I'd suggest that's on you folks to fix in your product rather than make Homebrew use Dagger to improve the integration here.

As @shykes mentioned above, seems like this was a communication issue on my end. Apologies for the confusion this generated 🙏 .


As a closing statement, if you still see value in any of the aspects this PR is addressing for Homebrew users, let us know so we can make the necessary changes to get the PR in a good shape. If you're also interested in the benefits that Dagger could bring to the Homebrew project but using Go is currently a limiting factor to adopt it, I could rewrite the Dagger part in either Python or Typescript if that's something that your team would be more comfortable working with.

Once again, this was a genuine contribution to help future Homebrew users and the project to level-up its CI/CD pipelines.

@MikeMcQuaid
Copy link
Member

The long build times issue is addressed primarily by the changes introduced in the Dockerfile.
...
As a closing statement, if you still see value in any of the aspects this PR is addressing for Homebrew users, let us know so we can make the necessary changes to get the PR in a good shape.

We'd review a PR to make changes exclusively to the Dockerfile to speed up generation.

seems like this was a communication issue on my end. Apologies for the confusion this

Appreciate the apology, thanks for that ❤️

If you're also interested in the benefits that Dagger could bring to the Homebrew project but using Go is currently a limiting factor to adopt it, I could rewrite the Dagger part in either Python or Typescript if that's something that your team would be more comfortable working with.

We're not interested just now, either in the tool in general or having Go/Python/Typescript in this repo, sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants