Skip to content

Commit bf37391

Browse files
fix(ci): prepare tasks before app rollout and promote at cutover (#7902)
* fix(ci): prepare tasks before app rollout and promote at cutover * fix(ci): correlate retry snapshots before checking failure
1 parent a730e59 commit bf37391

3 files changed

Lines changed: 230 additions & 87 deletions

File tree

‎.github/scripts/test-trigger-deploy.py‎

Lines changed: 151 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import json
33
import os
44
from pathlib import Path
5+
import re
56
import subprocess
67
import tempfile
78
import unittest
@@ -19,6 +20,17 @@ def execution(start=1000, digest=DIGEST, identifier='execution-current'):
1920
}
2021

2122

23+
def deploy_action(identifier='action-current', start=1000, status='InProgress'):
24+
return {'actionName': 'Deploy_to_ECS', 'actionExecutionId': identifier,
25+
'startTime': start, 'status': status, 'output': {}}
26+
27+
28+
def deploy_state(execution_id='execution-current', action_id='action-current', deployment_id='d-current'):
29+
return {'latestExecution': {'pipelineExecutionId': execution_id}, 'actionStates': [
30+
{'actionName': 'Deploy_to_ECS', 'latestExecution': {
31+
'actionExecutionId': action_id, 'externalExecutionId': deployment_id, 'status': 'InProgress'}}]}
32+
33+
2234
class DeploymentGateTests(unittest.TestCase):
2335
def run_script(self, script, args, responses):
2436
with tempfile.TemporaryDirectory() as directory:
@@ -88,7 +100,8 @@ def poll(self, updates=None, since='1000'):
88100
responses = {
89101
'list-pipeline-executions': {'json': [execution()]},
90102
'get-pipeline-execution': {'text': 'InProgress'},
91-
'list-action-executions': {'text': 'd-current'},
103+
'get-pipeline-state': {'json': deploy_state()},
104+
'list-action-executions': {'json': [deploy_action()]},
92105
'get-deployment': {'text': 'InProgress'},
93106
'list-deployment-targets': {'text': 'target-one\ttarget-two'},
94107
'get-deployment-target:target-one': {'text': 'Succeeded'},
@@ -175,10 +188,64 @@ def test_failed_and_superseded_pipeline_never_reach_deployment(self):
175188
self.assertNotIn('get-deployment ', calls)
176189

177190
def test_waits_for_queued_deploy_action(self):
178-
result, calls = self.poll({'list-action-executions': [{'text': 'None'}, {'text': 'd-current'}]})
191+
result, calls = self.poll({'list-action-executions': [
192+
{'json': []}, {'json': [deploy_action()]}]})
179193
self.assertEqual(result.returncode, 0, result.stderr)
180194
self.assertEqual(calls.count('list-action-executions '), 2)
181195

196+
def test_finds_live_deployment_before_action_history_has_output(self):
197+
result, calls = self.poll()
198+
self.assertEqual(result.returncode, 0, result.stderr)
199+
self.assertIn('get-pipeline-state --name app-pipeline', calls)
200+
self.assertIn('get-deployment --deployment-id d-current', calls)
201+
self.assertIn('Traffic cutover complete', result.stdout)
202+
203+
def test_waits_for_state_from_the_exact_pipeline_and_action(self):
204+
for stale in (None, deploy_state(execution_id='execution-old'),
205+
deploy_state(action_id='action-old'), deploy_state(deployment_id='')):
206+
with self.subTest(stale=stale):
207+
result, calls = self.poll({'get-pipeline-state': [
208+
{'json': stale}, {'json': deploy_state()}]})
209+
self.assertEqual(result.returncode, 0, result.stderr)
210+
self.assertEqual(calls.count('get-pipeline-state '), 2)
211+
self.assertEqual(calls.count('get-deployment '), 1)
212+
213+
def test_old_live_action_cannot_satisfy_a_retry(self):
214+
result, calls = self.poll({
215+
'list-action-executions': {'json': [deploy_action(), deploy_action('action-old', start=999)]},
216+
'get-pipeline-state': [
217+
{'json': deploy_state(action_id='action-old', deployment_id='d-old')},
218+
{'json': deploy_state()}],
219+
})
220+
self.assertEqual(result.returncode, 0, result.stderr)
221+
self.assertNotIn('--deployment-id d-old', calls)
222+
223+
def test_live_retry_waits_for_history_to_include_the_same_attempt(self):
224+
for previous_status in ('Failed', 'Abandoned'):
225+
with self.subTest(previous_status=previous_status):
226+
previous = deploy_action('action-old', start=999, status=previous_status)
227+
result, calls = self.poll({
228+
'list-action-executions': [
229+
{'json': [previous]},
230+
{'json': [previous, deploy_action()]},
231+
],
232+
})
233+
self.assertEqual(result.returncode, 0, result.stderr)
234+
self.assertEqual(calls.count('get-pipeline-state '), 2)
235+
self.assertEqual(calls.count('get-deployment '), 1)
236+
self.assertIn('get-deployment --deployment-id d-current', calls)
237+
238+
def test_bad_live_state_and_failed_actions_fail_closed(self):
239+
for updates in (
240+
{'get-pipeline-state': {'error': 'AccessDeniedException'}},
241+
{'get-pipeline-state': {'json': deploy_state(deployment_id='wrong-provider-id')}},
242+
{'list-action-executions': {'json': [deploy_action(status='Failed')]}},
243+
):
244+
with self.subTest(updates=updates):
245+
result, calls = self.poll(updates)
246+
self.assertNotEqual(result.returncode, 0)
247+
self.assertNotIn('get-deployment ', calls)
248+
182249
def test_failed_deployment_never_accepts_old_cutover(self):
183250
result, calls = self.poll({'get-deployment': {'text': 'Failed'}})
184251
self.assertNotEqual(result.returncode, 0)
@@ -260,5 +327,87 @@ def test_same_digest_tag_move_reports_unchanged(self):
260327
self.assertIn('app_image_changed=false', result.github_output)
261328

262329

330+
class ReleaseOrderingTests(unittest.TestCase):
331+
@classmethod
332+
def setUpClass(cls):
333+
workflow = SCRIPTS.parent / 'workflows' / 'ci.yml'
334+
parsed = subprocess.run([
335+
'bun', '-e', 'console.log(JSON.stringify(Bun.YAML.parse(await Bun.file(process.argv[1]).text())))',
336+
str(workflow)], check=True, capture_output=True, text=True)
337+
cls.jobs = json.loads(parsed.stdout)['jobs']
338+
339+
def eligible(self, job, branch, results, event='push', cancelled=False, promoted='true'):
340+
expression = self.jobs[job]['if']
341+
expression = re.sub(r'needs\.([\w-]+)\.result', lambda m: repr(results[m[1]]), expression)
342+
expression = expression.replace('needs.promote-images.outputs.promoted', repr(promoted))
343+
expression = expression.replace('github.ref', repr('refs/heads/' + branch))
344+
expression = expression.replace('github.event_name', repr(event))
345+
expression = expression.replace('!cancelled()', repr(not cancelled))
346+
expression = expression.replace('&&', ' and ').replace('||', ' or ')
347+
return eval(' '.join(expression.split()), {'__builtins__': {}})
348+
349+
def release_results(self, branch):
350+
active = ('migrate-dev', 'build-dev', 'deploy-trigger-dev') if branch == 'dev' else (
351+
'migrate', 'build-amd64', 'deploy-trigger')
352+
results = {name: 'success' if name in active else 'skipped'
353+
for name in self.jobs['promote-images']['needs']}
354+
return active, results
355+
356+
def test_uploads_and_image_builds_can_start_before_migration(self):
357+
for job in ('deploy-trigger', 'deploy-trigger-dev', 'build-amd64', 'build-dev'):
358+
self.assertFalse(self.jobs[job].get('needs'), job)
359+
for job in ('deploy-trigger', 'deploy-trigger-dev'):
360+
upload = next(step for step in self.jobs[job]['steps'] if step.get('id') == 'deploy')
361+
self.assertIn('--skip-promotion', upload['run'])
362+
363+
def test_each_release_waits_for_all_three_gates(self):
364+
for branch in ('main', 'staging', 'dev'):
365+
active, ready = self.release_results(branch)
366+
self.assertTrue(self.eligible('promote-images', branch, ready))
367+
for gate in active:
368+
self.assertIn(gate, self.jobs['promote-images']['needs'])
369+
for failure in ('failure', 'cancelled', 'skipped'):
370+
with self.subTest(branch=branch, gate=gate, result=failure):
371+
self.assertFalse(self.eligible('promote-images', branch, {**ready, gate: failure}))
372+
self.assertFalse(self.eligible('promote-images', branch, ready, cancelled=True))
373+
self.assertFalse(self.eligible('promote-images', branch, ready, event='pull_request'))
374+
375+
def test_migrations_still_require_successful_tests(self):
376+
self.assertIn('test-build', self.jobs['migrate']['needs'])
377+
for branch in ('main', 'staging'):
378+
self.assertTrue(self.eligible('migrate', branch, {'test-build': 'success'}))
379+
for result in ('failure', 'cancelled', 'skipped'):
380+
self.assertFalse(self.eligible('migrate', branch, {'test-build': result}))
381+
382+
def test_dev_build_cannot_move_deploy_tags(self):
383+
steps = self.jobs['build-dev']['steps']
384+
build = next(step for step in steps if step.get('uses') == './.github/actions/docker-build')
385+
self.assertTrue(build['with']['tags'].endswith(':${{ github.sha }}-dev'))
386+
self.assertNotIn('promote-app-image.sh', json.dumps(steps))
387+
self.assertNotIn('imagetools create', json.dumps(steps))
388+
389+
def test_task_promotion_requires_a_successful_fresh_app_release(self):
390+
for branch, job, upload in (('main', 'promote-trigger', 'deploy-trigger'),
391+
('staging', 'promote-trigger', 'deploy-trigger'),
392+
('dev', 'promote-trigger-dev', 'deploy-trigger-dev')):
393+
ready = {'promote-images': 'success', upload: 'success'}
394+
self.assertIn('promote-images', self.jobs[job]['needs'])
395+
self.assertTrue(self.eligible(job, branch, ready))
396+
self.assertFalse(self.eligible(job, branch, ready, promoted='false'))
397+
self.assertFalse(self.eligible(job, branch, {**ready, 'promote-images': 'failure'}))
398+
steps = self.jobs[job]['steps']
399+
wait = next(i for i, step in enumerate(steps) if 'wait-for-ecs-cutover.sh' in step.get('run', ''))
400+
promote = next(i for i, step in enumerate(steps) if 'promote "$VERSION"' in step.get('run', ''))
401+
self.assertLess(wait, promote)
402+
self.assertEqual(steps[promote]['env']['VERSION'], '${{ needs.' + upload + '.outputs.version }}')
403+
404+
def test_permission_check_and_other_images_precede_app_rollout(self):
405+
steps = self.jobs['promote-images']['steps']
406+
preflight = next(i for i, step in enumerate(steps) if 'get-pipeline-state' in step.get('run', ''))
407+
retag = next(i for i, step in enumerate(steps) if step.get('id') == 'promote')
408+
self.assertLess(preflight, retag)
409+
self.assertTrue(steps[retag]['env']['ECR_REPOS'].strip().endswith('${{ secrets.ECR_APP }}'))
410+
411+
263412
if __name__ == '__main__':
264413
unittest.main()

‎.github/scripts/wait-for-ecs-cutover.sh‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,39 @@ while [ -z "$DEPLOYMENT_ID" ] || [ "$DEPLOYMENT_ID" = 'None' ]; do
8181
InProgress|Succeeded) ;;
8282
*) log "ERROR: unexpected pipeline status: $status"; exit 1 ;;
8383
esac
84-
DEPLOYMENT_ID=$(aws_read codepipeline list-action-executions \
84+
# Action history does not publish the external deployment ID until cleanup
85+
# finishes. Live state exposes it while traffic is shifting. Correlate both
86+
# the stage execution and action attempt so old state cannot satisfy this run.
87+
deploy_state=$(aws_read codepipeline get-pipeline-state --name "$PIPELINE" \
88+
--query "stageStates[?stageName=='Deploy'] | [0]" --output json)
89+
deploy_actions=$(aws_read codepipeline list-action-executions \
8590
--pipeline-name "$PIPELINE" --filter pipelineExecutionId="$EXECUTION_ID" \
86-
--query "actionExecutionDetails[?stageName=='Deploy'].output.executionResult.externalExecutionId | [0]" \
87-
--output text)
91+
--query "actionExecutionDetails[?stageName=='Deploy']" --output json)
92+
DEPLOYMENT_ID=$(printf '%s\n' "$deploy_actions" | DEPLOY_STATE="$deploy_state" EXECUTION_ID="$EXECUTION_ID" python3 -c '
93+
import json, os, re, sys
94+
state = json.loads(os.environ["DEPLOY_STATE"])
95+
actions = json.load(sys.stdin)
96+
if not state or state.get("latestExecution", {}).get("pipelineExecutionId") != os.environ["EXECUTION_ID"] or not actions:
97+
print("")
98+
sys.exit(0)
99+
if len({a["actionName"] for a in actions}) != 1:
100+
raise SystemExit("ERROR: expected one Deploy action in the app pipeline")
101+
latest = max(actions, key=lambda a: a["startTime"])
102+
matches = [a["latestExecution"] for a in state.get("actionStates", [])
103+
if a["actionName"] == latest["actionName"]
104+
and a.get("latestExecution", {}).get("actionExecutionId") == latest["actionExecutionId"]]
105+
if len(matches) > 1:
106+
raise SystemExit("ERROR: ambiguous live Deploy action")
107+
if not matches:
108+
print("")
109+
sys.exit(0)
110+
if latest["status"] not in ("InProgress", "Succeeded"):
111+
raise SystemExit("ERROR: Deploy action ended in " + latest["status"])
112+
deployment_id = matches[0].get("externalExecutionId", "")
113+
if deployment_id and not re.fullmatch(r"d-[A-Za-z0-9]+", deployment_id):
114+
raise SystemExit("ERROR: invalid CodeDeploy deployment ID in pipeline state")
115+
print(deployment_id)
116+
')
88117
if [ -z "$DEPLOYMENT_ID" ] || [ "$DEPLOYMENT_ID" = 'None' ]; then
89118
if [ "$status" = 'Succeeded' ]; then
90119
log 'ERROR: successful pipeline has no CodeDeploy deployment'; exit 1

0 commit comments

Comments
 (0)