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

Failed CP's can break our CI/CD #6863

Closed
roryabraham opened this issue Dec 21, 2021 · 22 comments
Closed

Failed CP's can break our CI/CD #6863

roryabraham opened this issue Dec 21, 2021 · 22 comments
Labels
Engineering Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff Monthly KSv2 Not a priority

Comments

@roryabraham
Copy link
Contributor

Action Performed:

  1. Merge a dummy PR.
  2. Create a revert PR for that dummy PR.
  3. Give the revert PR the CP Staging label.
  4. Merge the revert PR.
  5. You'll observe that the app version on the deploy checklist increases. (BUG)
  6. There should be a merge conflict with the CP PR.
  7. Close the CP PR without resolving conflicts or merging it.
  8. Then without running any other CP's first, close the checklist to run a production deploy.

Expected Result:

The production and staging deploys should complete successfully, and we should have a new checklist.

Actual Result:

The production and staging deploys complete successfully, but creating a new checklist fails with the following type of error:

Found tag of previous StagingDeployCash: 1.1.22-1
Getting pull requests merged between the following refs: 1.1.22-1 1.1.22-2
Running command:  git log --format="{[%B]}" 1.1.22-1...1.1.22-2
fatal: ambiguous argument '1.1.22-1...1.1.22-2': unknown revision or path not in the working tree.

Where in this case 1.1.22-1 is the tag that was posted in the deploy checklist, but never actually created.

Workaround:

DANGER:

  1. Manually update the checklist to have the tag matching the latest production deploy (GitHub Release).
  2. Re-run the finishReleaseCycle.yml workflow
  3. IMPORTANT NOTE: This will attempt to deploy all the code from staging to production. It's imperative that you manually kill the workflow before that happens. It's best to kill the updateProtectedBranch workflow that's attempting to merge the code from staging -> production, but if you're too slow then you can kill the deploy.yml or platformDeploy.yml as well.

Platform:

GitHub Actions

@roryabraham roryabraham added Engineering Weekly KSv2 Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff labels Dec 21, 2021
@roryabraham roryabraham self-assigned this Dec 21, 2021
@roryabraham
Copy link
Contributor Author

We've seen similar problem that suggests a similar solution to this, namely: "Only update the app version on the StagingDeployCash once the tag has been created and pushed"

@roryabraham
Copy link
Contributor Author

No update here

@roryabraham
Copy link
Contributor Author

Another good improvement to make once pascalgn/automerge-action#177 is merged would be to delete the unmerged PR branch if the automerge-action fails.

@roryabraham
Copy link
Contributor Author

No update

@roryabraham
Copy link
Contributor Author

No update

@roryabraham
Copy link
Contributor Author

This is a pretty rare edge case that requires some human error to reproduce it (i.e: begin a CP but don't finish it). Going to move this to monthly and send it back to the pool for now. Would still be good to fix someday.

@MelvinBot MelvinBot removed the Overdue label Mar 15, 2022
@roryabraham roryabraham added Monthly KSv2 and removed Weekly KSv2 labels Mar 15, 2022
@roryabraham roryabraham removed their assignment Mar 15, 2022
@melvin-bot
Copy link

melvin-bot bot commented May 30, 2022

@roryabraham, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@roryabraham roryabraham reopened this Feb 2, 2023
@roryabraham
Copy link
Contributor Author

This is a a bug, not a new feature.

This isn't a bug that a user would experience

the "user" in this case is a deployer

@roryabraham roryabraham added Improvement Item broken or needs improvement. and removed NewFeature Something to build that is a new item. labels Feb 2, 2023
@thienlnam thienlnam assigned thienlnam and yuwenmemon and unassigned thienlnam and yuwenmemon Feb 2, 2023
@roryabraham roryabraham changed the title [Feature] Failed CP's can break our CI/CD Failed CP's can break our CI/CD Feb 2, 2023
@thienlnam
Copy link
Contributor

@roryabraham I've started to look into this (Slack Thread)

I am a little unsure how to proceed since this cherry pick action fails when there is some kind of merge conflict and needs to be merged manually.

But this workflow only breaks when we abort the CP and not even merge manually right? So if we were to move the logic to bump the version in the checklist to after it was successful or when the manual PR was merged - would it even be possible to check that it was a manual merge?

@melvin-bot melvin-bot bot added the Overdue label Mar 21, 2023
@thienlnam
Copy link
Contributor

Going to unassign this for now, having a hard time allocating time to look into this - might be better suited for an external agency since it's front-end

@melvin-bot melvin-bot bot removed the Overdue label Mar 22, 2023
@thienlnam thienlnam removed their assignment Mar 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Jun 5, 2023

@roryabraham, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@melvin-bot melvin-bot bot closed this as completed Jun 5, 2023
@roryabraham roryabraham reopened this Jun 6, 2023
@NajiNazzal
Copy link

Can I register?

@melvin-bot melvin-bot bot added the Overdue label Aug 7, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 7, 2023

📣 @NajiNazzal! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  2. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  3. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@NajiNazzal
Copy link

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01a352a7b4bf7c67cd

@melvin-bot
Copy link

melvin-bot bot commented Aug 7, 2023

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@melvin-bot melvin-bot bot closed this as completed Aug 21, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 21, 2023

@roryabraham, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@melvin-bot melvin-bot bot removed the Overdue label Aug 21, 2023
@roryabraham roryabraham reopened this Sep 2, 2023
Copy link

melvin-bot bot commented Nov 20, 2023

@roryabraham, this Monthly task hasn't been acted upon in 6 weeks; closing.

If you disagree, feel encouraged to reopen it -- but pick your least important issue to close instead.

@melvin-bot melvin-bot bot closed this as completed Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Engineering Improvement Item broken or needs improvement. Internal Requires API changes or must be handled by Expensify staff Monthly KSv2 Not a priority
Projects
None yet
Development

No branches or pull requests

7 participants