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

[STAL-3059] ci: rework release workflow #534

Merged
merged 12 commits into from
Oct 29, 2024
Merged

Conversation

amaanq
Copy link
Collaborator

@amaanq amaanq commented Oct 24, 2024

What problem are you trying to solve?

Currently, our release process requires too much manual intervention and is error prone. It also is difficult for us to publish GHCR containers with ARM builds.

What is your solution?

This PR reworks the release workflow quite significantly, and as a result there were some side effect changes that had to be made, which are done so in separate commits before the main refactor.

  • First, I bumped outdated actions using very old nodejs versions that have been EOL for a while, as seen by the screenshot below. image

  • Additionally, workflows currently are set to run when any push event occurs. This is problematic because this also includes tag pushes, which we do not want to run these actions on for reasons outlined later. As such, all workflows that run on any push now only run on pushes to branches

  • Lastly, I saw an opportunity that would greatly benefit us - I've migrated our actions that use dtolnay/rust-toolchain to actions-rust-lang/setup-rust-toolchain for a myriad of reasons. It automatically detects if we have a rust-toolchain.toml file and uses the toolchain version there, which is great for having deterministic CI workflows easily replicable locally. It also caches build artifacts and makes use of these caches as best as it can for decreasing build times, and it uses Problem Matchers to annotate our code in a workflow for any build messages that might come from cargo or clippy.

The release workflow itself also has plenty of changes made to it, and are outlined below

  • Instead of running when a release is created, we run this when a tag is pushed to git. The tag must match the pattern '[0-9]+.[0-9]+.[0-9]+', which is the convention we use for our git tags currently. If the tag does not match this pattern, we assume we want to cut a pre-release, and so the only changes from the normal workflow are that we upload a pre-release to github, and the GHCR container image is not tagged as latest.
  • The workflows we want to ensure run beforehand are now marked with on: workflow_call, meaning the release workflow can call these workflows. The ones of interest are test-rules, integration-tests, verify-schema, and -versions-check. integration-tests makes use of secret variables like DD_API_KEY, so we specify this workflow to inherit the release workflow's secrets.
  • We no longer need to install gh cli in the ubuntu 20 images, since we only upload the build artifacts (explained later)
  • We also do not need taiki-e/upload-rust-binary-action; it has too many features that we don't leverage, and all it does for us is build the binaries, zip them up, and upload them to GitHub releases. However, not only is this unnecessary for us, but it's also problematic since the workflow now controls when the release is made, and not us doing it manually. That means that there is no release to upload these binaries to with this action when it's being run, so we should remove it. Instead, we just build the binaries ourselves and zip them up later, which is exactly what the action does.
  • After the binaries are zipped, we upload these artifacts in the workflow, which can be downloaded later on.
  • Once all the build jobs for each platform+arch combination are finished, a release job is run. This job depends on the build jobs as well as all the test jobs mentioned earlier, so it will only run if all of these are successful. We download the uploaded artifacts from the build jobs, list them out, and run gh release create to create a release with these artifacts of interest. We auto-generate the release notes like we currently do in this CLI invocation, and in this case gh is readily available to use since it's using the ubuntu-latest runner.
  • Lastly, after the release is created and we have uploaded the artifacts, the workflow for building the containers to be uploaded to GHCR is run, and since we are always running this in a real release, we no longer need the changes in [STAL-3002] Only use latest tag on ghcr for stable releases #524, so this PR does partially revert that PR. As mentioned earlier, we check if the invocation is a release or a pre-release depending on the pattern of the tag that was pushed, and if it is a pre-release we do not add the latest label to the image.

Alternatives considered

What the reviewer should know

A successful run of this release workflow can be observed here. The actual release for that can be found here. Also, I modified a test that hardcoded the analyzer version, which failed when I updated the version in my fork to run the release workflow, and it now uses CARGO_VERSION to always use the current version of the analyzer.

A successful run of this workflow with a pre-release can be observed here. Note that the tag used is pre-release, and because it doesn't fit that pattern mentioned above, a pre-release was made instead here

An unsuccessful run of this workflow can be observed here, in which a tag was pushed on the non-main branch.

@amaanq amaanq requested a review from a team as a code owner October 24, 2024 21:48
@amaanq amaanq requested a review from dastrong October 24, 2024 21:48
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/test-rules.yaml Show resolved Hide resolved
.github/workflows/check-regressions.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/sca.yml Outdated Show resolved Hide resolved
.github/workflows/rust.yaml Outdated Show resolved Hide resolved
.github/workflows/integration-tests.yaml Outdated Show resolved Hide resolved
.github/workflows/integration-tests.yaml Show resolved Hide resolved
.github/workflows/verify-schema.yaml Show resolved Hide resolved
@amaanq amaanq force-pushed the amaan.qureshi/STAL-3059 branch from 7b5dba7 to 63f83bb Compare October 24, 2024 21:50
@jasonforal jasonforal requested review from jasonforal and removed request for dastrong October 25, 2024 13:37
Copy link
Collaborator

@jasonforal jasonforal left a comment

Choose a reason for hiding this comment

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

This is looking nice!

I think by reverting #524, we're missing a requirement though, in that we can't publish pre-releases.

If the tag matches ^\d+.\d+.\d+$, the GHCR image should receive the “latest” tag, and it should be marked “Latest” on GitHub releases. Otherwise, the GHCR image should not receive the “latest” tag, and the GitHub release should be marked “pre-release”.

It seems to me like this could be worked into your current approach without too much change

.github/workflows/check-regressions.yml Outdated Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/release.yml Show resolved Hide resolved
.github/workflows/integration-tests.yaml Outdated Show resolved Hide resolved
@amaanq amaanq force-pushed the amaan.qureshi/STAL-3059 branch 2 times, most recently from e3c9131 to 5f275b0 Compare October 25, 2024 22:07
@amaanq amaanq force-pushed the amaan.qureshi/STAL-3059 branch from 5f275b0 to 95fe979 Compare October 28, 2024 14:45
Copy link
Collaborator

@jasonforal jasonforal 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 nearly ready to ship. However, two more requests here:

  1. Update the PR description now that the missing requirement was added.
  2. Show an example of what happens when trying to deploy a "latest" release off a non-main branch (i.e. make sure we can't fat-finger a latest release)

.github/workflows/release.yml Outdated Show resolved Hide resolved
.github/workflows/ghcr.yml Outdated Show resolved Hide resolved
@amaanq amaanq merged commit e7fd707 into main Oct 29, 2024
70 checks passed
@jasonforal jasonforal deleted the amaan.qureshi/STAL-3059 branch November 8, 2024 09:38
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.

2 participants