Fix tag_deploy regression and add SKIP_TRANSFER for deploy reruns#12665
Merged
Conversation
- tag_deploy/tag_release: fall back to the script's own repo when $DEPLOY_DIR/openlibrary doesn't exist, so './deploy.sh tag' works standalone after cleanup and the wizard no longer aborts if the openlibrary deploy step is skipped (fixes regression from #12433) - deploy_openlibrary: move tag_deploy call inside the function (after the transfer block) so it runs for standalone deploys, not just the wizard - Add SKIP_TRANSFER env var: set SKIP_TRANSFER=1 to skip file copy, git-init-on-servers, prune, and image pulls -- useful when rerunning after a partial failure where servers already have current files - Fix wizard regex typo ^[Yy$] -> ^[Yy]$ (would have matched literal $) - Remove dead commented-out tag_deploy in wizard and redundant git describe call (tag_release already sets $LATEST_TAG_NAME)
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates scripts/deployment/deploy.sh to make deploy tagging more robust (avoiding a wizard abort when the Open Library deploy step is skipped), improve standalone tagging behavior, and add an escape hatch for rerunning deploys after partial failures.
Changes:
- Move
tag_deployexecution back intodeploy_openlibrary()and remove the unconditional wizard tagging step to avoid failures when$DEPLOY_DIR/openlibrarydoesn’t exist. - Add a repo-location fallback for
tag_deploy/tag_releaseso./deploy.sh tagcan run even after the temp deploy directory has been cleaned up. - Introduce
SKIP_TRANSFERto skip transfer/image-related steps during deploy reruns while still performing tagging.
… non-git repo guard - check_for_local_changes: skip git status if dir exists but has no .git (partial failure before git init would abort deploy with set -e) - SKIP_TRANSFER: check != '1' instead of -z so SKIP_TRANSFER=0 does not accidentally skip the transfer - tag_deploy/tag_release: print a warning with the HEAD SHA when falling back to the local repo so operators know which commit will be tagged
…MAGES deploy_openlibrary now has two independent skip gates: - SKIP_OL_TRANSFER_CODE=1: skips SCP file copy, server git-init, docker prune - SKIP_OL_TRANSFER_IMAGES=1: skips docker image pull across all nodes Wizard prompts for both after 'Run openlibrary deploy?': 'Transfer codebase to servers? [Y/n]' 'Transfer docker images to servers? [Y/n]' so operators can skip whichever expensive step they don't need on a rerun without memorising env var names. tag_deploy always runs regardless.
Member
Author
|
tag_deploy should not happen by default, but if we do it from the wizard then it should set env variable to do it. |
deploy_openlibrary no longer tags automatically -- tag_deploy only fires when TAG_DEPLOY=1 is set, so standalone './deploy.sh openlibrary' does not tag. The wizard sets TAG_DEPLOY=1 when the operator answers Y to the new 'Tag deploy?' prompt (default Y). Also rename wizard prompt 'Transfer docker images to servers?' to the shorter 'Deploy Docker Images?' and document TAG_DEPLOY in usage.
… repo When DEPLOY_DIR/openlibrary is gone (e.g. re-running `./deploy.sh tag` after cleanup), the fallback silently tagged production from whatever local HEAD happened to be checked out — risking a wrong-commit retag if the operator pulled new commits after deploying. Now the fallback prints the local HEAD SHA and requires explicit `y` confirmation before pushing. Abort path lets the operator check out the correct commit first.
Member
Author
|
Talked about it with Jim and given our current deploy is broken, merging and we can revisit if there's a better way to do this. I'll run during Thursday deploy. |
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
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.
Fixes regression introduced in #12433
Technical
tag_deployregression (#12433): That PR movedtag_deployout ofdeploy_openlibrary()intodeploy_wizard()unconditionally. Buttag_deployrequires$DEPLOY_DIR/openlibraryto exist — if the wizard's deploy step is skipped (user answers "n"), the directory never exists andset -ekills the whole wizard (as flagged by the Copilot review on that PR). Fixed by movingtag_deployback insidedeploy_openlibrary()where the fresh clone is guaranteed present.Standalone
./deploy.sh tagfallback: Bothtag_deployandtag_releasenow fall back to the script's own repo ($SCRIPT_DIR) when$DEPLOY_DIR/openlibraryis gone — so running./deploy.sh tagafter cleanup (e.g. to re-push a tag after a full deploy) works without re-cloning.SKIP_TRANSFERenv var: Wraps the SCP file-copy, server git-init, docker prune, and image-pull steps indeploy_openlibrary(). SetSKIP_TRANSFER=1to skip all transfers when rerunning after a partial failure where servers already have current files.tag_deploystill runs.Bonus fixes:
^[Yy$]→^[Yy]$($was inside the character class, matching a literal$rather than end-of-string)#tag_deployblock in wizardgit describecall (value already set bytag_release)SKIP_TRANSFERblockTesting
./deploy.shwizard, answer "n" to the openlibrary deploy — wizard should continue past the tag step without aborting./deploy.sh openlibrarystandalone —tag_deployfires and pushesdeploy-*+productiontags./deploy.sh tagafter a completed deploy (tmp dir already cleaned up) — falls back to local repo and pushes tagsSKIP_TRANSFER=1 ./deploy.sh openlibrary— skips SCP/image pulls but still tagsScreenshot
Stakeholders
@mekarpeles