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

DB Safety feature and GCP opta destroy + config upload #163

Merged
merged 3 commits into from
Mar 28, 2021

Conversation

juandiegopalomino
Copy link
Collaborator

  1. GCP and AWS dbs now have enable deletion protection enabled when possible, making the owners apply at least once with safety false before being able to destroy the dbs. This obvious safety feature would be much for prevalent if not for this stupid 2.5 year old bug which people are still begging for: "Variables may not be used here" for prevent_destroy hashicorp/terraform#22544
  2. Opta configs are uploaded to cloud storage bucket for gcp too
  3. Opta destroy now destroys services before environment for gcp too. And no special cases needed to be handled lol!

opta/core/gcp.py Outdated Show resolved Hide resolved
@kevjin
Copy link
Contributor

kevjin commented Mar 28, 2021

what happens when a customer tries to run opta destroy when safety is enabled? does the terraform destroy fail unsafely midway?
A big part of the reason we initially wanted to support rollback, was to recover terraform state that is commonly lost when apply/destroy fails hard.

opta/core/gcp.py Outdated Show resolved Hide resolved
@juandiegopalomino
Copy link
Collaborator Author

what happens when a customer tries to run opta destroy when safety is enabled? does the terraform destroy fail unsafely midway?
A big part of the reason we initially wanted to support rollback, was to recover terraform state that is commonly lost when apply/destroy fails hard.

That's a great question Kevin. So as it stands it will fail harshly with opta apply needing to be run again w/ safety on before destroying again (the state should not be mangled though). An update on this would be to catch the error beforehand and fail elegantly, which we can do in a follow up pr

@codecov
Copy link

codecov bot commented Mar 28, 2021

Codecov Report

Merging #163 (bc67aa5) into main (476442a) will decrease coverage by 0.84%.
The diff coverage is 34.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #163      +/-   ##
==========================================
- Coverage   76.27%   75.43%   -0.85%     
==========================================
  Files          39       39              
  Lines        2112     2149      +37     
==========================================
+ Hits         1611     1621      +10     
- Misses        501      528      +27     
Flag Coverage Δ
unittests 75.43% <34.14%> (-0.85%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
opta/commands/apply.py 78.94% <25.00%> (-3.00%) ⬇️
opta/core/gcp.py 43.24% <33.33%> (-11.76%) ⬇️
opta/commands/destroy.py 61.53% <36.84%> (-11.94%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 476442a...bc67aa5. Read the comment docs.

@juandiegopalomino juandiegopalomino merged commit 73cb441 into main Mar 28, 2021
@juandiegopalomino juandiegopalomino deleted the jd/safety branch March 28, 2021 22:54
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.

3 participants