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

feat(release): create Docker hub binaries when tagging #5138

Merged
merged 13 commits into from
Sep 16, 2022

Conversation

gustavovalverde
Copy link
Member

@gustavovalverde gustavovalverde commented Sep 12, 2022

Motivation

We'd like to have binaries published in Docker Hub for easier pulling, as people is more used to use the pull <org>/<image>:<tag> approach when pulling Docker images, instead of long URLs which could also change.

Designs

Publish binaries to Docker Hub when we're making a new release.

Solution

  • Create a new workflow for binary releases
  • Just build this binaries when a release is created (not a pre-release)
  • This binaries are meant to be push to Docker Hub

Fixes #1963

Review

We might want to be more strict with which tags to release to Docker Hub.

Reviewer Checklist

  • Code implements Specs and Designs
  • Tests for Expected Behavior

Follow Up Work

Update #4917 with this information (?)

@gustavovalverde gustavovalverde added A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement P-Medium ⚡ labels Sep 12, 2022
@gustavovalverde gustavovalverde self-assigned this Sep 12, 2022
@gustavovalverde gustavovalverde requested a review from a team as a code owner September 12, 2022 21:19
@gustavovalverde gustavovalverde requested review from teor2345 and removed request for a team September 12, 2022 21:19
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks really good, thanks for pulling it together.

But I'm wondering if triggering releases using a tag filter is confusing or error-prone.

We've never used tags for development - we just seem to use branches.
But if we ever do, it would be a bit hard for developers to remember to start tags with letters other than v. (And if they forget, their tag gets published as an official release, and everyone using zebrad:latest automatically gets it.)

Can we use the release events instead?
https://docs.github.com/en/developers/webhooks-and-events/webhooks/webhook-events-and-payloads#webhook-payload-object-41

@gustavovalverde
Copy link
Member Author

Seems like this needs further testing as this condition is not working as expected . I'm pulling it back to draft in the meanwhile as I have to find a better way to make this work as expected https://github.com/ZcashFoundation/zebra/pull/5138/files#diff-5b40193e09025fd041de92d38c976d0b469e3ee85bef45a71d05d8b5078b1ee8R111

@gustavovalverde gustavovalverde marked this pull request as draft September 13, 2022 21:03
teor2345
teor2345 previously approved these changes Sep 14, 2022
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for these changes!

I just went through the comments and made them match what I think is happening. Feel free to edit them or reject them.

I'm happy for anyone to re-approve this PR.

Co-authored-by: teor <teor@riseup.net>
@gustavovalverde gustavovalverde marked this pull request as ready for review September 14, 2022 15:27
@gustavovalverde
Copy link
Member Author

There were a bunch of edge cases, but this is finally ready!

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just want to check the tags before we merge.

@gustavovalverde
Copy link
Member Author

Delete https://github.com/ZcashFoundation/zebra/releases/tag/v1.0.0 when this is approved

Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to format all the version numbers in our tags the same way.

Co-authored-by: teor <teor@riseup.net>
Copy link
Contributor

@teor2345 teor2345 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes, looks good!

mergify bot added a commit that referenced this pull request Sep 16, 2022
@mergify mergify bot merged commit 7b6da4b into main Sep 16, 2022
@mergify mergify bot deleted the fix-release-binaries branch September 16, 2022 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-devops Area: Pipelines, CI/CD and Dockerfiles C-enhancement Category: This is an improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Work out how to release Zebra binaries
2 participants