diff --git a/src/server/db/migrations/033_add_after_build_completion_key.ts b/src/server/db/migrations/033_add_after_build_completion_key.ts new file mode 100644 index 0000000..5701c92 --- /dev/null +++ b/src/server/db/migrations/033_add_after_build_completion_key.ts @@ -0,0 +1,33 @@ +/** + * Copyright 2026 GoodRx, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import { Knex } from 'knex'; + +export async function up(knex: Knex): Promise { + if (await knex.schema.hasColumn('deploys', 'afterBuildCompletionKey')) return; + + await knex.schema.alterTable('deploys', (table) => { + table.text('afterBuildCompletionKey').nullable(); + }); +} + +export async function down(knex: Knex): Promise { + if (!(await knex.schema.hasColumn('deploys', 'afterBuildCompletionKey'))) return; + + await knex.schema.alterTable('deploys', (table) => { + table.dropColumn('afterBuildCompletionKey'); + }); +} diff --git a/src/server/models/Deploy.ts b/src/server/models/Deploy.ts index cfca9e3..df729d4 100644 --- a/src/server/models/Deploy.ts +++ b/src/server/models/Deploy.ts @@ -62,6 +62,7 @@ export default class Deploy extends Model { buildOutput: string; deployOutput: string; buildJobName: string; + afterBuildCompletionKey: string | null; manifest: string; devMode: boolean; devModeSessionId: number | null; diff --git a/src/server/services/__tests__/build.test.ts b/src/server/services/__tests__/build.test.ts index 1a63899..48ce891 100644 --- a/src/server/services/__tests__/build.test.ts +++ b/src/server/services/__tests__/build.test.ts @@ -1239,76 +1239,43 @@ describe('BuildService deployment reconciliation', () => { mockAcceptDeploymentIntent.mockResolvedValue({ accepted: true, generation: 1, scopeKey: 'all' }); }); - test.each([ - ['a newer live PR generation', createBuild({ desiredGeneration: 8, runUUID: 'run-c', pullRequestId: 11 }), 7, true], - [ - 'a newer live static generation', - createBuild({ - desiredGeneration: 8, - runUUID: 'run-c', - pullRequest: null, - pullRequestId: null, - deployEnabled: true, - isStatic: true, - }), - 7, - true, - ], - ['the same generation', createBuild({ desiredGeneration: 7, runUUID: 'other-run' }), 7, false], - [ - 'a closed PR', - createBuild({ - desiredGeneration: 8, - runUUID: 'run-c', - pullRequestId: 11, - pullRequest: { status: 'closed', deployOnUpdate: true }, - }), - 7, - false, - ], - [ - 'a deploy-disabled PR', - createBuild({ - desiredGeneration: 8, - runUUID: 'run-c', - pullRequestId: 11, - pullRequest: { status: 'open', deployOnUpdate: false }, - }), - 7, - false, - ], - [ - 'an API teardown', - createBuild({ - desiredGeneration: 8, - runUUID: 'build-teardown-1', - status: BuildStatus.TEARING_DOWN, - pullRequest: null, - pullRequestId: null, - deployEnabled: false, - }), - 7, - false, - ], - [ - 'a PR teardown owner before status publication', - createBuild({ - desiredGeneration: 8, - runUUID: 'build-teardown-1', - pullRequestId: 11, - }), - 7, - false, - ], - ['a deleted Build', createBuild({ desiredGeneration: 8, deletedAt: '2026-08-06T00:00:00Z' }), 7, false], - ['a stale run without a generation', createBuild({ desiredGeneration: 8 }), undefined, false], - ])('permits stale after-build only for %s', async (_case, build, expectedGeneration, expected) => { + const deploymentScopeHarness = (buildImagesResult: boolean) => { const { service } = serviceHarness(); - jest.spyOn(service as any, 'loadBuildDeploymentAuthority').mockResolvedValue(build); + const build = createBuild({ id: 1, runUUID: 'run-current', namespace: 'env-sample' }); + jest.spyOn(service as any, 'isDeploymentRunCurrent').mockResolvedValue(true); + const buildImages = jest.spyOn(service as any, 'buildImages').mockResolvedValue(buildImagesResult); + jest.spyOn(service as any, 'deployCLIServices').mockResolvedValue(true); + const updateStatus = jest.spyOn(service as any, 'updateStatusAndComment').mockResolvedValue(undefined); + const applyManifests = jest.spyOn(service as any, 'generateAndApplyManifests').mockResolvedValue(true); + const preparation = { + build, + runUUID: 'run-current', + githubRepositoryId: 100, + sourceGithubRepositoryId: 100, + sourceRef: 'commit-a', + sourceBranch: 'main', + }; + return { service, preparation, buildImages, updateStatus, applyManifests }; + }; - await expect( - service.isSupersededByNewerLiveDeploymentGeneration(1, expectedGeneration as number | undefined) - ).resolves.toBe(expected); + test('a failed image phase reports ERROR and never reaches manifest apply', async () => { + const { service, preparation, buildImages, updateStatus, applyManifests } = deploymentScopeHarness(false); + + const result = await (service as any).executeDeploymentScope(preparation, 7); + + expect(result).toEqual({ status: BuildStatus.ERROR }); + expect(buildImages).toHaveBeenCalledTimes(1); + expect(updateStatus).not.toHaveBeenCalled(); + expect(applyManifests).not.toHaveBeenCalled(); + }); + + test('a successful image phase proceeds to manifest apply and reports DEPLOYED', async () => { + const { service, preparation, applyManifests } = deploymentScopeHarness(true); + + const result = await (service as any).executeDeploymentScope(preparation, 7); + + expect(result).toEqual({ status: BuildStatus.DEPLOYED }); + expect(applyManifests).toHaveBeenCalledTimes(1); }); test('signals only the exact durable generation accepted by the mailbox', async () => { diff --git a/src/server/services/__tests__/deploy.test.ts b/src/server/services/__tests__/deploy.test.ts index a46afc9..f10a5a0 100644 --- a/src/server/services/__tests__/deploy.test.ts +++ b/src/server/services/__tests__/deploy.test.ts @@ -15,6 +15,7 @@ */ import mockRedisClient from 'server/lib/__mocks__/redisClientMock'; +import hash from 'object-hash'; import DeployService from '../deploy'; import { DeployStatus, DeployTypes } from 'shared/constants'; import { ChartType } from 'server/lib/nativeHelm'; @@ -35,7 +36,6 @@ const mockBuildWithNative = jest.fn(); const mockGlobalConfigGetAllConfigs = jest.fn(); const mockGlobalConfigGetOrgChartName = jest.fn(); const mockCreateOrUpdateNamespace = jest.fn(); -const mockIsSupersededByNewerLiveDeploymentGeneration = jest.fn(); const mockLoggerInfo = jest.fn(); const mockLoggerWarn = jest.fn(); const mockGetLogger = jest.fn(() => ({ @@ -136,8 +136,6 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { mockCodefreshWaitForImage.mockReset(); mockBuildWithNative.mockReset(); mockCreateOrUpdateNamespace.mockReset(); - mockIsSupersededByNewerLiveDeploymentGeneration.mockReset(); - mockIsSupersededByNewerLiveDeploymentGeneration.mockResolvedValue(false); mockGlobalConfigGetOrgChartName.mockResolvedValue('org-chart'); mockGlobalConfigGetAllConfigs.mockResolvedValue({ lifecycleDefaults: { @@ -163,12 +161,7 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { query: jest.fn(() => ({ where: conditionalDeployWhere, findOne: currentDeployFindOne })), }, }, - services: { - BuildService: { - isSupersededByNewerLiveDeploymentGeneration: (...args: any[]) => - mockIsSupersededByNewerLiveDeploymentGeneration(...args), - }, - }, + services: {}, }; mockRedis = {}; @@ -935,6 +928,20 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { }) ); expect(mockCodefreshWaitForImage).toHaveBeenCalledWith('after-build-run'); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + status: DeployStatus.BUILDING, + statusMessage: 'Running after-build pipeline...', + buildLogs: null, + }); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + buildLogs: 'https://g.codefresh.io/build/after-build-run', + }); + const runningPatchIndex = conditionalDeployPatch.mock.calls.findIndex( + ([params]) => params.buildLogs === 'https://g.codefresh.io/build/after-build-run' + ); + expect(conditionalDeployPatch.mock.invocationCallOrder[runningPatchIndex]).toBeLessThan( + mockCodefreshWaitForImage.mock.invocationCallOrder[0] + ); expect(mockGetLogger).toHaveBeenCalledWith( expect.objectContaining({ afterBuildPipelineId: 'sample/after-build', @@ -949,11 +956,10 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { ); }); - test('a superseded native build still invokes its after-build pipeline without publishing stale image state', async () => { + test('a superseded native build publishes nothing and does not trigger its after-build pipeline', async () => { let current = true; const deploy = createNativeAfterBuildDeploy(); const patchTagSpy = prepareNativeAfterBuildTest(() => current); - mockIsSupersededByNewerLiveDeploymentGeneration.mockResolvedValue(true); mockBuildWithNative.mockImplementation(async () => { current = false; return { success: true, logs: 'native build completed' }; @@ -969,31 +975,25 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { ); expect(result).toBe(true); - expect(mockCodefreshTriggerPipeline).toHaveBeenCalledTimes(1); - expect(mockIsSupersededByNewerLiveDeploymentGeneration).toHaveBeenCalledWith(91, 7); + expect(mockCodefreshTriggerPipeline).not.toHaveBeenCalled(); expect(mockCodefreshWaitForImage).not.toHaveBeenCalled(); expect(patchTagSpy).not.toHaveBeenCalled(); - expect(conditionalDeployPatch).not.toHaveBeenCalled(); - expect(mockGetLogger).toHaveBeenCalledWith( - expect.objectContaining({ - afterBuildPipelineId: 'sample/after-build', - pipelineId: 'after-build-run', - }) - ); + expect(conditionalDeployPatch).toHaveBeenCalledTimes(1); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ afterBuildCompletionKey: null, buildLogs: null }); expect(mockLoggerInfo).toHaveBeenCalledWith( - expect.stringMatching( - /^Codefresh: after-build pipeline detached reason=superseded afterBuildPipelineId=sample\/after-build pipelineId=after-build-run imageTag=/ - ) + 'Image: native result publication skipped reason=superseded result=success' ); }); - test('a native build superseded by teardown does not invoke its after-build pipeline', async () => { + test('a run superseded during its after-build trigger still awaits the pipeline result', async () => { let current = true; const deploy = createNativeAfterBuildDeploy(); const patchTagSpy = prepareNativeAfterBuildTest(() => current); - mockBuildWithNative.mockImplementation(async () => { + mockBuildWithNative.mockResolvedValue({ success: true }); + conditionalDeployPatch.mockImplementation(async () => (current ? 1 : 0)); + mockCodefreshTriggerPipeline.mockImplementation(async () => { current = false; - return { success: true, logs: 'native build completed' }; + return 'after-build-run'; }); const result = await deployService.buildImageForHelmAndGithub( @@ -1006,26 +1006,24 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { ); expect(result).toBe(true); - expect(mockIsSupersededByNewerLiveDeploymentGeneration).toHaveBeenCalledWith(91, 7); - expect(mockCodefreshTriggerPipeline).not.toHaveBeenCalled(); - expect(mockCodefreshWaitForImage).not.toHaveBeenCalled(); + expect(mockCodefreshTriggerPipeline).toHaveBeenCalledTimes(1); + expect(mockCodefreshWaitForImage).toHaveBeenCalledWith('after-build-run'); expect(patchTagSpy).not.toHaveBeenCalled(); - expect(conditionalDeployPatch).not.toHaveBeenCalled(); - expect(mockLoggerInfo).toHaveBeenCalledWith( - expect.stringMatching( - /^Codefresh: after-build pipeline skipped reason=no_live_successor afterBuildPipelineId=sample\/after-build imageTag=/ - ) - ); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + buildLogs: 'https://g.codefresh.io/build/after-build-run', + }); + expect( + conditionalDeployWhere.mock.calls.every(([where]) => where.id === deploy.id && where.runUUID === 'run-a') + ).toBe(true); }); - test('a stale native build fails closed when live-successor authority cannot be read', async () => { + test('a failed superseded native build neither invokes the after-build pipeline nor publishes failure', async () => { let current = true; const deploy = createNativeAfterBuildDeploy(); - prepareNativeAfterBuildTest(() => current); - mockIsSupersededByNewerLiveDeploymentGeneration.mockRejectedValue(new Error('database unavailable')); + const patchTagSpy = prepareNativeAfterBuildTest(() => current); mockBuildWithNative.mockImplementation(async () => { current = false; - return { success: true }; + return { success: false, logs: 'native build failed' }; }); const result = await deployService.buildImageForHelmAndGithub( @@ -1040,46 +1038,165 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { expect(result).toBe(true); expect(mockCodefreshTriggerPipeline).not.toHaveBeenCalled(); expect(mockCodefreshWaitForImage).not.toHaveBeenCalled(); - expect(mockLoggerWarn).toHaveBeenCalledWith('Codefresh: after-build successor check failed'); + expect(patchTagSpy).not.toHaveBeenCalled(); + expect(deployService.patchAndUpdateActivityFeed).not.toHaveBeenCalledWith( + deploy, + { status: DeployStatus.BUILD_FAILED }, + 'run-a' + ); }); - test('a native after-build pipeline is left detached when superseded during its trigger', async () => { - let current = true; + test('a current native after-build pipeline failure still fails the image phase', async () => { const deploy = createNativeAfterBuildDeploy(); - const patchTagSpy = prepareNativeAfterBuildTest(() => current); + const patchTagSpy = prepareNativeAfterBuildTest(() => true); mockBuildWithNative.mockResolvedValue({ success: true }); - mockCodefreshTriggerPipeline.mockImplementation(async () => { - current = false; - return 'after-build-run'; - }); + mockCodefreshWaitForImage.mockResolvedValue(false); const result = await deployService.buildImageForHelmAndGithub( deploy as any, - 'run-a', + 'run-1', undefined, undefined, undefined, 7 ); - expect(result).toBe(true); - expect(patchTagSpy).toHaveBeenCalledTimes(1); + expect(result).toBe(false); + expect(patchTagSpy).not.toHaveBeenCalled(); + expect(deployService.patchAndUpdateActivityFeed).toHaveBeenCalledWith( + deploy, + { + status: DeployStatus.BUILD_FAILED, + statusMessage: 'After-build pipeline failed.', + }, + 'run-1' + ); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + buildLogs: 'https://g.codefresh.io/build/after-build-run', + }); expect(mockCodefreshTriggerPipeline).toHaveBeenCalledTimes(1); + expect(mockCodefreshWaitForImage).toHaveBeenCalledWith('after-build-run'); + expect(mockGetLogger).toHaveBeenCalledWith( + expect.objectContaining({ + afterBuildPipelineId: 'sample/after-build', + pipelineId: 'after-build-run', + }) + ); + expect(mockLoggerWarn).toHaveBeenCalledWith( + expect.stringMatching( + /^Codefresh: after-build pipeline completed result=failure afterBuildPipelineId=sample\/after-build pipelineId=after-build-run imageTag=/ + ) + ); + }); + + test('a current after-build trigger error publishes a terminal after-build failure', async () => { + const deploy = createNativeAfterBuildDeploy(); + const patchTagSpy = prepareNativeAfterBuildTest(() => true); + mockBuildWithNative.mockResolvedValue({ success: true }); + mockCodefreshTriggerPipeline.mockRejectedValue(new Error('Codefresh unavailable')); + + const result = await deployService.buildImageForHelmAndGithub( + deploy as any, + 'run-1', + undefined, + undefined, + undefined, + 7 + ); + + expect(result).toBe(false); expect(mockCodefreshWaitForImage).not.toHaveBeenCalled(); + expect(patchTagSpy).not.toHaveBeenCalled(); + expect(deployService.patchAndUpdateActivityFeed).toHaveBeenCalledWith( + deploy, + { status: DeployStatus.BUILD_FAILED, statusMessage: 'After-build pipeline failed.' }, + 'run-1' + ); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + status: DeployStatus.BUILDING, + statusMessage: 'Running after-build pipeline...', + buildLogs: null, + }); + expect(conditionalDeployPatch).not.toHaveBeenCalledWith( + expect.objectContaining({ buildLogs: expect.stringContaining('g.codefresh.io/build/') }) + ); + expect(mockLoggerWarn).toHaveBeenCalledWith('Codefresh: after-build pipeline trigger failed'); }); - test('a failed superseded native build neither invokes the after-build pipeline nor publishes failure', async () => { - let current = true; + const flushUntil = async (done: () => boolean) => { + for (let i = 0; i < 20 && !done(); i++) await new Promise((resolve) => setImmediate(resolve)); + }; + + test('a run that finds the image already pushed still runs and awaits the after-build pipeline before BUILT', async () => { const deploy = createNativeAfterBuildDeploy(); - const patchTagSpy = prepareNativeAfterBuildTest(() => current); - mockBuildWithNative.mockImplementation(async () => { - current = false; - return { success: false, logs: 'native build failed' }; + const patchTagSpy = prepareNativeAfterBuildTest(() => true); + mockCodefreshTagExists.mockResolvedValue(true); + const pipelineResult: { resolve?: (completed: boolean) => void } = {}; + mockCodefreshWaitForImage.mockImplementation( + () => + new Promise((resolve) => { + pipelineResult.resolve = resolve; + }) + ); + + const resultPromise = deployService.buildImageForHelmAndGithub( + deploy as any, + 'run-b', + undefined, + undefined, + undefined, + 7 + ); + await flushUntil(() => mockCodefreshWaitForImage.mock.calls.length > 0); + + expect(mockBuildWithNative).not.toHaveBeenCalled(); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + status: DeployStatus.BUILDING, + statusMessage: 'Running after-build pipeline...', + buildLogs: null, }); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + buildLogs: 'https://g.codefresh.io/build/after-build-run', + }); + const afterBuildUrlPatchIndex = conditionalDeployPatch.mock.calls.findIndex( + ([params]) => params.buildLogs === 'https://g.codefresh.io/build/after-build-run' + ); + expect(conditionalDeployPatch.mock.invocationCallOrder[afterBuildUrlPatchIndex]).toBeLessThan( + mockCodefreshWaitForImage.mock.invocationCallOrder[0] + ); + expect(mockCodefreshTriggerPipeline).toHaveBeenCalledTimes(1); + expect(mockCodefreshTriggerPipeline).toHaveBeenCalledWith( + 'sample/after-build', + 'cli', + expect.objectContaining({ + FEATURE_FLAG: 'enabled', + TAG: expect.stringContaining('/sample/app-images:lfc-abcdef1-'), + branch: 'feature-branch', + }) + ); + expect(mockCodefreshWaitForImage).toHaveBeenCalledWith('after-build-run'); + expect(patchTagSpy).not.toHaveBeenCalled(); + + pipelineResult.resolve!(true); + await expect(resultPromise).resolves.toBe(true); + expect(patchTagSpy).toHaveBeenCalledTimes(1); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + afterBuildCompletionKey: expect.stringMatching( + /^sample\/after-build@.*\/sample\/app-images:lfc-abcdef1-[^@]+$/ + ), + }); + }); + + test('a matching completion record skips the after-build on the existing-image path', async () => { + const deploy = createNativeAfterBuildDeploy() as any; + const patchTagSpy = prepareNativeAfterBuildTest(() => true); + mockCodefreshTagExists.mockResolvedValue(true); + const envVarsHash = hash({ FEATURE_FLAG: 'enabled' }); + deploy.afterBuildCompletionKey = `sample/after-build@123456789012.dkr.ecr.us-west-2.amazonaws.com/sample/app-images:lfc-abcdef1-${envVarsHash}`; const result = await deployService.buildImageForHelmAndGithub( - deploy as any, - 'run-a', + deploy, + 'run-b', undefined, undefined, undefined, @@ -1089,23 +1206,116 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { expect(result).toBe(true); expect(mockCodefreshTriggerPipeline).not.toHaveBeenCalled(); expect(mockCodefreshWaitForImage).not.toHaveBeenCalled(); - expect(patchTagSpy).not.toHaveBeenCalled(); - expect(deployService.patchAndUpdateActivityFeed).not.toHaveBeenCalledWith( + expect(patchTagSpy).toHaveBeenCalledTimes(1); + expect(conditionalDeployPatch).not.toHaveBeenCalledWith(expect.objectContaining({ buildLogs: null })); + }); + + test('rebuilding an image clears the previous completion and build link before building', async () => { + const deploy = createNativeAfterBuildDeploy() as any; + deploy.afterBuildCompletionKey = 'sample/after-build@stale-tag'; + const patchTagSpy = prepareNativeAfterBuildTest(() => true); + mockBuildWithNative.mockResolvedValue({ success: true }); + + const result = await deployService.buildImageForHelmAndGithub( deploy, - { status: DeployStatus.BUILD_FAILED }, - 'run-a' + 'run-1', + undefined, + undefined, + undefined, + 7 ); + + expect(result).toBe(true); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ afterBuildCompletionKey: null, buildLogs: null }); + const rebuildClearIndex = conditionalDeployPatch.mock.calls.findIndex( + ([params]) => params.afterBuildCompletionKey === null && params.buildLogs === null + ); + expect(conditionalDeployPatch.mock.invocationCallOrder[rebuildClearIndex]).toBeLessThan( + mockBuildWithNative.mock.invocationCallOrder[0] + ); + expect(patchTagSpy).toHaveBeenCalledTimes(1); }); - test('a current native after-build pipeline failure still fails the image phase', async () => { + test('a rebuild that cannot clear the completion record does not build', async () => { + const deploy = createNativeAfterBuildDeploy() as any; + deploy.afterBuildCompletionKey = 'sample/after-build@stale-tag'; + prepareNativeAfterBuildTest(() => true); + conditionalDeployPatch.mockResolvedValueOnce(0); + + const result = await deployService.buildImageForHelmAndGithub( + deploy, + 'run-1', + undefined, + undefined, + undefined, + 7 + ); + + expect(result).toBe(true); + expect(mockBuildWithNative).not.toHaveBeenCalled(); + expect(mockCodefreshBuildImage).not.toHaveBeenCalled(); + expect(mockCodefreshTriggerPipeline).not.toHaveBeenCalled(); + }); + + test('a codefresh-engine cold build records completion when its embedded after-build is not detached', async () => { + const deploy = createNativeAfterBuildDeploy() as any; + deploy.deployable.builder = { engine: 'ci' }; + const patchTagSpy = prepareNativeAfterBuildTest(() => true); + mockCodefreshBuildImage.mockResolvedValue('codefresh-build-123'); + mockCodefreshGetLogs.mockResolvedValue('codefresh logs'); + + const result = await deployService.buildImageForHelmAndGithub( + deploy, + 'run-1', + undefined, + undefined, + undefined, + 7 + ); + + expect(result).toBe(true); + expect(mockBuildWithNative).not.toHaveBeenCalled(); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + afterBuildCompletionKey: expect.stringMatching( + /^sample\/after-build@.*\/sample\/app-images:lfc-abcdef1-[^@]+$/ + ), + }); + expect(patchTagSpy).toHaveBeenCalledTimes(1); + }); + + test('a detached codefresh-engine cold build does not record completion', async () => { + const deploy = createNativeAfterBuildDeploy() as any; + deploy.deployable.builder = { engine: 'ci' }; + deploy.deployable.detatchAfterBuildPipeline = true; + const patchTagSpy = prepareNativeAfterBuildTest(() => true); + mockCodefreshBuildImage.mockResolvedValue('codefresh-build-123'); + mockCodefreshGetLogs.mockResolvedValue('codefresh logs'); + + const result = await deployService.buildImageForHelmAndGithub( + deploy, + 'run-1', + undefined, + undefined, + undefined, + 7 + ); + + expect(result).toBe(true); + expect(conditionalDeployPatch).not.toHaveBeenCalledWith( + expect.objectContaining({ afterBuildCompletionKey: expect.any(String) }) + ); + expect(patchTagSpy).toHaveBeenCalledTimes(1); + }); + + test('a failed after-build on the existing-image path never publishes BUILT', async () => { const deploy = createNativeAfterBuildDeploy(); const patchTagSpy = prepareNativeAfterBuildTest(() => true); - mockBuildWithNative.mockResolvedValue({ success: true }); + mockCodefreshTagExists.mockResolvedValue(true); mockCodefreshWaitForImage.mockResolvedValue(false); const result = await deployService.buildImageForHelmAndGithub( deploy as any, - 'run-1', + 'run-b', undefined, undefined, undefined, @@ -1113,19 +1323,101 @@ describe('DeployService - shouldTriggerGithubDeployment', () => { ); expect(result).toBe(false); - expect(patchTagSpy).toHaveBeenCalledTimes(1); - expect(mockCodefreshTriggerPipeline).toHaveBeenCalledTimes(1); - expect(mockCodefreshWaitForImage).toHaveBeenCalledWith('after-build-run'); - expect(mockGetLogger).toHaveBeenCalledWith( + expect(patchTagSpy).not.toHaveBeenCalled(); + expect(deployService.patchAndUpdateActivityFeed).toHaveBeenCalledWith( + deploy, + { + status: DeployStatus.BUILD_FAILED, + statusMessage: 'After-build pipeline failed.', + }, + 'run-b' + ); + expect(conditionalDeployPatch).toHaveBeenCalledWith({ + buildLogs: 'https://g.codefresh.io/build/after-build-run', + }); + }); + + test('a codefresh-engine cache hit still runs and awaits the after-build pipeline before BUILT', async () => { + const deploy = createNativeAfterBuildDeploy() as any; + deploy.deployable.builder = { engine: 'ci' }; + const patchTagSpy = prepareNativeAfterBuildTest(() => true); + mockCodefreshTagExists.mockResolvedValue(true); + const pipelineResult: { resolve?: (completed: boolean) => void } = {}; + mockCodefreshWaitForImage.mockImplementation( + () => + new Promise((resolve) => { + pipelineResult.resolve = resolve; + }) + ); + + const resultPromise = deployService.buildImageForHelmAndGithub( + deploy, + 'run-b', + undefined, + undefined, + undefined, + 7 + ); + await flushUntil(() => mockCodefreshWaitForImage.mock.calls.length > 0); + + expect(mockBuildWithNative).not.toHaveBeenCalled(); + expect(mockCodefreshBuildImage).not.toHaveBeenCalled(); + expect(mockCodefreshTriggerPipeline).toHaveBeenCalledWith( + 'sample/after-build', + 'cli', expect.objectContaining({ - afterBuildPipelineId: 'sample/after-build', - pipelineId: 'after-build-run', + TAG: expect.stringContaining('/sample/app-images:lfc-abcdef1-'), + SOURCE_REVISION: 'abcdef1234567890', + SOURCE_BRANCH: 'feature-branch', }) ); - expect(mockLoggerWarn).toHaveBeenCalledWith( - expect.stringMatching( - /^Codefresh: after-build pipeline completed result=failure afterBuildPipelineId=sample\/after-build pipelineId=after-build-run imageTag=/ - ) + expect(patchTagSpy).not.toHaveBeenCalled(); + + pipelineResult.resolve!(true); + await expect(resultPromise).resolves.toBe(true); + expect(patchTagSpy).toHaveBeenCalledTimes(1); + }); + + test('a run superseded during the existing-image after-build wait publishes neither BUILT nor BUILD_FAILED', async () => { + let current = true; + const deploy = createNativeAfterBuildDeploy(); + const patchTagSpy = prepareNativeAfterBuildTest(() => current); + mockCodefreshTagExists.mockResolvedValue(true); + const pipelineResult: { resolve?: (completed: boolean) => void } = {}; + mockCodefreshWaitForImage.mockImplementation( + () => + new Promise((resolve) => { + pipelineResult.resolve = resolve; + }) + ); + + const resultPromise = deployService.buildImageForHelmAndGithub( + deploy as any, + 'run-b', + undefined, + undefined, + undefined, + 7 + ); + await flushUntil(() => mockCodefreshWaitForImage.mock.calls.length > 0); + + current = false; + pipelineResult.resolve!(true); + + await expect(resultPromise).resolves.toBe(true); + expect(patchTagSpy).not.toHaveBeenCalled(); + expect(deployService.patchAndUpdateActivityFeed).not.toHaveBeenCalledWith( + deploy, + { status: DeployStatus.BUILT }, + 'run-b' + ); + expect(deployService.patchAndUpdateActivityFeed).not.toHaveBeenCalledWith( + deploy, + { status: DeployStatus.BUILD_FAILED }, + 'run-b' + ); + expect(mockLoggerInfo).toHaveBeenCalledWith( + 'Image: after-build publication skipped reason=superseded result=success' ); }); diff --git a/src/server/services/build.ts b/src/server/services/build.ts index 69637e6..b0a335d 100644 --- a/src/server/services/build.ts +++ b/src/server/services/build.ts @@ -2616,26 +2616,6 @@ export default class BuildService extends BaseService { return build ?? null; } - /** - * Prove that stale work lost authority to a newer live deployment intent, - * rather than teardown, deletion, or another non-deployment owner. - */ - async isSupersededByNewerLiveDeploymentGeneration(buildId: number, expectedGeneration?: number): Promise { - if (expectedGeneration == null || !Number.isSafeInteger(expectedGeneration)) return false; - - const current = await this.loadBuildDeploymentAuthority(buildId); - const desiredGeneration = Number(current?.desiredGeneration); - return Boolean( - current && - Number.isSafeInteger(desiredGeneration) && - desiredGeneration > expectedGeneration && - current.status !== BuildStatus.TEARING_DOWN && - current.status !== BuildStatus.TORN_DOWN && - current.runUUID !== buildTeardownRunUUID(buildId) && - this.deploymentBlockReason(current) == null - ); - } - private async isBuildSetupRunCurrent(buildId: number, runUUID: string): Promise { const current = await this.loadBuildDeploymentAuthority(buildId); return Boolean(current && current.runUUID === runUUID && this.buildSetupBlockReason(current) == null); diff --git a/src/server/services/deploy.ts b/src/server/services/deploy.ts index d6bce34..3deccd7 100644 --- a/src/server/services/deploy.ts +++ b/src/server/services/deploy.ts @@ -67,6 +67,9 @@ interface SyncedServiceExternalSecrets { buildSecretEnvKeys: Set; } +const AFTER_BUILD_RUNNING_MESSAGE = 'Running after-build pipeline...'; +const AFTER_BUILD_FAILED_MESSAGE = 'After-build pipeline failed.'; + export default class DeployService extends BaseService { /** * Creates all of the relevant deploys for a build, based on the provided environment, if they do not already exist. @@ -851,6 +854,68 @@ export default class DeployService extends BaseService { }); } + // A Lifecycle tag (sha + env hash) identifies equivalent build outputs, so it keys the completion. + private afterBuildCompletionKeyFor(afterBuildPipelineId: string, ecrRepoTag: string): string { + return `${afterBuildPipelineId}@${ecrRepoTag}`; + } + + private async runAfterBuildPipeline({ + deploy, + runUUID, + afterBuildPipelineId, + ecrRepoTag, + branchName, + sourceRevision, + }: { + deploy: Deploy; + runUUID: string; + afterBuildPipelineId: string; + ecrRepoTag: string; + branchName: string; + sourceRevision: string; + }): Promise { + const current = await this.patchDeployForRun(deploy, runUUID, { + status: DeployStatus.BUILDING, + statusMessage: AFTER_BUILD_RUNNING_MESSAGE, + buildLogs: null, + }); + if (!current) return false; + + let pipelineId: string; + try { + // Superset of the native (branch) and Codefresh-embedded (SOURCE_*) invocation contracts. + pipelineId = await codefresh.triggerPipeline(afterBuildPipelineId, 'cli', { + ...deploy.env, + TAG: ecrRepoTag, + branch: branchName, + SOURCE_REVISION: sourceRevision, + SOURCE_BRANCH: branchName, + }); + } catch (error) { + getLogger({ error, afterBuildPipelineId, imageTag: ecrRepoTag }).warn( + 'Codefresh: after-build pipeline trigger failed' + ); + return false; + } + const details = `afterBuildPipelineId=${afterBuildPipelineId} pipelineId=${pipelineId} imageTag=${ecrRepoTag}`; + const logger = getLogger({ afterBuildPipelineId, pipelineId, imageTag: ecrRepoTag }); + logger.info(`Codefresh: after-build pipeline triggered ${details}`); + + await this.patchDeployForRun(deploy, runUUID, { + buildLogs: `https://g.codefresh.io/build/${pipelineId}`, + }); + const completed = await codefresh.waitForImage(pipelineId); + if (!completed) { + logger.warn(`Codefresh: after-build pipeline completed result=failure ${details}`); + return false; + } + await this.patchDeployForRun(deploy, runUUID, { + afterBuildCompletionKey: this.afterBuildCompletionKeyFor(afterBuildPipelineId, ecrRepoTag), + }); + logger.info(`Codefresh: after-build pipeline completed result=success ${details}`); + return true; + } + private async syncServiceExternalSecrets({ deploy, serviceName, @@ -1088,6 +1153,14 @@ export default class DeployService extends BaseService { } await deploy.reload(); + const prepared = await this.patchDeployForRun(deploy, runUUID, { + buildLogs: null, + ...(deployable.afterBuildPipelineId ? { afterBuildCompletionKey: null } : {}), + }); + if (!prepared) { + getLogger().info('Image: stopped before build reason=superseded'); + return true; + } await this.patchAndUpdateActivityFeed( deploy, { status: DeployStatus.BUILDING, sha: fullSha, statusMessage: `Building ${deploy?.uuid}...` }, @@ -1186,72 +1259,35 @@ export default class DeployService extends BaseService { } if (result.success) { - if (runIsCurrent) { - await this.patchDeployWithTag({ tag, initTag, deploy, ecrDomain, runUUID }); - runIsCurrent = await this.isDeploymentRunCurrent(deploy, runUUID, expectedGeneration); - } + // The successor re-runs any missing after-build on its image-exists path. + if (!runIsCurrent) return true; if (buildOptions?.afterBuildPipelineId) { - // This pipeline belongs to the produced image. A newer live generation may detach it, - // while teardown or deletion must suppress new external work. - const ecrRepoTag = constructEcrTag({ repo: ecrRepo, tag, ecrDomain }); - - if (!runIsCurrent) { - let hasLiveSuccessor = false; - try { - hasLiveSuccessor = await this.db.services.BuildService.isSupersededByNewerLiveDeploymentGeneration( - deploy.buildId, - expectedGeneration - ); - } catch (error) { - getLogger({ error }).warn('Codefresh: after-build successor check failed'); - } - - if (!hasLiveSuccessor) { - const skippedLog = { - afterBuildPipelineId: buildOptions.afterBuildPipelineId, - imageTag: ecrRepoTag, - }; - getLogger(skippedLog).info( - `Codefresh: after-build pipeline skipped reason=no_live_successor ` + - `afterBuildPipelineId=${skippedLog.afterBuildPipelineId} imageTag=${skippedLog.imageTag}` - ); - return true; - } - } - - const afterbuildPipeline = await codefresh.triggerPipeline(buildOptions.afterBuildPipelineId, 'cli', { - ...deploy.env, - ...{ TAG: ecrRepoTag }, - ...{ branch: branchName }, - }); - const afterBuildLog = { + const completed = await this.runAfterBuildPipeline({ + deploy, + runUUID, afterBuildPipelineId: buildOptions.afterBuildPipelineId, - pipelineId: afterbuildPipeline, - imageTag: ecrRepoTag, - }; - const afterBuildLogDetails = - `afterBuildPipelineId=${afterBuildLog.afterBuildPipelineId} ` + - `pipelineId=${afterBuildLog.pipelineId} imageTag=${afterBuildLog.imageTag}`; - const afterBuildLogger = getLogger(afterBuildLog); - afterBuildLogger.info(`Codefresh: after-build pipeline triggered ${afterBuildLogDetails}`); - - if (!runIsCurrent || !(await this.isDeploymentRunCurrent(deploy, runUUID, expectedGeneration))) { - afterBuildLogger.info( - `Codefresh: after-build pipeline detached reason=superseded ${afterBuildLogDetails}` + ecrRepoTag: constructEcrTag({ repo: ecrRepo, tag, ecrDomain }), + branchName, + sourceRevision: fullSha, + }); + if (!(await this.isDeploymentRunCurrent(deploy, runUUID, expectedGeneration))) { + getLogger().info( + `Image: after-build publication skipped reason=superseded result=${completed ? 'success' : 'failure'}` ); return true; } - - const completed = await codefresh.waitForImage(afterbuildPipeline); if (!completed) { - afterBuildLogger.warn( - `Codefresh: after-build pipeline completed result=failure ${afterBuildLogDetails}` + await this.patchAndUpdateActivityFeed( + deploy, + { status: DeployStatus.BUILD_FAILED, statusMessage: AFTER_BUILD_FAILED_MESSAGE }, + runUUID ); return false; } - afterBuildLogger.info(`Codefresh: after-build pipeline completed result=success ${afterBuildLogDetails}`); } + + await this.patchDeployWithTag({ tag, initTag, deploy, ecrDomain, runUUID }); return true; } else { if (!runIsCurrent) return true; @@ -1275,6 +1311,15 @@ export default class DeployService extends BaseService { await this.patchDeployForRun(deploy, runUUID, { buildOutput }); if (buildSuccess) { + if (buildOptions?.afterBuildPipelineId && !deployable.detatchAfterBuildPipeline) { + // A non-detached embedded PostBuild must succeed for the parent pipeline to succeed. + await this.patchDeployForRun(deploy, runUUID, { + afterBuildCompletionKey: this.afterBuildCompletionKeyFor( + buildOptions.afterBuildPipelineId, + constructEcrTag({ repo: ecrRepo, tag, ecrDomain }) + ), + }); + } await this.patchDeployWithTag({ tag, initTag, deploy, ecrDomain, runUUID }); getLogger().info('Image: built'); return true; @@ -1302,6 +1347,36 @@ export default class DeployService extends BaseService { return true; } } + + // An existing image is not proof its after-build ran; the recorded completion is. + if (deployable.afterBuildPipelineId) { + const ecrRepoTag = constructEcrTag({ repo: ecrRepo, tag, ecrDomain }); + const completionKey = this.afterBuildCompletionKeyFor(deployable.afterBuildPipelineId, ecrRepoTag); + if (deploy.afterBuildCompletionKey !== completionKey) { + const completed = await this.runAfterBuildPipeline({ + deploy, + runUUID, + afterBuildPipelineId: deployable.afterBuildPipelineId, + ecrRepoTag, + branchName, + sourceRevision: fullSha, + }); + if (!(await this.isDeploymentRunCurrent(deploy, runUUID, expectedGeneration))) { + getLogger().info( + `Image: after-build publication skipped reason=superseded result=${completed ? 'success' : 'failure'}` + ); + return true; + } + if (!completed) { + await this.patchAndUpdateActivityFeed( + deploy, + { status: DeployStatus.BUILD_FAILED, statusMessage: AFTER_BUILD_FAILED_MESSAGE }, + runUUID + ); + return false; + } + } + } await this.patchDeployWithTag({ tag, initTag, deploy, ecrDomain, runUUID }); await this.patchAndUpdateActivityFeed(deploy, { status: DeployStatus.BUILT }, runUUID); return true;