Release management in CI #18398

oguzkocer · 2023-05-10T19:50:40Z

This PR adds the necessary configurations to handle the project's release management in CI. It closely follows the approach we've explored in release-toolkit-demo project and discussed internally in our RFCs.

There are a few moving parts to be aware of:

There are new Buildkite triggers to start release management jobs. These are contained in fastlane/lanes/release_management_in_ci.rb. We intend to remove these triggers and use curl instead.
There are new pipelines for each release management step, such as .buildkite/code-freeze.yml, .buildkite/complete-code-freeze.yml etc. These are pretty straightforward as they just call the relevant lane after some basic configuration.
There are a few new Buildkite helper commands. .buildkite/commands/configure-git-for-release-management.sh configures the git client, which we intend to move to our agents. .buildkite/commands/checkout-release-branch.sh checks out the release branch which is necessary because Buildkite starts the job in a detached state and we need to be on a branch to be able to commit and push the changes. Similarly .buildkite/commands/checkout-editorial-branch.sh checks out the editorial branch which is a parameter in the job's trigger.
We are temporarily using the remove/git-push-actions release-toolkit branch which is already approved. I intend to create the new release tomorrow. As a result of this, you'll notice a few additions of push_to_git_remote.
The rest of the changes are in the release & localization lanes. They are mostly related to creating an intermediate branch, pushing the changes to remote and creating a pull request.
Note that downloading translations will likely not work well in CI in this current iteration. That's because we get 429: Too Many Requests errors as we try to download several translations and we need to manually confirm to retry. I'm working on a possible fix for this, but it won't be part of this PR.

To test:

Unfortunately there is no way to test the full release flow directly in WPAndroid. However, we used release-toolkit-demo project to test this implementation before bringing it to WPAndroid. So, hopefully any issues we might experience will be minimal.

I was able the test updating release notes flow and it verified that the general setup is working correctly. It's a bit cumbersome to test this and since it's in CI, there isn't anything new to expect, so I suggest skipping this testing step.

We expect some minor issues with the initial implementation, but I am in a position to quickly address them as this project's release manager.

Regression Notes

Potential unintended areas of impact
Parts of release management might break as we move it to CI.
What I did to test those areas of impact (or what existing automated tests I relied on)
N/A
What automated tests I added (or what prevented me from doing so)
N/A

PR submission checklist:

I have completed the Regression Notes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

UI Changes testing checklist:

N/A

wpmobilebot · 2023-05-10T20:09:49Z

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr18398-0af7b97
	Commit	`0af7b97`
	Direct Download	`jetpack-prototype-build-pr18398-0af7b97.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2023-05-10T20:10:01Z

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr18398-0af7b97
	Commit	`0af7b97`
	Direct Download	`wordpress-prototype-build-pr18398-0af7b97.apk`

Note: Google Login is not supported on these builds.

oguzkocer · 2023-05-10T20:37:39Z

.buildkite/commands/configure-git-for-release-management.sh

We intend to bring this configuration to our agents, so that we don't have to do it in every pipeline.

oguzkocer · 2023-05-11T02:49:43Z

Gemfile

 # gem 'fastlane-plugin-wpmreleasetoolkit', path: '../../release-toolkit'
-# gem 'fastlane-plugin-wpmreleasetoolkit', git: 'https://github.com/wordpress-mobile/release-toolkit', branch: 'trunk'
+gem 'fastlane-plugin-wpmreleasetoolkit', git: 'https://github.com/wordpress-mobile/release-toolkit', branch: 'remove/git-push-actions'


I'll do a proper release before we merge this PR in.

oguzkocer · 2023-05-11T02:53:15Z

fastlane/helpers/github.rb

I am not sure where the best place for this helper is. I've asked @jkmassel's opinion and he agreed that this approach made sense. However, if you have a suggestion, I'd be happy to apply it.

Note that I initially had this in the lanes/release_management_in_ci.rb file but I couldn't get it to work. I am now realizing that if I import that before the other lanes, that approach also would have worked. However, I like the separate file a bit better.

This seems like a good start – long-term, might be good to see if we can turn it into a release toolkit action – I find that helpers generally end up there!

I agree. To provide some context;

I initially intended to do this and created an internal discussion where Olivier pointed out that there is already a fastlane action for creating pull requests. That's when I moved on to adding it to the project instead. I went back and forth on where it belongs but decided to go with this initial implementation because this helper doesn't have much added value. However, at some point, I think it'll make sense to have separate helpers for code freeze, new beta, finalize release etc and then it'll make sense to move it to release-toolkit so we can use it in other projects as well.

+1. It makes sense to start local and extract/abstract later.

oguzkocer · 2023-05-11T02:55:27Z

fastlane/helpers/github.rb

@@ -0,0 +1,10 @@
+def create_release_management_pull_request(base_branch, title)
+  create_pull_request(
+      api_token: ENV['GITHUB_TOKEN'],


Fastlane uses ENV["GITHUB_API_TOKEN"] as the default whereas we are using ENV["GITHUB_TOKEN"]. Instead of adding another environment variable, I thought it'd be better to default to the existing one.

This is a good way to go. The reason we use that environment variable is that it's what OctoKit expects FWIW.

oguzkocer · 2023-05-11T02:56:49Z

fastlane/helpers/github.rb

+      title: title,
+      head: Fastlane::Helper::GitHelper.current_git_branch,
+      base: base_branch,
+      labels: 'Releases'


We can add reviewers field, but I wanted to skip that for now to not create extra noise. I have tested creating PRs a few times and will likely do a few more. Once everything settles down, it's easy enough to add this field.

oguzkocer · 2023-05-11T02:59:24Z

fastlane/helpers/github.rb

+      api_token: ENV['GITHUB_TOKEN'],
+      repo: GHHELPER_REPO,
+      title: title,
+      head: Fastlane::Helper::GitHelper.current_git_branch,


Fastlane defaults to the current branch. However, it does so by using the git_branch helper which uses environment variables to override the value. We do not want this behavior which is why we added our own helper in wordpress-mobile/release-toolkit#463.

oguzkocer · 2023-05-11T03:02:20Z

fastlane/lanes/localization.rb


    apps.each do |app|
      app_values = APP_SPECIFIC_VALUES[app]

      metadata_folder = File.join(PROJECT_ROOT_FOLDER, 'WordPress', app_values[:metadata_dir])
-      version = options.fetch(:version, android_get_app_version)


The version value is not application specific, in fact it's fetched from version.properties file which has a shared value. So, there is no harm in moving this outside of the loop and reuse the value in release_branch variable.

oguzkocer · 2023-05-11T03:04:56Z

fastlane/lanes/release_management_in_ci.rb

Our goal is to remove these triggers from fastlane. We'll likely use curl for this, so that release managers don't need to have anything setup in their local environment. However, since we already have buildkite_trigger_build helper, it made sense to use it during the initial implementation phase.

I like that this is nicely separated out for easy removal later!

mokagio · 2023-05-11T06:38:33Z

Please DO NOT merge this PR, as we need to do a release-toolkit release first.

Created and added the "Do Not Merge" label, and Peril picked it up.

mokagio · 2023-05-11T06:41:08Z

.buildkite/commands/checkout-editorial-branch.sh

+
+# EDITORIAL_BRANCH is passed as an environment variable from fastlane to Buildkite
+#
+if [[ -z "${EDITORIAL_BRANCH}" ]]; then


TIL about this approach to avoid the set -u from the shebang to fail the script at this point because of an unbound access. Neat 👏

mokagio

I started looking at this but I don't have the energy to give it the attention it deserves right now.

Just a not. Looking at the various new pipelines and how similar they are, it occurred to me that we might make the setup easier to maintain – as a possible followup! – by generating the pipeline. See the dynamic pipelines docs.

oguzkocer · 2023-05-11T07:29:26Z

Just a not. Looking at the various new pipelines and how similar they are, it occurred to me that we might make the setup easier to maintain – as a possible followup! – by generating the pipeline. See the dynamic pipelines docs.

It's definitely worth exploring. I think the main benefit we should look to gain from that should be to get rid of the boilerplate part but still keep the command steps. As long as we keep each file separate - or somehow put it all in a small and easy to read file, then it's a net positive for us. However, if it makes it harder to navigate the code, then I don't think we should do it.

jkmassel

Looks solid to me!

jkmassel · 2023-05-11T21:26:10Z

fastlane/helpers/github.rb

This seems like a good start – long-term, might be good to see if we can turn it into a release toolkit action – I find that helpers generally end up there!

jkmassel · 2023-05-11T21:26:44Z

fastlane/helpers/github.rb

@@ -0,0 +1,10 @@
+def create_release_management_pull_request(base_branch, title)
+  create_pull_request(
+      api_token: ENV['GITHUB_TOKEN'],


This is a good way to go. The reason we use that environment variable is that it's what OctoKit expects FWIW.

jkmassel · 2023-05-11T21:27:36Z

fastlane/lanes/release_management_in_ci.rb

I like that this is nicely separated out for easy removal later!

mokagio · 2023-05-12T02:14:54Z

.buildkite/commands/checkout-editorial-branch.sh

+fi
+
+# Buildkite, by default, checks out a specific commit. When we update the app store strings, we open
+# a PR from the current branch. So, we need to checkout the `EDITORIAL_BRANCH`.


Is the difference that we wouldn't be able to open the PR if we were on a commit (detached HEAD?)?

We can't add a new commit in a detached HEAD. When we update the app store strings, we need to create new commits and push it to remote.

.buildkite/commands/configure-git-for-release-management.sh

mokagio · 2023-05-12T02:20:54Z

fastlane/lanes/release.rb


    new_version = android_get_app_version()
    trigger_beta_build(branch_to_build: "release/#{new_version}")
+
+    create_release_management_pull_request('trunk', "Merge #{new_version} code freeze to trunk")


Nitpick. Since we dropped the develop-trunk setup, I stopped adding "to trunk" in my PRs because that's the branch where usually release PRs land.

Suggested change

create_release_management_pull_request('trunk', "Merge #{new_version} code freeze to trunk")

create_release_management_pull_request('trunk', "Merge #{new_version} code freeze")

With release management in CI, not all PRs target the trunk branch. Some of them target the release branch, so I think it's useful to have this information.

mokagio · 2023-05-12T02:21:39Z

fastlane/lanes/release.rb

    setfrozentag(repository: GHHELPER_REPO, milestone: version['name'], freeze: false)
    create_new_milestone(repository: GHHELPER_REPO)
    close_milestone(repository: GHHELPER_REPO, milestone: version['name'])

    # Trigger release build
    trigger_release_build(branch_to_build: "release/#{version['name']}")
+
+    create_release_management_pull_request('trunk', "Merge #{app_version} final to trunk")


Same rationale as above.

Suggested change

create_release_management_pull_request('trunk', "Merge #{app_version} final to trunk")

create_release_management_pull_request('trunk', "Merge #{app_version} final")

Same rationale as above. 😅

fastlane/lanes/release_management_in_ci.rb

+      buildkite_pipeline: BUILDKITE_PIPELINE,
+      branch: "release/#{release_version}",
+      pipeline_file: 'complete-code-freeze.yml',
+      message: 'Complete Code Freeze in CI',


fastlane/lanes/release_management_in_ci.rb

mokagio

Only left nitpicks that have little to do with the actual implementation. Things are looking pretty good 😄

Co-authored-by: Gio Lodi <gio.lodi@automattic.com>

peril-wordpress-mobile · 2023-05-12T03:14:09Z

	Warnings
⚠️	This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

Co-authored-by: Gio Lodi <gio.lodi@automattic.com>

oguzkocer · 2023-05-12T03:18:37Z

Thank you for the review @mokagio! I've applied all your suggestions except for dropping to trunk part of the PR titles. I left a comment about it, but I'll re-iterate it here. With release management in CI, not all PRs will be targeting trunk, so I think it's good to be able to see that information at a glance - especially when the GitHub url is unfurled in Slack or p2.

AliSoftware · 2023-05-12T11:19:07Z

.buildkite/commands/configure-git-for-release-management.sh

+git config --global user.name "Automattic Release Bot"
+
+# Buildkite is currently using the https url to checkout. We need to override it to be able to use the deploy key.
+git remote set-url origin git@github.com:wordpress-mobile/WordPress-Android.git


💡 I wonder if it would be better to make this a bit more generic, like:

REPO_NO_TRAILING=${BUILDKITE_REPO%/} git remote set-url origin ${REPO_NO_TRAILING/#https:\/\/github.com\//git@github:}

Or alternatively (inspired by this):

REPO_SLUG=$(basename $(dirname "$BUILDKITE_REPO"))/$(basename "$BUILDKITE_REPO%.git") git remote set-url origin git@github.com:${REPO_SLUG}.git

If this was in a toolkit repo, it'd make sense to go with a generic version. However, since this is part of WordPress-Android, I think this just makes it harder to understand the code.

Furthermore, it looks like I forgot to mention that this is a temporary issue specific to WordPress-Android. We should not be using the https url to clone in the first place. I just didn't have a chance to figure out why this is the case for WordPress-Android. I think we used to have a specific configuration for WPAndroid, but I couldn't find it yet.

For example, we don't need this bit in release-toolkit-demo. I also checked FluxC & WCAndroid and they both use the ssh url.

Aaah.. and I think I might have found the culprit. All this time I've been checking the Buildkite settings' Checkout using: section which says that we are using ssh to checkout. However, I just checked the terraform and the repository field for WPAndroid is https://github.com/wordpress-mobile/WordPress-Android/. I compared this against WPiOS, WCAndroid & FluxC and they all use the ssh url. So, I think we'll be able to get rid of this configuration all together.

Having said that, I hope my previous logic also made sense in terms of keeping the specific url.

If this was in a toolkit repo, it'd make sense to go with a generic version. However, since this is part of WordPress-Android, I think this just makes it harder to understand the code.

🤦‍♂️ I've reviewed so many PRs in the past few days that I think I lost track of which repo each PR was about, and since I reviewed a PR about buildkite-ci repo just before this one, I think I didn't realize I was now reviewing a WPAndroid repo when I switched to this review 🤦‍♂️🤦 Sorry 😅

No worries at all 🤗

I've reviewed so many PRs in the past few days

I noticed ❤️ - thank you for doing that! 🙇‍♂️

oguzkocer added 7 commits May 10, 2023 15:09

Adds release management Buildkite pipelines

f34ed36

Adds Buildkite triggers to release_management_in_ci lanes

b656d9f

Create pull request after complete code freeze

0e6a276

Adds support for handling new_beta_release in CI

1fad391

Adds support for handling finalize_release in CI

904842e

Use remove/git-push-actions release-toolkit branch

647d6ff

Adds push_to_git_remote to release lanes

ba8c70b

oguzkocer added the Tooling label May 10, 2023

oguzkocer added this to the 22.4 milestone May 10, 2023

oguzkocer commented May 10, 2023

View reviewed changes

Push changes to remote and open a pr after updating app store strings

686f0f6

oguzkocer force-pushed the release-management-in-ci branch from 1e8c36e to 686f0f6 Compare May 10, 2023 21:01

oguzkocer added 5 commits May 10, 2023 17:02

Adds the missing 'end' trigger_update_appstore_strings_in_ci lane

8ebf981

Move create_release_management_pull_request to a new helper file

08b6070

Change where we set the version variable in update_appstore_strings

9d27c99

Adds release_management_in_ci lanes to Fastfile

64da4d2

Set remote url in configure-git-for-release-management.sh

7f19187

oguzkocer commented May 11, 2023

View reviewed changes

oguzkocer marked this pull request as ready for review May 11, 2023 03:39

oguzkocer requested a review from a team May 11, 2023 03:39

mokagio added the Do Not Merge In PRs with this label, our automation will fail a require check, preventing accidental merging label May 11, 2023

mokagio reviewed May 11, 2023

View reviewed changes

oguzkocer requested a review from a team May 11, 2023 19:28

jkmassel approved these changes May 11, 2023

View reviewed changes

Update release toolkit to 8.0.0

7db6ef3

oguzkocer removed the Do Not Merge In PRs with this label, our automation will fail a require check, preventing accidental merging label May 12, 2023