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

Create lockDeploys workflow #1938

Merged
merged 2 commits into from
Mar 19, 2021
Merged

Create lockDeploys workflow #1938

merged 2 commits into from
Mar 19, 2021

Conversation

roryabraham
Copy link
Contributor

@roryabraham roryabraham commented Mar 19, 2021

Details

To avoid a race condition between this job and preDeploy:version, added a new action called https://github.com/TomChv/wait-my-workflow/ . Tested in Public-Test-Repo and seems to work well.

Fixed Issues

Fixes https://github.com/Expensify/Expensify/issues/155210

Tests

  1. Merge this PR, then quickly add the 🔐 LockCashDeploys 🔐 label to the StagingDeployCash. We should then expect that:
  • The lockDeploys workflow should wait for the preDeploy::version job to finish before proceeding past wait-my-workflow. But then once it does...
  • The bumpVersion action should be executed w/ SEMVER_LEVEL = PATCH
  • An automerge PR should be merged to master to bump the PATCH version
  • After the preDeploy workflow runs again, an automerge PR should be merged to staging from master.
  • The StagingDeployCash should be updated with the new PATCH version
  1. Then merge another PR to master, and we should expect:
  • OS Botify comments on the PR to say it will be deployed later
  • The staging deploy should be skipped for this PR. No version-bump PR should be created.
  • The StagingDeployCash should not be modified
  1. Then close the StagingDeployCash, and:
  • A release should be created w/ the PATCH version created in step 1
  • A "production deploy" should happen
  • The BUILD version should be bumped on master
  • A staging deploy should happen with the new BUILD version on master.
  • A new StagingDeployCash should be created, and should include the PR merged in step 2.
  1. Add the 🔐 LockCashDeploys 🔐 to some random issue, and this workflow should be skipped.

@roryabraham roryabraham requested a review from AndrewGable March 19, 2021 19:59
@roryabraham roryabraham requested a review from a team as a code owner March 19, 2021 19:59
@roryabraham roryabraham self-assigned this Mar 19, 2021
@botify botify requested review from MariaHCD and removed request for a team March 19, 2021 20:00
@roryabraham roryabraham merged commit 11ab1fc into master Mar 19, 2021
@roryabraham roryabraham deleted the Rory-lockCashDeploys branch March 19, 2021 20:27
@github-actions github-actions bot locked and limited conversation to collaborators Mar 19, 2021
@roryabraham
Copy link
Contributor Author

Testing step 1:

LockDeploys workflow: https://github.com/Expensify/Expensify.cash/runs/2151878987?check_suite_focus=true
PR against master: https://github.com/Expensify/Expensify.cash/pulls/1943
Staging deploys happened:
image

preDeploy after master PR was merged: https://github.com/Expensify/Expensify.cash/actions/runs/669167652

StagingDeployCash updated #1926

@roryabraham
Copy link
Contributor Author

roryabraham commented Mar 19, 2021

Automerge master failed silently https://github.com/Expensify/Expensify.cash/runs/2151890115?check_suite_focus=true

So our new version wasn't updated on master or staging. This was of course the result of a race condition. preDeploy from this PR being merged had created an automerge PR against master and finished, but the PR had not yet been auto-merged. So the lockDeploys workflow did the patch version bump without first pulling the latest version-bump on master, so when it created the PR there was a merge conflict.

To resolve, we'll make sure there are no automerge jobs going either, and pull master after waiting.

@roryabraham
Copy link
Contributor Author

roryabraham commented Mar 19, 2021

Testing step 1 round 2:

lockDeploys workflow: https://github.com/Expensify/Expensify.cash/actions/runs/669259652
PR against master: #1951 ❌ didn't merge

I think that after the preDeploy workflow completed, the automerge workflow hadn't started by the time the next step in lockDeploys happened.

@roryabraham
Copy link
Contributor Author

roryabraham commented Mar 19, 2021

Testing step 1 round 3:

lockDeploys workflow: https://github.com/Expensify/Expensify.cash/actions/runs/669426206
PR agains master: #1957
preDeploy didn't update staging because StagingDeployCash was locked https://github.com/Expensify/Expensify.cash/actions/runs/669431604
StagingDeployCash was updated #1926

The fact that staging wasn't updated is a problem, but not until we need to build CPs. When we do that, we'll need to make sure that we're not deploying staging until the staging branch is actually updated.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants