search

brigadecore / brigade

Event-driven scripting for Kubernetes
https://brigade.sh/
Apache License 2.0
2.41k stars 246 forks source link

Build Brigade container images for PRs #761

Closed radu-matei closed 3 years ago

radu-matei commented 5 years ago

When testing PRs, it's really helpful to have built container images for Brigade components - meaning as a reviewer I don't have to build and push the image myself, plus we can easily rollback to any previous PR.

We could keep the intermediate images at least during a release cycle, and when we cut a new one, simply delete all PR images from the previous release cycle?

What are the downsides for this? cc @vdice

dgkanatsios commented 5 years ago

Great idea, I think that there would also be a need for the respective Helm templates to be created as well for easier testing, right?

radu-matei commented 5 years ago

Most of the time when I'm reviewing a PR it's one component that is changed - and I simply edit the deployment for that component directly (and I blame VS Code for making it this easy...), or I can directly helm install --set <>.image

vdice commented 5 years ago

This definitely sounds reasonable and should be straightforward to add publishing images to a registry in our brigade.js. The easiest path would to just publish to the default registry currently referenced in the charts (deis org on Dockerhub), though this would have the effect of filling up the tags view for a given image on Dockerhub with non-release tags (perhaps not a big deal; if it were, we could consider using the old deisci org of old, where we used to throw PR/ephemeral images back in the Workflow days, though this would introduce complexity of adjusting the chart for testing to fetch from this non-default org). We can think about what a cleanup mechanism might look like.

Ideally, this would spark a refresh/revive of this project's functional/e2e test approach, which we could also add in to the PR checks pipeline, to run after component image publishing.

radu-matei commented 5 years ago

Ideally, this would spark a refresh/revive of this project's functional/e2e test approach, which we could also add in to the PR checks pipeline, to run after component image publishing.

This would be fantastic!

krancour commented 5 years ago

This should all be substantially easier now that a bunch of pipeline improvements went in a couple months ago. In the current state, we publish images tagged with git sha and the mutable tag "edge" on every merge to master. All we need to do is change things around so that publishing of sha-tagged images occurs toward the end of the PR pipeline (after unit tests, before e2e tests). If we consider the moving target "edge" to be representative of what's at the head of master, that should still only be published upon merge to master.

Long story short... this should be pretty easy to accomplish and I'd be happy to tackle this.

One thing I would suggest, however, is using a separate repository (image name) for images built from PRs to very unambiguously convey that the images were built from code that may be in a completely unvetted state.

krancour commented 3 years ago

Closing due to staleness. Please do re-open if appropriate.