fix: make consul deploy swaps atomic#78
Merged
Merged
Conversation
Delay old ASG cleanup until every selected Consul pod passes health checks so one failed pod rolls back the whole deploy set. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Pull request overview
Updates the Consul-based deploy flow to behave atomically: only clean up the old ASGs after all selected Consul pods pass health checks, and tear down the new-release ASGs for the whole deploy set if any pod fails. This reduces the chance of ending up in a partially-swapped state during Consul deploys.
Changes:
- Defers old-ASG cleanup until after all Consul service health checks pass; aborts and deletes new-release ASGs for the full selected pod set on any failure.
- Removes per-pod old-ASG deletion from the Consul health-check loop to support atomic swaps.
- Bumps the package version to
0.0.94.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/app.ts | Reworks Consul health-check success/failure handling to make deploy cleanup atomic and postpones old-ASG deletion until all pods are healthy. |
| package.json | Bumps Stack version from 0.0.93 to 0.0.94. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+405
to
+420
| // Only perform a swap if there are already running instances. | ||
| if (!this.options.applyOnly && alreadyRunningInstances.length) { | ||
| // It's possible the above apply command removed instances, so need to check again | ||
| const currentlyRunningInstances = await this.alreadyRunningInstances([ | ||
| ...new Set(podNames).difference(failedPods), | ||
| ]); | ||
|
|
||
| if (currentlyRunningInstances.length) { | ||
| const currentlyRunningInstancesByPod = | ||
| await this.alreadyRunningInstancesByPod([ | ||
| ...new Set(podNames).difference(failedPods), | ||
| ]); | ||
| // Run the pre-terminate script for each pod | ||
| const preTerminateScriptExitStatus = | ||
| await this.runPreContainerShutdownScripts( | ||
| [...new Set(podNames).difference(failedPods)], |
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.
Summary
0.0.94for the next stable release.Rollout
merkle-team/stack.v0.0.94from the merge commit.v0.0.94GitHub Release asset and Docker image.deploy-stack-action@v1defaults to latest stable Stack whenstack-versionis unset, normal deploy workflows will pick upv0.0.94on deployers that do not already havestackinstalled.stack, runstack updateor pinstack-version: v0.0.94in the workflow to force the new binary. Backend should use this before/with the backend migration-failure PR rollout.Test plan
bun run lintlocally becausebunis not installed in this environmentPart of NEYN-11598.
Made with Cursor