diff --git a/plugins/builds/index.js b/plugins/builds/index.js index 6f7ece399..955dffc1a 100644 --- a/plugins/builds/index.js +++ b/plugins/builds/index.js @@ -38,7 +38,8 @@ const { trimJobName, getParallelBuilds, isStartFromMiddleOfCurrentStage, - Status + Status, + getSameParentEvents } = require('./triggers/helpers'); const { getFullStageJobName } = require('../helper'); @@ -186,14 +187,43 @@ async function triggerNextJobs(config, app) { }); groupEventBuilds.push(...parallelBuilds); + } else { + const sameParentEvents = await getSameParentEvents({ + eventFactory, + parentEventId: currentEvent.id, + pipelineId: strToInt(joinedPipelineId) + }); + + if (sameParentEvents.length > 0) { + externalEvent = sameParentEvents[0]; + } } const buildsToRestart = buildsToRestartFilter(joinedPipeline, groupEventBuilds, currentEvent, currentBuild); const isRestart = buildsToRestart.length > 0; // If user used external trigger syntax, the jobs are triggered as external - if (isCurrentPipeline || isRestart) { + if (isCurrentPipeline) { externalEvent = null; + } else if (isRestart) { + // If parentEvent and currentEvent have the same pipelineId, then currentEvent is the event that started the restart + // If restarted from the downstream pipeline, the remote trigger must create a new event in the upstream pipeline + const parentEvent = await eventFactory.get({ id: currentEvent.parentEventId }); + const isRestartPipeline = strToInt(currentEvent.pipelineId) === strToInt(parentEvent.pipelineId); + + if (isRestartPipeline) { + const sameParentEvents = await getSameParentEvents({ + eventFactory, + parentEventId: currentEvent.id, + pipelineId: strToInt(joinedPipelineId) + }); + + if (sameParentEvents.length > 0) { + externalEvent = sameParentEvents[0]; + } else { + externalEvent = null; + } + } } // no need to lock if there is no external event @@ -225,8 +255,21 @@ async function triggerNextJobs(config, app) { // Restart case if (isRestart) { - externalEventConfig.groupEventId = joinedPipeline.event.id; + // 'joinedPipeline.event.id' is restart event, not group event. + const groupEvent = await eventFactory.get({ id: joinedPipeline.event.id }); + + externalEventConfig.groupEventId = groupEvent.groupEventId; externalEventConfig.parentBuilds = buildsToRestart[0].parentBuilds; + } else { + const sameParentEvents = await getSameParentEvents({ + eventFactory, + parentEventId: currentEvent.groupEventId, + pipelineId: strToInt(joinedPipelineId) + }); + + if (sameParentEvents.length > 0) { + externalEventConfig.groupEventId = sameParentEvents[0].groupEventId; + } } try { diff --git a/plugins/builds/triggers/helpers.js b/plugins/builds/triggers/helpers.js index 27daaf2b6..4a7148386 100644 --- a/plugins/builds/triggers/helpers.js +++ b/plugins/builds/triggers/helpers.js @@ -678,6 +678,24 @@ async function getParallelBuilds({ eventFactory, parentEventId, pipelineId }) { return parallelBuilds; } +/** + * Get all events with a given event ID and pipeline ID + * @param {Object} arg + * @param {EventFactory} eventFactory Event factory + * @param {Number} parentEventId Parent event ID + * @param {Number} pipelineId Pipeline ID + * @returns {Promise} Array of events with same parent event ID and same pipeline ID + */ +async function getSameParentEvents({ eventFactory, parentEventId, pipelineId }) { + const parallelEvents = await eventFactory.list({ + params: { + parentEventId + } + }); + + return parallelEvents.filter(pe => strToInt(pe.pipelineId) === pipelineId); +} + /** * Get subsequent job names which the root is the start from node * @param {Array} [workflowGraph] Array of graph vertices @@ -1148,6 +1166,7 @@ module.exports = { parseJobInfo, createInternalBuild, getParallelBuilds, + getSameParentEvents, mergeParentBuilds, updateParentBuilds, getParentBuildStatus, diff --git a/test/plugins/builds.test.js b/test/plugins/builds.test.js index 48cf62292..3044ee28f 100644 --- a/test/plugins/builds.test.js +++ b/test/plugins/builds.test.js @@ -3111,6 +3111,7 @@ describe('build plugin test', () => { const externalEventMock = { ...eventMock }; eventFactoryMock.create.resolves(externalEventMock); + eventFactoryMock.list.resolves([]); return newServer.inject(options).then(() => { assert.calledOnce(buildFactoryMock.create); @@ -4942,6 +4943,7 @@ describe('build plugin test', () => { { src: '~sd@2:a', dest: 'a' } ] }; + eventMock.groupEventId = '8887'; eventMock.parentEventId = 8887; buildMock.parentBuilds = { 2: { eventId: '8887', jobs: { a: 12345 } } @@ -4974,22 +4976,6 @@ describe('build plugin test', () => { }, start: sinon.stub().resolves() }); - const eventConfig = { - causeMessage: 'Triggered by sd@123:a', - groupEventId: '8889', - parentBuildId: 12345, - parentBuilds: { - 123: { eventId: '8888', jobs: { a: 12344, c: 45678 } } - }, - parentEventId: '8888', - pipelineId: '2', - scmContext: 'github:github.com', - sha: 'sha', - startFrom: '~sd@123:a', - skipMessage: 'Skip bulk external builds creation', - type: 'pipeline', - username: 'foo' - }; buildC.update = sinon.stub().resolves(updatedBuildC); const externalEventMock = { @@ -5089,8 +5075,7 @@ describe('build plugin test', () => { buildFactoryMock.get.withArgs(5555).resolves({ status: 'SUCCESS' }); // d is done return newServer.inject(options).then(() => { - assert.calledOnce(eventFactoryMock.create); - assert.calledWith(eventFactoryMock.create, eventConfig); + assert.notCalled(eventFactoryMock.create); assert.calledOnce(buildFactoryMock.getLatestBuilds); assert.calledOnce(buildFactoryMock.create); assert.calledOnce(buildC.update); diff --git a/test/plugins/data/trigger/a_b-downstream.yaml b/test/plugins/data/trigger/a_b-downstream.yaml new file mode 100644 index 000000000..aa66f2e8a --- /dev/null +++ b/test/plugins/data/trigger/a_b-downstream.yaml @@ -0,0 +1,14 @@ +shared: + image: node:20 + steps: + - test: echo 'test' + +jobs: + a: + requires: [ ~sd@1:a ] + + b: + requires: [ ~sd@1:b ] + + target: + requires: [ a, b ] diff --git a/test/plugins/data/trigger/~sd@1:a-and-~sd@1:b-downstream.yaml b/test/plugins/data/trigger/~sd@1:a-and-~sd@1:b-downstream.yaml new file mode 100644 index 000000000..8f8bd54ad --- /dev/null +++ b/test/plugins/data/trigger/~sd@1:a-and-~sd@1:b-downstream.yaml @@ -0,0 +1,11 @@ +shared: + image: node:20 + steps: + - test: echo 'test' + +jobs: + a: + requires: [ ~sd@1:a ] + + target: + requires: [ ~sd@1:b ] diff --git a/test/plugins/data/trigger/~sd@1:a-and-~sd@1:b-upstream.yaml b/test/plugins/data/trigger/~sd@1:a-and-~sd@1:b-upstream.yaml new file mode 100644 index 000000000..b17b1cde3 --- /dev/null +++ b/test/plugins/data/trigger/~sd@1:a-and-~sd@1:b-upstream.yaml @@ -0,0 +1,14 @@ +shared: + image: node:20 + steps: + - test: echo 'test' + +jobs: + hub: + requires: [ ~commit, ~pr ] + + a: + requires: [ ~hub ] + + b: + requires: [ ~sd@2:a ] diff --git a/test/plugins/trigger.helper.test.js b/test/plugins/trigger.helper.test.js index 9bfc42d6d..47269363d 100644 --- a/test/plugins/trigger.helper.test.js +++ b/test/plugins/trigger.helper.test.js @@ -659,6 +659,40 @@ describe('getParallelBuilds function', () => { }); }); +describe('getSameParentEvents function', () => { + let eventFactoryMock; + + const getSameParentEvents = RewiredTriggerHelper.__get__('getSameParentEvents'); + + beforeEach(() => { + eventFactoryMock = { + list: sinon.stub() + }; + }); + + it('should get same parent events correctly', async () => { + const parentEventId = 101; + const pipelineId = 1; + + const sameParentEvent1 = { + pipelineId: 1, + parentEventId: 101 + }; + const sameParentEvent2 = { + pipelineId: 2, + parentEventId: 101 + }; + + eventFactoryMock.list.resolves([sameParentEvent1, sameParentEvent2]); + + const result = await getSameParentEvents({ eventFactory: eventFactoryMock, parentEventId, pipelineId }); + + const expected = [{ pipelineId: 1, parentEventId: 101 }]; + + assert.deepEqual(result, expected); + }); +}); + describe('mergeParentBuilds function', () => { let loggerWarnStub; diff --git a/test/plugins/trigger.test.js b/test/plugins/trigger.test.js index edbf13dc6..9a4180eae 100644 --- a/test/plugins/trigger.test.js +++ b/test/plugins/trigger.test.js @@ -1442,7 +1442,7 @@ describe('trigger tests', () => { assert.equal(upstreamPipeline.getBuildsOf('target').length, 1); }); - it('[ ~sd@1:a, ~sd@1:b ] is triggered twice in a downstream when sd@1:a and sd@1:b succeed', async () => { + it('[ ~sd@1:a, ~sd@1:b ] is triggered once in a downstream when sd@1:a and sd@1:b succeed', async () => { const upstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@1:a_~sd@1:b-upstream.yaml'); const downstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@1:a_~sd@1:b-downstream.yaml'); @@ -1460,9 +1460,8 @@ describe('trigger tests', () => { const downstreamEvent2 = downstreamPipeline.getLatestEvent(); - assert.notEqual(downstreamEvent1.id, downstreamEvent2.id); + assert.equal(downstreamEvent1.id, downstreamEvent2.id); assert.equal(downstreamEvent1.getBuildOf('target').status, 'RUNNING'); - assert.equal(downstreamEvent2.getBuildOf('target').status, 'RUNNING'); }); it('[ ~sd@2:a, ~sd@2:b ] is triggered once in a upstream when sd@2:a and sd@2:b succeed', async () => { @@ -1499,8 +1498,8 @@ describe('trigger tests', () => { const downstreamEvent2 = downstreamPipeline.getLatestEvent(); - assert.isNull(downstreamEvent1.getBuildOf('target')); - assert.equal(downstreamEvent2.getBuildOf('target').status, 'RUNNING'); + assert.equal(downstreamEvent1.id, downstreamEvent2.id); + assert.equal(downstreamEvent1.getBuildOf('target').status, 'RUNNING'); }); it('[ ~sd@2:a, ~b ] is triggered when sd@2:a succeeds', async () => { @@ -1559,7 +1558,7 @@ describe('trigger tests', () => { assert.equal(upstreamPipeline.getBuildsOf('target').length, 1); }); - it('[ ~sd@1:a, ~b ] is triggered twice in downstream when sd@1:a and b succeed', async () => { + it('[ ~sd@1:a, ~b ] is triggered once in downstream when sd@1:a and b succeed', async () => { const upstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@1:a_~b-upstream.yaml'); const downstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@1:a_~b-downstream.yaml'); @@ -1571,11 +1570,10 @@ describe('trigger tests', () => { // run all builds await buildFactoryMock.run(); - const [downstreamEvent1, downstreamEvent2] = eventFactoryMock.getChildEvents(upstreamEvent.id); + const [downstreamEvent1] = eventFactoryMock.getChildEvents(upstreamEvent.id); assert.equal(downstreamEvent1.getBuildOf('target').status, 'SUCCESS'); - assert.equal(downstreamEvent2.getBuildOf('target').status, 'SUCCESS'); - assert.equal(downstreamPipeline.getBuildsOf('target').length, 2); + assert.equal(downstreamPipeline.getBuildsOf('target').length, 1); }); it('[ ~sd@2:a, ~b ] is triggered once in upstream when sd@2:a and b succeed', async () => { @@ -1724,7 +1722,6 @@ describe('trigger tests', () => { const upstreamRestartEvent = upstreamPipeline.getLatestEvent(); - assert.equal(upstreamEvent.getBuildOf('target').status, 'RUNNING'); assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING'); assert.equal(upstreamPipeline.getBuildsOf('target').length, 2); }); @@ -2821,6 +2818,165 @@ describe('trigger tests', () => { assert.equal(firstPipeline.getBuildsOf('target').length, 1); }); + it('[ ~sd@1:a ], [ ~sd@1:b ] is triggered by same groupEventId when sd@1:a succeeds', async () => { + const upstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@1:a_~sd@1:b-upstream.yaml'); + const downstreamPipeline = await pipelineFactoryMock.createFromFile('a_b-downstream.yaml'); + + const upstreamEvent = await eventFactoryMock.create({ + pipelineId: upstreamPipeline.id, + startFrom: 'hub' + }); + + await upstreamEvent.getBuildOf('hub').complete('SUCCESS'); + await upstreamEvent.getBuildOf('a').complete('SUCCESS'); + const downstreamEvent = downstreamPipeline.getLatestEvent(); + + assert.equal(downstreamEvent.getBuildOf('a').status, 'RUNNING'); + assert.isNull(downstreamEvent.getBuildOf('b')); + + await upstreamEvent.getBuildOf('b').complete('SUCCESS'); + assert.equal(downstreamEvent.getBuildOf('b').status, 'RUNNING'); + + await downstreamEvent.getBuildOf('a').complete('SUCCESS'); + await downstreamEvent.getBuildOf('b').complete('SUCCESS'); + + assert.equal(downstreamEvent.id, downstreamPipeline.getLatestEvent().id); + assert.equal(downstreamEvent.getBuildOf('target').status, 'RUNNING'); + }); + + it('[ a, b ] in downstream is not triggered by same groupEventId', async () => { + const upstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@1:a_~sd@1:b-upstream.yaml'); + const downstreamPipeline = await pipelineFactoryMock.createFromFile('a_b-downstream.yaml'); + + const upstreamEvent = await eventFactoryMock.create({ + pipelineId: upstreamPipeline.id, + startFrom: 'hub' + }); + + await upstreamEvent.getBuildOf('hub').complete('SUCCESS'); + await upstreamEvent.getBuildOf('a').complete('SUCCESS'); + const downstreamEvent = downstreamPipeline.getLatestEvent(); + + assert.isNull(downstreamEvent.getBuildOf('b')); + assert.equal(downstreamEvent.getBuildOf('a').status, 'RUNNING'); + await downstreamEvent.getBuildOf('a').complete('SUCCESS'); + + await upstreamEvent.getBuildOf('b').complete('SUCCESS'); + + assert.equal(downstreamEvent.getBuildOf('b').status, 'RUNNING'); + await downstreamEvent.getBuildOf('b').complete('SUCCESS'); + + assert.equal(downstreamEvent.getBuildOf('target').status, 'RUNNING'); + assert.equal(downstreamEvent, downstreamPipeline.getLatestEvent()); + }); + + it('[ ~sd@2:a ] is triggered in a upstream when restarts and succeeds', async () => { + const upstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@2:a-upstream.yaml'); + const downstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@2:a-downstream.yaml'); + + const upstreamEvent = await eventFactoryMock.create({ + pipelineId: upstreamPipeline.id, + startFrom: 'hub' + }); + + await upstreamEvent.run(); + + const downstreamEvent = downstreamPipeline.getLatestEvent(); + + await downstreamEvent.getBuildOf('a').complete('SUCCESS'); + await upstreamEvent.getBuildOf('target').complete('SUCCESS'); + + // Restart + const upstreamRestartEvent = await upstreamEvent.restartFrom('a'); + + await upstreamRestartEvent.getBuildOf('a').complete('SUCCESS'); + assert.equal(upstreamEvent.groupEventId, upstreamRestartEvent.groupEventId); + assert.isNull(upstreamRestartEvent.getBuildOf('target')); + + // downstream (after restart) + const downstreamRestartEvent = downstreamPipeline.getLatestEvent(); + + assert.notEqual(downstreamEvent.id, downstreamRestartEvent.id); + assert.equal(downstreamEvent.groupEventId, downstreamRestartEvent.groupEventId); + + await downstreamRestartEvent.getBuildOf('a').complete('SUCCESS'); + + assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING'); + }); + + it('[ ~sd@1:b ] is triggered in a downstream when restarts and succeeds', async () => { + const upstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@1:a-and-~sd@1:b-upstream.yaml'); + const downstreamPipeline = await pipelineFactoryMock.createFromFile('~sd@1:a-and-~sd@1:b-downstream.yaml'); + + const upstreamEvent = await eventFactoryMock.create({ + pipelineId: upstreamPipeline.id, + startFrom: 'hub' + }); + + // run all builds + await upstreamEvent.run(); + + const downstreamEvent = downstreamPipeline.getLatestEvent(); + + await downstreamEvent.getBuildOf('a').complete('SUCCESS'); + await upstreamEvent.getBuildOf('b').complete('SUCCESS'); + + assert.equal(downstreamEvent.getBuildOf('target').status, 'RUNNING'); + + const upstreamRestartEvent = await upstreamEvent.restartFrom('a'); + + await upstreamRestartEvent.getBuildOf('a').complete('SUCCESS'); + const downstreamRestartEvent = downstreamPipeline.getLatestEvent(); + + await downstreamRestartEvent.getBuildOf('a').complete('SUCCESS'); + assert.notEqual(downstreamEvent.id, downstreamRestartEvent.id); + + assert.equal(upstreamRestartEvent.id, upstreamPipeline.getLatestEvent().id); + + await upstreamRestartEvent.getBuildOf('b').complete('SUCCESS'); + + assert.equal(downstreamRestartEvent.id, downstreamPipeline.getLatestEvent().id); + assert.equal(downstreamRestartEvent.getBuildOf('target').status, 'RUNNING'); + }); + + it('[ sd@2:a, sd@3:a ] is triggered when restarts a and wait for downstream restart builds', async () => { + const upstreamPipeline = await pipelineFactoryMock.createFromFile('sd@2:a_sd@3:a-upstream.yaml'); + const downstreamPipeline1 = await pipelineFactoryMock.createFromFile('sd@2:a_sd@3:a-downstream.yaml'); + const downstreamPipeline2 = await pipelineFactoryMock.createFromFile('sd@2:a_sd@3:a-downstream.yaml'); + + const upstreamEvent = await eventFactoryMock.create({ + pipelineId: upstreamPipeline.id, + startFrom: 'hub' + }); + + // run all builds + await upstreamEvent.run(); + + const downstreamEvent1 = downstreamPipeline1.getLatestEvent(); + const downstreamEvent2 = downstreamPipeline2.getLatestEvent(); + + await downstreamEvent1.getBuildOf('a').complete('SUCCESS'); + await downstreamEvent2.getBuildOf('a').complete('SUCCESS'); + await upstreamEvent.getBuildOf('target').complete('SUCCESS'); + + const upstreamRestartEvent = await upstreamEvent.restartFrom('hub'); + + await upstreamRestartEvent.getBuildOf('hub').complete('SUCCESS'); + + assert.isNull(upstreamRestartEvent.getBuildOf('target')); + + await upstreamRestartEvent.getBuildOf('a').complete('SUCCESS'); + + const downstreamRestartEvent1 = downstreamPipeline1.getLatestEvent(); + const downstreamRestartEvent2 = downstreamPipeline2.getLatestEvent(); + + await downstreamRestartEvent1.getBuildOf('a').complete('SUCCESS'); + await downstreamRestartEvent2.getBuildOf('a').complete('SUCCESS'); + + assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING'); + assert.equal(upstreamPipeline.getBuildsOf('target').length, 2); + }); + it('[ ~pr ] is triggered', async () => { const pipeline = await pipelineFactoryMock.createFromFile('~pr.yaml'); @@ -3288,54 +3444,4 @@ describe('trigger tests', () => { await event.getBuildOf('c').complete('SUCCESS'); assert.equal(event.getBuildOf('d7').status, 'RUNNING'); }); - - describe('Tests for behavior not ideal', () => { - it('[ sd@2:a, sd@3:a ] is triggered when restarts a and wait for downstream restart builds', async () => { - const upstreamPipeline = await pipelineFactoryMock.createFromFile('sd@2:a_sd@3:a-upstream.yaml'); - const downstreamPipeline1 = await pipelineFactoryMock.createFromFile('sd@2:a_sd@3:a-downstream.yaml'); - const downstreamPipeline2 = await pipelineFactoryMock.createFromFile('sd@2:a_sd@3:a-downstream.yaml'); - - const upstreamEvent = await eventFactoryMock.create({ - pipelineId: upstreamPipeline.id, - startFrom: 'hub' - }); - - // run all builds - await upstreamEvent.run(); - - const downstreamEvent1 = downstreamPipeline1.getLatestEvent(); - const downstreamEvent2 = downstreamPipeline2.getLatestEvent(); - - await downstreamEvent1.getBuildOf('a').complete('SUCCESS'); - await downstreamEvent2.getBuildOf('a').complete('SUCCESS'); - await upstreamEvent.getBuildOf('target').complete('SUCCESS'); - - const upstreamRestartEvent = await upstreamEvent.restartFrom('hub'); - - await upstreamRestartEvent.getBuildOf('hub').complete('SUCCESS'); - - assert.isNull(upstreamRestartEvent.getBuildOf('target')); - - await upstreamRestartEvent.getBuildOf('a').complete('SUCCESS'); - - const downstreamRestartEvent1 = downstreamPipeline1.getLatestEvent(); - const downstreamRestartEvent2 = downstreamPipeline2.getLatestEvent(); - - await downstreamRestartEvent1.getBuildOf('a').complete('SUCCESS'); - await downstreamRestartEvent2.getBuildOf('a').complete('SUCCESS'); - - // expected target job triggerd in restart event and number of target builds are 2 (start and restart). - // https://github.com/screwdriver-cd/screwdriver/issues/3177 - // expected - // assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING'); - // assert.equal(upstreamPipeline.getBuildsOf('target').length, 2); - - // actual - const upstreamRemoteTriggerEvent = upstreamPipeline.getLatestEvent(); - - assert.isNull(upstreamRestartEvent.getBuildOf('target')); - assert.equal(upstreamRemoteTriggerEvent.getBuildOf('target').status, 'RUNNING'); - assert.equal(upstreamPipeline.getBuildsOf('target').length, 3); - }); - }); });