Fixed destroy tests staging databases when a PR is merged#730
Conversation
ref no-issue - Fixed destroy staging databases when a PR is merged
WalkthroughThis change refactors the GitHub Actions CI/CD workflow by extracting the logic for destroying test databases during the staging deployment for the Possibly related PRs
Suggested labels
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/cicd.yml (1)
436-444: Refine database-list filter and improve error handling
The filtername~test*may not strictly match only test databases (and might misinterpret the*quantifier), and the script lacks strict failure controls. I recommend:
- Anchor the regex to match names beginning with “test”:
- TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~test*" --format="value(name)" --project ${GCP_PROJECT}) + TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~^test.*$" --format="value(name)" --project ${GCP_PROJECT})
- Add
set -euo pipefailat the top of therun:block to ensure the step fails fast on any error..github/workflows/ephemeral-staging-teardown.yml (1)
92-104: Tighten regex and enforce strict shell options
To ensure only the intended PR-specific test databases are deleted and to catch failures immediately:
- Anchor the regex for matching PR test DBs:
- TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~pr_${PR_NUMBER}_test*" --format="value(name)" --project ${GCP_PROJECT}) + TEST_DATABASES=$(gcloud sql databases list --instance=stg-netherlands-activitypub --filter="name~^pr_${PR_NUMBER}_test.*$" --format="value(name)" --project ${GCP_PROJECT})
- Prepend
set -euo pipefailto therun:block:run: | set -euo pipefail for PR_NUMBER in ${DESTROY_PRS}; do …
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
.github/workflows/cicd.yml(3 hunks).github/workflows/ephemeral-staging-teardown.yml(1 hunks)
| cleanup-staging-tests-dbs: | ||
| name: Cleanup Staging Tests Databases | ||
| runs-on: ubuntu-latest | ||
| if: github.ref == 'refs/heads/main' | ||
| steps: | ||
| - name: "Authenticate with GCP" | ||
| uses: google-github-actions/auth@v2 | ||
| with: | ||
| token_format: access_token | ||
| workload_identity_provider: projects/687476608778/locations/global/workloadIdentityPools/github-oidc-activitypub/providers/github-provider-activitypub | ||
| service_account: stg-activitypub-cicd-stg-envs@ghost-activitypub.iam.gserviceaccount.com |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add explicit job dependency to enforce correct ordering
This new cleanup-staging-tests-dbs job currently runs in parallel with all other jobs on main. To prevent potential race conditions (for example, deleting test databases while a staging or production deploy is still in progress), consider adding a needs: deploy-staging (or appropriate job) so it only executes after the deployment steps finish.
🤖 Prompt for AI Agents
In .github/workflows/cicd.yml around lines 424 to 434, the
cleanup-staging-tests-dbs job lacks an explicit dependency and runs in parallel
with other jobs, risking race conditions. Add a needs: deploy-staging line under
the cleanup-staging-tests-dbs job definition to ensure it only runs after the
deploy-staging job completes, enforcing correct job execution order.
1 participant
ref no-issue