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

ci: update release process with release-please #549

Merged

Conversation

antonbaliasnikov
Copy link
Contributor

@antonbaliasnikov antonbaliasnikov commented Jan 17, 2025

What 💻

  • Add release-please configuration and workflows
  • Update Docker image with cargo chef build stage that allows the proper layers caching improving docker image building time from ~30 min to ~15 min.
  • Add GH attestations to Docker images as well as for binaries
  • Add the capability to manually trigger binary release workflow to generate a prerelease for testing purposes
  • Create only one attestation for all binaries instead of individual attestation for each binary per platform.

@antonbaliasnikov antonbaliasnikov force-pushed the 474-releases-improve-automated-releases-and-review branch 6 times, most recently from da93b4a to 7db8cb0 Compare January 17, 2025 16:57
@antonbaliasnikov antonbaliasnikov marked this pull request as ready for review January 17, 2025 16:58
@antonbaliasnikov antonbaliasnikov requested a review from a team as a code owner January 17, 2025 16:58
@antonbaliasnikov antonbaliasnikov force-pushed the 474-releases-improve-automated-releases-and-review branch from 7db8cb0 to 452459d Compare January 17, 2025 17:19
@dutterbutter
Copy link
Collaborator

dutterbutter commented Jan 17, 2025

@antonbaliasnikov were you able to test successfully in a fork by any chance? I suppose this will also Closes #474

@antonbaliasnikov
Copy link
Contributor Author

antonbaliasnikov commented Jan 17, 2025

@antonbaliasnikov were you able to test successfully in a fork by any chance? I suppose this will also Closes #474

@dutterbutter , I've tested it in this PR by enabling pull_request target and making sure that docker and binaries are successfully built.

In the fork, I've tested release-please as well, here is a next release PR example:
antonbaliasnikov#9

But I will test a bit more as I found see some issues with release-please tagging.
Update: it was related to the default token that by some reason was not able to properly add labels in PRs even with the right permission settings. What is more interesting that absolutely identical settings are working in my foundry-zksync fork with no issues.

I suppose this will also Closes #474
Yes, it will. Is is aligned with the issue through the git branch name.

But we need to discuss minor details with you and @itegulov about the flow. Because release-please will always create the release first, and only then we start jobs to add binaries and publish docker images. We can re-try them if they fail, but anyway it would be nice to discuss the flow we would like to see.

@antonbaliasnikov antonbaliasnikov force-pushed the 474-releases-improve-automated-releases-and-review branch 2 times, most recently from 17b8157 to d6b0e58 Compare January 20, 2025 11:01
Copy link
Contributor

@itegulov itegulov left a comment

Choose a reason for hiding this comment

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

Because release-please will always create the release first, and only then we start jobs to add binaries and publish docker images. We can re-try them if they fail, but anyway it would be nice to discuss the flow we would like to see.

Can't we make release-please create preleases where the corresponding docker tag would be prerelease-vx.y.z and then once we ensure everything is buildable/pushable, we promote prerelease that triggers retagging from prelease-vx.y.z to vx.y.z which should be infallible?

That being said I don't want to complicate the flow too much so I think it is fine to leave it as is if there are no reasonable options. Let's just make sure that job failures are visible, I don't want them to be "swallowed" silently. I am guessing at least the person who pressed the merge button on the PR would get an email notification about job failure, correct?

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
.github/release-please/manifest.json Outdated Show resolved Hide resolved
.github/workflows/build-push-docker.yml Show resolved Hide resolved
.github/workflows/build-push-docker.yml Show resolved Hide resolved
.github/workflows/release-please.yml Show resolved Hide resolved
@antonbaliasnikov antonbaliasnikov force-pushed the 474-releases-improve-automated-releases-and-review branch from d6b0e58 to 84541b3 Compare January 21, 2025 11:26
@antonbaliasnikov
Copy link
Contributor Author

antonbaliasnikov commented Jan 21, 2025

Because release-please will always create the release first, and only then we start jobs to add binaries and publish docker images. We can re-try them if they fail, but anyway it would be nice to discuss the flow we would like to see.

Can't we make release-please create preleases where the corresponding docker tag would be prerelease-vx.y.z and then once we ensure everything is buildable/pushable, we promote prerelease that triggers retagging from prelease-vx.y.z to vx.y.z which should be infallible?

That being said I don't want to complicate the flow too much so I think it is fine to leave it as is if there are no reasonable options. Let's just make sure that job failures are visible, I don't want them to be "swallowed" silently. I am guessing at least the person who pressed the merge button on the PR would get an email notification about job failure, correct?

Prereleases are barely supported with release-please.
Although I think that we can do better and improve on the process, I suggest we do it iteratively. Let's merge this flow and I'll check prereleases and possibilities here separately if you don't mind.

For now, yes, visibility will be highly better and email notifications + Slack notifications about failures should be sent.

@antonbaliasnikov
Copy link
Contributor Author

@itegulov , please, take another look. I fixed all issues.

Copy link
Contributor

@itegulov itegulov left a comment

Choose a reason for hiding this comment

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

LGTM! Left a small nit but feel free to merge as is


concurrency:
group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true
cancel-in-progress: false
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: do we even need concurrency groups anymore?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I guess it can be removed in this case.

@antonbaliasnikov antonbaliasnikov merged commit 1061c30 into main Jan 22, 2025
14 checks passed
@antonbaliasnikov antonbaliasnikov deleted the 474-releases-improve-automated-releases-and-review branch January 22, 2025 08:58
@antonbaliasnikov antonbaliasnikov linked an issue Jan 22, 2025 that may be closed by this pull request
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.

releases: improve automated releases and review
3 participants