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

ci: disaster recovery dry run on prod #2017

Merged
merged 2 commits into from
Feb 21, 2025

Conversation

rubenruizdegauna
Copy link
Contributor

This PR:

  • fixes slack notifications for staging disaster recovery errors
  • removes --recursive from rm when deleting original
  • adds job for production bucket

@rubenruizdegauna rubenruizdegauna requested a review from a team as a code owner February 12, 2025 09:57

- name: set README and a file that will not be rolled back
run: |
echo "This folder is meant to test the disaster recevery dry-run." > DISASTER_TEST_README.md

Choose a reason for hiding this comment

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

Suggested change
echo "This folder is meant to test the disaster recevery dry-run." > DISASTER_TEST_README.md
echo "This folder is meant to test the disaster recovery dry-run." > DISASTER_TEST_README.md

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in c160495

Comment on lines +42 to +44
echo AWS_PROFILE="${{ env.TEMP_AWS_PROFILE }}" >> $GITHUB_ENV
echo AWS_REGION="${{ env.AWS_REGION }}" >> $GITHUB_ENV
echo TEST_FOLDER_ABS_PATH="s3://${{ env.BUCKET_NAME }}/${{ env.MANDATORY_PREFIX }}/${{ env.TEST_FOLDER }}" >> $GITHUB_ENV

Choose a reason for hiding this comment

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

Why are those variables set up here instead of the env block at the top?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't want to expose AWS_PROFILE and AWS_REGION before calling aws_credentials as it could affect that script.
The third one was just not to copy and paste the same strings on top in 2 variables :)


- name: Get current datetime and sleep for a couple of minutes
run: |
now=$( date +"%m-%d-%Y %H:%M:%S %z" )

Choose a reason for hiding this comment

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

Sorry I didn't spot this on the staging one. Should we add --utc to assure the date is in UTC?
🤔 Maybe it is not important since it includes %z info

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I'll add it just in case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in c160495

with:
payload: |
{
"text": ":rotating_light: Dry-Run Recover S3 Repository failed :warning: :warning: :warning: @hero check <${{ env.GITHUB_JOB_URL }}> :rotating_light:"

Choose a reason for hiding this comment

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

Nit: I'm not sure if we should say Dry-Run, even if it runs in a testing folder it is actually running (and failing if we get this message! 😅 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds reasonable :) I'll rename it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done in c160495

@coveralls
Copy link

Pull Request Test Coverage Report for Build 13431009553

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall first build on ci/disaster_recovery_dry_run_prod at 57.42%

Totals Coverage Status
Change from base Build 13258238324: 57.4%
Covered Lines: 15404
Relevant Lines: 26827

💛 - Coveralls

Copy link

@sigilioso sigilioso left a comment

Choose a reason for hiding this comment

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

LGTM

@rubenruizdegauna rubenruizdegauna merged commit da069a4 into master Feb 21, 2025
23 of 24 checks passed
@rubenruizdegauna rubenruizdegauna deleted the ci/disaster_recovery_dry_run_prod branch February 21, 2025 14:16
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.

4 participants