diff --git a/lua/copy_build.lua b/lua/copy_build.lua index 78332c9..67ee33a 100644 --- a/lua/copy_build.lua +++ b/lua/copy_build.lua @@ -14,10 +14,10 @@ if res.status ~= 200 then return end --- Check that build_tgt is empty, or allow resuming a partial promote. +-- Check that build_tgt is empty, or allow resuming a partial/complete promote. +-- A fully promoted target (.final_status present) is treated as success (idempotent). -- A partial target (files present but no .final_status) is resumed by skipping --- already-copied objects. A fully promoted target (.final_status present) is --- rejected immediately. +-- already-copied objects. -- ngx.say("Checking if the target reference '" .. build_tgt .. "' is empty") ngx.flush(true) @@ -32,7 +32,8 @@ else "/force_real_request/download/" .. build_tgt .. "/.final_status" ) if fs_res.status == 200 then - ngx.say('FAILED: target already fully promoted') + ngx.say('Target already fully promoted, nothing to do') + ngx.say('BUILD COPIED') ngx.flush(true) return end @@ -208,6 +209,18 @@ if next(target_files) ~= nil then objects = remaining end +-- Remove .final_status from the main copy list so it can be written last. +-- Its presence on the target is the completion seal: if a promote is interrupted +-- after some files are copied but before this final write, the target will not +-- have .final_status and can be safely resumed. +local main_objects = {} +for _, obj in ipairs(objects) do + if obj ~= '.final_status' then + table.insert(main_objects, obj) + end +end +objects = main_objects + local total_number_of_objects = #objects local batch_size = 16 local current_object = 0 @@ -314,4 +327,17 @@ else end +-- Copy .final_status last as the completion seal. +ngx.say("Copying .final_status as completion seal") +ngx.flush(true) +local fs_copy_res = ngx.location.capture( + "/force_real_request/copy/" .. build_src .. "/" .. build_tgt .. "/.final_status", + { method = ngx.HTTP_PUT, body = "" } +) +if fs_copy_res.status ~= 200 then + ngx.say('FAILED: could not copy .final_status') + ngx.flush(true) + return +end + ngx.say("BUILD COPIED") diff --git a/tests/end2end/test_copy.py b/tests/end2end/test_copy.py index 68f3327..044ac40 100644 --- a/tests/end2end/test_copy.py +++ b/tests/end2end/test_copy.py @@ -50,25 +50,19 @@ def test_copy_source_and_target_listings_are_identical( assert src.content == tgt.content -def test_copy_fails_when_target_already_exists( +def test_copy_idempotent_when_target_already_fully_promoted( session, artifacts_url, upload_file, finish_build ): - """A second copy to a fully-promoted target is rejected.""" + """A second copy to a fully-promoted target succeeds (idempotent).""" upload_file(STAGING_BUILD, 'file.txt', b'data') finish_build(STAGING_BUILD) session.get(f'{artifacts_url}/copy/{STAGING_BUILD}/{COPY_BUILD}/') - # Second attempt — target is fully promoted (has .final_status) + # Second attempt — target is fully promoted (has .final_status): should succeed. resp = session.get(f'{artifacts_url}/copy/{STAGING_BUILD}/{COPY_BUILD}/') assert resp.status_code == 200 - lines = resp.content.splitlines() - expected_check_line = ( - b"Checking if the target reference '%b' is empty" - % COPY_BUILD.encode() - ) - assert lines[-2] == expected_check_line - assert lines[-1] == b'FAILED: target already fully promoted' + assert resp.content.splitlines()[-1] == b'BUILD COPIED' def test_copy_promotes_staging_to_promoted_bucket( @@ -155,10 +149,11 @@ def test_resume_partial_promote( def test_resume_already_complete_promote( session, artifacts_url, upload_file, finish_build ): - """Resuming a fully-promoted target is rejected with a clear error. + """Resuming a fully-promoted target succeeds (idempotent). A target that already has a .final_status is considered complete; a second - promote attempt must fail rather than silently overwriting it. + promote attempt must succeed so that retrying a failed workflow does not + block on this step. """ upload_file(STAGING_BUILD, 'file.txt', b'data') finish_build(STAGING_BUILD) @@ -166,10 +161,35 @@ def test_resume_already_complete_promote( # First promote — completes successfully. session.get(f'{artifacts_url}/copy/{STAGING_BUILD}/{COPY_BUILD}/') - # Second attempt — target is fully promoted. + # Second attempt — target is fully promoted: should succeed (idempotent). + resp = session.get(f'{artifacts_url}/copy/{STAGING_BUILD}/{COPY_BUILD}/') + assert resp.status_code == 200 + assert resp.content.splitlines()[-1] == b'BUILD COPIED' + + +def test_resume_promote_interrupted_before_final_status( + session, artifacts_url, upload_file, finish_build +): + """Promote resumes when interrupted just before .final_status was written. + + .final_status is copied last and acts as the completion seal. A target that + has all artifacts but no .final_status (promote crashed at the final step) + must be resumable rather than treated as complete. + """ + n = 3 + for i in range(n): + upload_file(STAGING_BUILD, f'obj-{i}', f'content-{i}'.encode()) + finish_build(STAGING_BUILD) + + # Pre-populate target with all artifacts but without .final_status. + for i in range(n): + upload_file(COPY_BUILD, f'obj-{i}', f'content-{i}'.encode()) + resp = session.get(f'{artifacts_url}/copy/{STAGING_BUILD}/{COPY_BUILD}/') assert resp.status_code == 200 - assert resp.content.splitlines()[-1] == b'FAILED: target already fully promoted' + assert resp.content.splitlines()[-1] == b'BUILD COPIED' + + assert session.get(f'{artifacts_url}/download/{COPY_BUILD}/.final_status').status_code == 200 def test_copy_behind_ingress(session, artifacts_url, upload_file, finish_build):