Skip to content

Commit

Permalink
fix(3230): fix assigned remote trigger events (#3231)
Browse files Browse the repository at this point in the history
  • Loading branch information
yakanechi authored Oct 24, 2024
1 parent ef31fca commit ffdfc1d
Show file tree
Hide file tree
Showing 8 changed files with 307 additions and 81 deletions.
49 changes: 46 additions & 3 deletions plugins/builds/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ const {
trimJobName,
getParallelBuilds,
isStartFromMiddleOfCurrentStage,
Status
Status,
getSameParentEvents
} = require('./triggers/helpers');
const { getFullStageJobName } = require('../helper');

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down
19 changes: 19 additions & 0 deletions plugins/builds/triggers/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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<Event[]>} 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
Expand Down Expand Up @@ -1148,6 +1166,7 @@ module.exports = {
parseJobInfo,
createInternalBuild,
getParallelBuilds,
getSameParentEvents,
mergeParentBuilds,
updateParentBuilds,
getParentBuildStatus,
Expand Down
21 changes: 3 additions & 18 deletions test/plugins/builds.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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 } }
Expand Down Expand Up @@ -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 = {
Expand Down Expand Up @@ -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);
Expand Down
14 changes: 14 additions & 0 deletions test/plugins/data/trigger/a_b-downstream.yaml
Original file line number Diff line number Diff line change
@@ -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 ]
11 changes: 11 additions & 0 deletions test/plugins/data/trigger/~sd@1:a-and-~sd@1:b-downstream.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
shared:
image: node:20
steps:
- test: echo 'test'

jobs:
a:
requires: [ ~sd@1:a ]

target:
requires: [ ~sd@1:b ]
14 changes: 14 additions & 0 deletions test/plugins/data/trigger/~sd@1:a-and-~sd@1:b-upstream.yaml
Original file line number Diff line number Diff line change
@@ -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 ]
34 changes: 34 additions & 0 deletions test/plugins/trigger.helper.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Loading

0 comments on commit ffdfc1d

Please sign in to comment.