Prefix dump scripts with keyed resource name#12
Merged
stevethomas merged 24 commits intomainfrom Mar 26, 2026
Merged
Conversation
Prevents multiple YOLO apps on the same scheduler instance from overwriting each other's dump scripts and temp files. Scripts and dumps now live under /home/ubuntu/yolo/ with app-specific filenames via keyedResourceName (e.g. yolo-production-lp-app-mysqlbackup.sh). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The dump cron is a dump concern, not a scheduler concern. The backup step now owns its own /etc/cron.d/ entry with a keyed resource name, removing the MYSQLBACKUP_PATH placeholder from the scheduler stub. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use ~/yolo-{env}-{app}/ as the app directory so scripts get simple
names and dumps are isolated per app too.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
The directory is created by root during deploy but the cron runs as ubuntu — without chown, ubuntu can't create the dumps subdirectory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
chown -R /home/ubuntu in the dedicated step instead of individual chowns scattered across dump and bash profile steps. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Follow the Nightwatch pattern — unlink files on skip so disabling the feature in the manifest actually removes the artefacts. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Use file_exists/is_dir checks consistent with Nightwatch pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Keeps home directory clean — all YOLO artefacts live under
/home/ubuntu/yolo/{keyed-resource-name}/.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
The chown -R /home/ubuntu was running before the dump steps created root-owned directories. Moving it to just before RestartServicesStep ensures all files written during the start phase get chowned. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cron now logs to /var/log/yolo/{keyed-resource-name}/mysqlbackup.log
instead of /dev/null. Added logrotate config for yolo logs (weekly,
12 rotations). Backup step creates the log directory on sync.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
One shared /etc/logrotate.d/yolo config with /var/log/yolo/*/*.log covers all apps — no per-app logrotate entry needed. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Avoids confusion with the app directory at /var/www/{name}.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Centralise path construction instead of inline sprintf in each step. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Log directory is created by root but cron writes as ubuntu. Guarded with test -d since the directory only exists when mysqldump is enabled. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Scripts are harmless without the cron — always write them so they're available for manual use. Only the cron entry is conditional. Removes cleanup logic and simplifies both steps. Table dump step has no conditional at all now. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
mkdir in SetOwnershipAndPermissionsStep so the log directory exists on all server groups, not just scheduler. Removes the test -d guard. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Create ~/yolo/{app}/ and /var/log/yolo/{app}/ once in the ownership
step so dump steps just write files like everything else.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Runs first in StartCommand to create ~/yolo/{app}/ and
/var/log/yolo/{app}/ before any file-writing steps.
SetOwnershipAndPermissionsStep stays last for chown.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Both dump scripts should be executable for direct invocation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Scripts are always invoked via bash, chmod unnecessary. Added StartCommand step ordering and filesystem conventions to CLAUDE.md. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Don't chown the entire /var/www — just the app's directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
stevethomas
commented
Mar 26, 2026
Member
Author
stevethomas
left a comment
stevethomas
left a comment
There was a problem hiding this comment.
Review Summary
Verdict: ✅ Approved
Risk Assessment
| Category | Status |
|---|---|
| Data Integrity | ✅ |
| Security | ✅ |
| Breaking Changes | ✅ |
| Operational | |
| Test Coverage | ✅ |
Findings
Rollback scenario: deploying this creates a keyed cron file at /etc/cron.d/{app}-mysqlbackup. Rolling back to old code rewrites the scheduler stub with the embedded backup line but does not remove the keyed cron file — both fire at 09:00, producing duplicate S3 uploads. Add a rollback note to the PR description.
🤖 Reviewed by Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
~/yolo/{keyed-resource-name}/(e.g.~/yolo/yolo-production-lp-app/mysqlbackup.sh,~/yolo/yolo-production-lp-app/dumps/)/etc/cron.d/entry via a dedicated cron stub/var/log/yolo/{keyed-resource-name}/mysqlbackup.loginstead of/dev/null, with logrotate (weekly, 12 rotations)mysqldumpflag — only the cron entry is conditionalProvisionDirectoriesStepruns first inStartCommandto create~/yolo/{app}/and/var/log/yolo/{app}/SetOwnershipAndPermissionsStepmoved to run last (before restart) — chowns/home/ubuntu,/var/log/yolo, and/var/wwwPaths::yoloDir()andPaths::logDir()helpers centralise path constructionAnything the reviewer needs to know
🤖 Generated with Claude Code