Skip to content

Fix tag_deploy regression and add SKIP_TRANSFER for deploy reruns#12665

Merged
mekarpeles merged 5 commits into
masterfrom
fix/deploy-tag-and-skip-transfer
May 12, 2026
Merged

Fix tag_deploy regression and add SKIP_TRANSFER for deploy reruns#12665
mekarpeles merged 5 commits into
masterfrom
fix/deploy-tag-and-skip-transfer

Conversation

@mekarpeles

@mekarpeles mekarpeles commented May 8, 2026

Copy link
Copy Markdown
Member

Fixes regression introduced in #12433

Technical

tag_deploy regression (#12433): That PR moved tag_deploy out of deploy_openlibrary() into deploy_wizard() unconditionally. But tag_deploy requires $DEPLOY_DIR/openlibrary to exist — if the wizard's deploy step is skipped (user answers "n"), the directory never exists and set -e kills the whole wizard (as flagged by the Copilot review on that PR). Fixed by moving tag_deploy back inside deploy_openlibrary() where the fresh clone is guaranteed present.

Standalone ./deploy.sh tag fallback: Both tag_deploy and tag_release now fall back to the script's own repo ($SCRIPT_DIR) when $DEPLOY_DIR/openlibrary is gone — so running ./deploy.sh tag after cleanup (e.g. to re-push a tag after a full deploy) works without re-cloning.

SKIP_TRANSFER env var: Wraps the SCP file-copy, server git-init, docker prune, and image-pull steps in deploy_openlibrary(). Set SKIP_TRANSFER=1 to skip all transfers when rerunning after a partial failure where servers already have current files. tag_deploy still runs.

Bonus fixes:

  • Wizard regex ^[Yy$]^[Yy]$ ($ was inside the character class, matching a literal $ rather than end-of-string)
  • Remove dead commented-out #tag_deploy block in wizard
  • Remove redundant git describe call (value already set by tag_release)
  • Fix tab/space indentation inconsistency in SKIP_TRANSFER block

Testing

  • Run ./deploy.sh wizard, answer "n" to the openlibrary deploy — wizard should continue past the tag step without aborting
  • Run ./deploy.sh openlibrary standalone — tag_deploy fires and pushes deploy-* + production tags
  • Run ./deploy.sh tag after a completed deploy (tmp dir already cleaned up) — falls back to local repo and pushes tags
  • Run SKIP_TRANSFER=1 ./deploy.sh openlibrary — skips SCP/image pulls but still tags

Screenshot

Stakeholders

@mekarpeles

- 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)
Copilot AI review requested due to automatic review settings May 8, 2026 05:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_deploy execution back into deploy_openlibrary() and remove the unconditional wizard tagging step to avoid failures when $DEPLOY_DIR/openlibrary doesn’t exist.
  • Add a repo-location fallback for tag_deploy/tag_release so ./deploy.sh tag can run even after the temp deploy directory has been cleaned up.
  • Introduce SKIP_TRANSFER to skip transfer/image-related steps during deploy reruns while still performing tagging.

Comment thread scripts/deployment/deploy.sh Outdated
Comment thread scripts/deployment/deploy.sh
Comment thread scripts/deployment/deploy.sh Outdated
mekarpeles added 2 commits May 7, 2026 22:40
… 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.
@mekarpeles

Copy link
Copy Markdown
Member Author

https://github.com/internetarchive/openlibrary/blob/a6f7ed6bcc2191f26e7523ccdd585329b9a1f366/scripts/deployment/deploy.sh#L462C1-L462C15

tag_deploy should not happen by default, but if we do it from the wizard then it should set env variable to do it.

mekarpeles added 2 commits May 9, 2026 11:31
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.
@mekarpeles mekarpeles added the Priority: 1 Do this week, receiving emails, time sensitive, . [managed] label May 11, 2026
@mekarpeles

Copy link
Copy Markdown
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.

@mekarpeles mekarpeles merged commit c9568c8 into master May 12, 2026
8 checks passed
@mekarpeles mekarpeles deleted the fix/deploy-tag-and-skip-transfer branch May 12, 2026 19:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Priority: 1 Do this week, receiving emails, time sensitive, . [managed]

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants