fix(3133): upstream event creation when restarting remote join from d…

…ownstream build (#3250)

Co-authored-by: Yuta Kaneda <[email protected]>

plugins/builds/index.js

@@ -199,31 +199,27 @@ async function triggerNextJobs(config, app) {
             }
         }
 
-        const buildsToRestart = buildsToRestartFilter(joinedPipeline, groupEventBuilds, currentEvent, currentBuild);
-        const isRestart = buildsToRestart.length > 0;
+        let isRestartPipeline = false;
+
+        if (currentEvent.parentEventId) {
+            const parentEvent = await eventFactory.get({ id: currentEvent.parentEventId });
+
+            isRestartPipeline = parentEvent && strToInt(currentEvent.pipelineId) === strToInt(parentEvent.pipelineId);
+        }
 
         // If user used external trigger syntax, the jobs are triggered as external
         if (isCurrentPipeline) {
             externalEvent = null;
-        } else if (isRestart) {
+        } else if (isRestartPipeline) {
             // 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)
-                });
+            const sameParentEvents = await getSameParentEvents({
+                eventFactory,
+                parentEventId: currentEvent.id,
+                pipelineId: strToInt(joinedPipelineId)
+            });
 
-                if (sameParentEvents.length > 0) {
-                    externalEvent = sameParentEvents[0];
-                } else {
-                    externalEvent = null;
-                }
-            }
+            externalEvent = sameParentEvents.length > 0 ? sameParentEvents[0] : null;
         }
 
         // no need to lock if there is no external event
@@ -253,6 +249,9 @@ async function triggerNextJobs(config, app) {
                 groupEventId: null
             };
 
+            const buildsToRestart = buildsToRestartFilter(joinedPipeline, groupEventBuilds, currentEvent, currentBuild);
+            const isRestart = buildsToRestart.length > 0;
+
             // Restart case
             if (isRestart) {
                 // 'joinedPipeline.event.id' is restart event, not group event.
@@ -267,9 +266,8 @@ async function triggerNextJobs(config, app) {
                     pipelineId: strToInt(joinedPipelineId)
                 });
 
-                if (sameParentEvents.length > 0) {
-                    externalEventConfig.groupEventId = sameParentEvents[0].groupEventId;
-                }
+                externalEventConfig.groupEventId =
+                    sameParentEvents.length > 0 ? sameParentEvents[0].groupEventId : currentEvent.groupEventId;
             }
 
             try {

plugins/builds/triggers/helpers.js

@@ -811,7 +811,7 @@ function mergeParentBuilds(parentBuilds, relatedBuilds, currentEvent, nextEvent)
 
             if (strToInt(pipelineId) !== strToInt(currentEvent.pipelineId)) {
                 if (nextEvent) {
-                    if (strToInt(pipelineId) !== nextEvent.pipelineId) {
+                    if (strToInt(pipelineId) !== strToInt(nextEvent.pipelineId)) {
                         nodeName = `sd@${pipelineId}:${nodeName}`;
                     }
                     workflowGraph = nextEvent.workflowGraph;

test/plugins/data/trigger/sd@2:b_sd@2:c-downstream.yaml

@@ -0,0 +1,12 @@
+shared:
+  image: node:20
+  steps:
+    - test: echo 'test'
+
+jobs:
+  hub:
+    requires: [ ~sd@1:hub ]
+  b:
+    requires: [ ~hub ]
+  c:
+    requires: [ ~hub ]

test/plugins/data/trigger/sd@2:b_sd@2:c-upstream.yaml

@@ -0,0 +1,10 @@
+shared:
+  image: node:20
+  steps:
+    - test: echo 'test'
+
+jobs:
+  hub:
+    requires: [ ~commit, ~pr ]
+  target:
+    requires: [ sd@2:b, sd@2:c ]

test/plugins/trigger.test.js

@@ -1195,8 +1195,10 @@ describe('trigger tests', () => {
 
         await downstreamRestartEvent.getBuildOf('a').complete('SUCCESS');
 
-        assert.equal(upstreamEvent.id, upstreamPipeline.getLatestEvent().id);
-        assert.equal(upstreamEvent.getBuildOf('target').status, 'RUNNING');
+        const upstreamRestartEvent = upstreamPipeline.getLatestEvent();
+
+        assert.notEqual(upstreamEvent.id, upstreamRestartEvent.id);
+        assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING');
     });
 
     it('[ sd@1:a ] is triggered in a downstream', async () => {
@@ -1363,7 +1365,9 @@ describe('trigger tests', () => {
 
         await downstreamRestartEvent.getBuildOf('a').complete('SUCCESS');
 
-        assert.equal(upstreamEvent.getBuildOf('target').status, 'RUNNING');
+        const upstreamRestartEvent = await upstreamPipeline.getLatestEvent();
+
+        assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING');
     });
 
     it('[ ~sd@1:a, ~sd@1:b ] is triggered when sd@1:a succeeds', async () => {
@@ -1651,7 +1655,9 @@ describe('trigger tests', () => {
 
         await downstreamRestartEvent.getBuildOf('b').complete('SUCCESS');
 
-        assert.equal(upstreamEvent.getBuildOf('target').status, 'RUNNING');
+        const upstreamRestartEvent = upstreamPipeline.getLatestEvent();
+
+        assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING');
         assert.equal(upstreamPipeline.getBuildsOf('target').length, 1);
     });
 
@@ -1679,6 +1685,35 @@ describe('trigger tests', () => {
         assert.equal(upstreamPipeline.getBuildsOf('target').length, 0);
     });
 
+    it('[ sd@2:b, sd@2:c ] is triggered when sd@2:b and sd@2:c fail and restart and both succeed', async () => {
+        const upstreamPipeline = await pipelineFactoryMock.createFromFile('sd@2:b_sd@2:c-upstream.yaml');
+        const downstreamPipeline = await pipelineFactoryMock.createFromFile('sd@2:b_sd@2:c-downstream.yaml');
+
+        const upstreamEvent = await eventFactoryMock.create({
+            pipelineId: upstreamPipeline.id,
+            startFrom: 'hub'
+        });
+
+        await upstreamEvent.run();
+
+        const downstreamEvent = await downstreamPipeline.getLatestEvent();
+
+        await downstreamEvent.getBuildOf('hub').complete('SUCCESS');
+        await downstreamEvent.getBuildOf('b').complete('FAILURE');
+        await downstreamEvent.getBuildOf('c').complete('FAILURE');
+
+        const downstreamRestartEvent = await downstreamEvent.restartFrom('hub');
+
+        await downstreamRestartEvent.getBuildOf('hub').complete('SUCCESS');
+        await downstreamRestartEvent.getBuildOf('b').complete('SUCCESS');
+        await downstreamRestartEvent.getBuildOf('c').complete('SUCCESS');
+
+        const upstreamRestartEvent = await upstreamPipeline.getLatestEvent();
+
+        assert.isNull(upstreamEvent.getBuildOf('target'));
+        assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING');
+    });
+
     it('[ sd@2:a, sd@2:b ] is triggered', async () => {
         const upstreamPipeline = await pipelineFactoryMock.createFromFile('sd@2:a_sd@2:b-upstream.yaml');
         const downstreamPipeline = await pipelineFactoryMock.createFromFile('sd@2:a_sd@2:b-downstream.yaml');
@@ -1864,9 +1899,11 @@ describe('trigger tests', () => {
 
         await downstreamRestartEvent.getBuildOf('a').complete('SUCCESS');
 
-        assert.equal(upstreamEvent, upstreamPipeline.getLatestEvent());
+        const upstreamRestartEvent = await upstreamPipeline.getLatestEvent();
+
+        assert.notEqual(upstreamEvent.id, upstreamRestartEvent.id);
         assert.isNull(upstreamEvent.getBuildOf('target'));
-        assert.equal(upstreamPipeline.getBuildsOf('target').length, 0);
+        assert.isNull(upstreamRestartEvent.getBuildOf('target'));
     });
 
     it('[ sd@2:a, b ] is not triggered when sd@2:a fails and b succeeds', async () => {
@@ -1937,9 +1974,10 @@ describe('trigger tests', () => {
 
         await downstreamRestartEvent.getBuildOf('a').complete('SUCCESS');
 
-        assert.equal(upstreamEvent, upstreamPipeline.getLatestEvent());
-        assert.equal(upstreamEvent.getBuildOf('target').status, 'RUNNING');
-        assert.equal(upstreamPipeline.getBuildsOf('target').length, 1);
+        const upstreamRestartEvent = await upstreamPipeline.getLatestEvent();
+
+        assert.equal(upstreamEvent.getBuildOf('target').status, 'CREATED');
+        assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING');
     });
 
     it('[ sd@2:a, b ] is triggered in restart event when only b restarts', async () => {
@@ -2036,8 +2074,11 @@ describe('trigger tests', () => {
 
         await downstreamRestartEvent1.getBuildOf('a').complete('SUCCESS');
 
-        assert.equal(upstreamEvent, upstreamPipeline.getLatestEvent());
+        const upstreamRestartEvent = upstreamPipeline.getLatestEvent();
+
+        assert.notEqual(upstreamEvent.id, upstreamRestartEvent.id);
         assert.isNull(upstreamEvent.getBuildOf('target'));
+        assert.isNull(upstreamRestartEvent.getBuildOf('target'));
         assert.equal(upstreamPipeline.getBuildsOf('target').length, 0);
     });
 
@@ -2064,9 +2105,10 @@ describe('trigger tests', () => {
 
         await downstreamRestartEvent2.getBuildOf('a').complete('SUCCESS');
 
-        assert.equal(upstreamEvent, upstreamPipeline.getLatestEvent());
-        assert.equal(upstreamEvent.getBuildOf('target').status, 'RUNNING');
-        assert.equal(upstreamPipeline.getBuildsOf('target').length, 1);
+        const upstreamRestartEvent = await upstreamPipeline.getLatestEvent();
+
+        assert.isNull(upstreamEvent.getBuildOf('target'));
+        assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING');
     });
 
     it('[ sd@2:a, sd@3:a ] is triggered in restart event when only sd@3:a restarts', async () => {
@@ -2278,8 +2320,10 @@ describe('trigger tests', () => {
 
         await downstreamRestartEvent.getBuildOf('c').complete('SUCCESS');
 
-        assert.equal(upstreamEvent, upstreamPipeline.getLatestEvent());
-        assert.equal(upstreamEvent.getBuildOf('target').status, 'RUNNING');
+        const upstreamRestartEvent = await upstreamPipeline.getLatestEvent();
+
+        assert.isNull(upstreamEvent.getBuildOf('target'));
+        assert.equal(upstreamRestartEvent.getBuildOf('target').status, 'RUNNING');
         assert.equal(upstreamPipeline.getBuildsOf('target').length, 1);
     });