DOCSP-46223: Add Disaster Recovery example script for go sdk #74
DOCSP-46223: Add Disaster Recovery example script for go sdk #74cbullinger wants to merge 3 commits intomongodb:mainfrom
Conversation
dacharyc
left a comment
dacharyc
left a comment
There was a problem hiding this comment.
Overall looks good, but a couple of small fixups are needed. There are a couple of files here that I think probably should not be here, and we probably need to provide some info about the new expected DR env vars.
There was a problem hiding this comment.
It looks like this is a net new file as of this PR, so we can just omit this from this PR, can't we? I assume we don't need a comment about tests moving if they're net new here.
There was a problem hiding this comment.
Can we omit this empty file from this PR?
There was a problem hiding this comment.
This is the first test file I've seen in the examples directory, so just curious about the decision to test here instead of in internal.
There was a problem hiding this comment.
it was so i could test the private helper functions that i ended up leaving in main.go so they'd be part of the on-page snippet
| AddNodes: defaultAddNodes, | ||
| } | ||
|
|
||
| o.Scenario = strings.ToLower(strings.TrimSpace(os.Getenv("DR_SCENARIO"))) |
There was a problem hiding this comment.
I see we're now expecting some disaster-recovery-related vars in the env file, but I don't see any info about that in the README or .env.example. How are we exposing these new required vars to devs?
There was a problem hiding this comment.
these are now part of the config in a new stanza (similar to the scaling settings)
| // | ||
| // DR_SCENARIO (req) regional-outage | data-deletion | ||
| // ATLAS_PROJECT_ID (red unless provided via config loader) | ||
| // ATLAS_CLUSTER_NAME (req) target cluster name |
There was a problem hiding this comment.
It looks like this PR is expecting ATLAS_CLUSTER_NAME in the env file, but other PRs have them in the config. Should we be consistent about where we expect this to live?
| var summary string | ||
| var opErr error | ||
|
|
||
| switch opts.Scenario { |
There was a problem hiding this comment.
Overall, nothing here jumps out at me. However, I wasn't able to actually test this. When I tried testing the regional outage scenario in Bushicorp, I get an error that scenario failed because the target region isn't found in replication specs. I didn't try to test the data recovery scenario because I'm not sure how to get/create a snapshot ID.
So my caveat here is that I haven't fully validated the functionality as "works on my machine."
| var opErr error | ||
|
|
||
| switch opts.Scenario { | ||
| case scenarioRegionalOutage: |
There was a problem hiding this comment.
Everything looks good here in terms of output files, and I was able to build and execute this new example, but same caveat applies that I wasn't able to fully complete either scenario due to configuration constraints.
| MONGODB_ATLAS_SERVICE_ACCOUNT_ID=<your_service_account_id> | ||
| MONGODB_ATLAS_SERVICE_ACCOUNT_SECRET=<your_service_account_secret> | ||
| ATLAS_DOWNLOADS_DIR="tmp/atlas_downloads" # optional download directory | ||
| CONFIG_PATH="configs/config.development.json" # optional path to Atlas config file |
There was a problem hiding this comment.
Just noting I'm not seeing the new DR env vars we expect here.
2 participants
recovery/main.gowith sample disaster recovery script (for handling data restore via snapshot and regional outage scenarios)