ci: build all engine variants before publishing#8269
Merged
Conversation
Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
We move all of the publishing of the engine image into dagger - this allows us to very easily implement functionality that first builds all the variants, and only once this has succeeded, then can publish all the variants. This helps prevent the case where some variants may not build, and we end up in an invalid engine publishing state. To do this reorganization, we need to re-organize some of the dev module's engine api. Specifically, we need to lift the base image and gpu-support parameters into *just* the `Container` method, instead of parameterizing it across the entire engine object. This is so that we can then easily use a go table of targets, and use this to parameterize the calls to `Engine.Container`. Signed-off-by: Justin Chadwell <me@jedevc.com>
gerhard
approved these changes
Aug 29, 2024
Contributor
gerhard
left a comment
gerhard
left a comment
There was a problem hiding this comment.
This is a great refactoring!
LGTM 👍 🚀
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed during the v0.12.6 release process:
This consolidates a ton of the publishing logic together into a loop over a table (familiar to anyone familiar with go table tests). Because all of the logic in go, it's now super easy to force evaluation of all the containers before we start pushing them.
Finally, I removed the
TestPublishfunction, and replaced it with adryRunparameter inPublish(to avoid duplicating a ton of logic) which matches with how we already do this for the SDKs.Here's an example dry-run: https://dagger.cloud/jedevc/traces/7c283e20d59719ff45c1038d7e74174d