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

[PAY][$250] Combine deploy.yml and platformDeploy.yml #48432

Closed
roryabraham opened this issue Sep 2, 2024 · 18 comments
Closed

[PAY][$250] Combine deploy.yml and platformDeploy.yml #48432

roryabraham opened this issue Sep 2, 2024 · 18 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement.

Comments

@roryabraham
Copy link
Contributor

roryabraham commented Sep 2, 2024

Problem

We have started to adopt the practice of shipping checklists but skipping prod deploys when there's only a few blockers left caused by a PR we don't want to revert. This allows us to engage Applause in QA of new code on staging and to uncover and fix any other blockers lurking on main while we work on fixing the remaining known blockers on staging. As a result of this practice, we avoid building a huge backlog of un-QA'd PRs, we avoid scenarios where we have many blockers introduced to staging at once, which saves our team time (particularly for the deployer that week).
However, this practice results in broken prod deploy comments. If a checklist is shipped and the prod deploy for that checklist is skipped, then the PRs included in that checklist never receive "deployed to prod" comments, even after the next complete and successful prod deploy. This is a problem because it leads to confusion for our BZ process and makes it harder to track when payments are due to contributors.

The root cause for this problem with deploy comments in this edge case lies in getDeployPullRequestList. That action looks for the version of the last completed deploy for the given environment, and tries to enumerate all PRs between that version and the just-deployed version. It does not work in the edge case of a skipped prod deploy, because there's always a staging deploy with a matching version. When determining if a deploy matches the given environment, it looks at whether the release for that deploy is a prerelease (staging) or finalized (production). However, even if a prod deploy was skipped, that skipped deploy workflow was triggered by a finalized release. So our code mistakes the staging deploy with the same version code as the skipped prod deploy for a prod deploy.

Solution

First some background... Right now, we have two workflows involved in creating deploys.

  1. First, deploy.yml is triggered when code is pushed to staging or production branches. This is the "root" event that puts a deploy in motion. deploy.yml just:
    1. Creates and pushes a tag for the version on that branch (staging)
    2. creates a release for that tag (staging)
    3. finalizes the release for that tag (production)
  2. The event of creating or finalizing a release in turn triggers platformDeploy.yml, which builds the code for that release, puts it where it needs to go, and leaves deploy comments when it's done.

So to solve this problem of staging deploys being mistaken for prod deploys, we can actually combine these two workflows into a single workflow, which we'll call deploy.yml. It will:

  1. Create and push a tag for the version on the branch that triggered it (staging)
  2. Run the staging or prod deploy using the code on that branch
  3. Once the actual deploy completes successfully on at least one platform:
    1. Leave deploy comments on all PRs since the last prerelease (staging) or finalized release (prod)
    2. Once the deploy completes successfully, create or finalize a release for that deploy

By waiting for an actual deploy to complete before creating a release, we never have to worry about releases being associated with cancelled deploys, or about them being falsely matched with a workflow different than the one that created or finalized them.
When looking for the last completed staging deploy, we simply look at the newest release (prerelease or finalized). When looking for the last completed prod deploy, we simply look at the latest finalized release.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~019fe3122b194caa3c
  • Upwork Job ID: 1830720256093569547
  • Last Price Increase: 2024-09-02
Issue OwnerCurrent Issue Owner: @
@roryabraham roryabraham added Weekly KSv2 Improvement Item broken or needs improvement. labels Sep 2, 2024
@roryabraham roryabraham added the Bug Something is broken. Auto assigns a BugZero manager. label Sep 2, 2024
Copy link

melvin-bot bot commented Sep 2, 2024

Triggered auto assignment to @sakluger (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Sep 2, 2024
@roryabraham
Copy link
Contributor Author

auto-assigner is going to be confused here. I'll be the "C+" for this issue and @rayane-djouah will be the contributor. And we're skipping the proposal phase because we've already hashed out the solution in slack

@roryabraham roryabraham added the External Added to denote the issue can be worked on by a contributor label Sep 2, 2024
@melvin-bot melvin-bot bot changed the title Combine deploy.yml and platformDeploy.yml [$250] Combine deploy.yml and platformDeploy.yml Sep 2, 2024
Copy link

melvin-bot bot commented Sep 2, 2024

Job added to Upwork: https://www.upwork.com/jobs/~019fe3122b194caa3c

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 2, 2024
Copy link

melvin-bot bot commented Sep 2, 2024

Current assignee @rayane-djouah is eligible for the External assigner, not assigning anyone new.

@roryabraham roryabraham removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Sep 2, 2024
@roryabraham
Copy link
Contributor Author

@rayane-djouah any update here?

@rayane-djouah
Copy link
Contributor

PR in progress

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Sep 5, 2024
@rayane-djouah
Copy link
Contributor

@roryabraham PR ready for review

@rayane-djouah
Copy link
Contributor

No checklist is needed here since this is a Github Actions improvement

@rayane-djouah
Copy link
Contributor

@roryabraham Is there anything left to do here?

@roryabraham
Copy link
Contributor Author

Nope, I think we can issue payment now. Thanks for the follow-through

@roryabraham roryabraham added Awaiting Payment Auto-added when associated PR is deployed to production and removed Reviewing Has a PR in review labels Sep 28, 2024
@roryabraham
Copy link
Contributor Author

@sakluger no need for a regression period, this can be paid right away

Copy link

melvin-bot bot commented Sep 30, 2024

@sakluger @roryabraham @rayane-djouah this issue is now 4 weeks old, please consider:

  • Finding a contributor to fix the bug
  • Closing the issue if BZ has been unable to add the issue to a VIP or Wave project
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@roryabraham roryabraham changed the title [$250] Combine deploy.yml and platformDeploy.yml [PAY][$250] Combine deploy.yml and platformDeploy.yml Oct 1, 2024
@sakluger sakluger added Daily KSv2 and removed Weekly KSv2 labels Oct 2, 2024
@sakluger
Copy link
Contributor

sakluger commented Oct 2, 2024

@rayane-djouah here's the Upwork offer: https://www.upwork.com/nx/wm/offer/104233258

Please let me know once you've accepted it.

@rayane-djouah
Copy link
Contributor

@sakluger - Offer accepted. Thanks!

@sakluger
Copy link
Contributor

sakluger commented Oct 3, 2024

All paid, thanks!

@sakluger sakluger closed this as completed Oct 3, 2024
@rayane-djouah
Copy link
Contributor

@sakluger - Coming from https://expensify.slack.com/archives/C01GTK53T8Q/p1728319262830679, I haven't received my payment for this issue. Could you please look into it? Thank you!

@sakluger
Copy link
Contributor

sakluger commented Oct 7, 2024

Hi @rayane-djouah, sorry about that, there is an Upwork bug affecting some payments. I added a $250 bonus on the original contract. Should be good to go now!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Improvement Item broken or needs improvement.
Projects
None yet
Development

No branches or pull requests

3 participants