From a6c0bba7ffac695f6bc6ab93fba0133b700cc9c8 Mon Sep 17 00:00:00 2001 From: hamza iftikhar Date: Wed, 7 Aug 2024 12:32:13 -0400 Subject: [PATCH 1/2] Refactoring --- package.json | 4 ++-- plugins/builds/triggers/helpers.js | 24 ++++++++++++++++-------- 2 files changed, 18 insertions(+), 10 deletions(-) diff --git a/package.json b/package.json index d90e079ff..dd005eb0b 100644 --- a/package.json +++ b/package.json @@ -105,7 +105,7 @@ "screwdriver-config-parser": "^10.4.1", "screwdriver-coverage-bookend": "^2.0.0", "screwdriver-coverage-sonar": "^4.1.1", - "screwdriver-data-schema": "^23.2.0", + "screwdriver-data-schema": "^23.7.0", "screwdriver-datastore-sequelize": "^8.2.0", "screwdriver-executor-base": "^9.0.1", "screwdriver-executor-docker": "^6.0.0", @@ -123,7 +123,7 @@ "screwdriver-scm-github": "^12.6.0", "screwdriver-scm-gitlab": "^3.1.0", "screwdriver-scm-router": "^7.1.0", - "screwdriver-template-validator": "^8.1.0", + "screwdriver-template-validator": "^8.2.0", "screwdriver-workflow-parser": "^4.1.1", "sqlite3": "^5.1.4", "stream": "0.0.3", diff --git a/plugins/builds/triggers/helpers.js b/plugins/builds/triggers/helpers.js index 70346e3f0..afbd62bbc 100644 --- a/plugins/builds/triggers/helpers.js +++ b/plugins/builds/triggers/helpers.js @@ -4,9 +4,8 @@ const logger = require('screwdriver-logger'); const workflowParser = require('screwdriver-workflow-parser'); const merge = require('lodash.mergewith'); const schema = require('screwdriver-data-schema'); -const { EXTERNAL_TRIGGER_ALL } = schema.config.regex; +const { EXTERNAL_TRIGGER_ALL, STAGE_SETUP_PATTERN } = schema.config.regex; const { getFullStageJobName } = require('../../helper'); -const STAGE_SETUP_PATTERN = /^stage@([\w-]+)(?::setup)$/; /** * @typedef {import('screwdriver-models').JobFactory} JobFactory @@ -1074,26 +1073,35 @@ function isStageSetup(jobName) { } /** - * Check if the job is a stage job + * get the stage name of a job * @param {String} jobName Job name * @param {Object} workflowGraph Workflow Graph - * @return {Boolean} + * @return {String} Stage name */ -function isStageJob(workflowGraph, jobName) { +function getStageName(workflowGraph, jobName) { const jobNode = workflowGraph.nodes.find(n => n.name === jobName); - return jobNode.stageName !== undefined; + return jobNode ? jobNode.stageName : null; } /** - * Check if the current job is a stage setup and the next job is a non-setup stage job + * Check if the current job is a stage setup and the next job is a non-setup job in the same stage * @param {String} triggerJob Current job * @param {String} startFrom Next job * @param {Object} workflowGraph Workflow Graph * @return {Boolean} */ function isStartFromMiddleOfStage(triggerJob, startFrom, workflowGraph) { - return isStageSetup(triggerJob) && !isStageSetup(startFrom) && isStageJob(workflowGraph, startFrom); + if (isStageSetup(triggerJob)) { + const startFromStageName = getStageName(workflowGraph, triggerJob); + const triggerStageName = getStageName(workflowGraph, triggerJob); + + if (!isStageSetup(startFrom) && startFromStageName === triggerStageName) { + return true; + } + } + + return false; } module.exports = { From ce7fa2cec4fe87717efb00a4606385d142af490e Mon Sep 17 00:00:00 2001 From: hamza iftikhar Date: Thu, 8 Aug 2024 13:13:47 -0400 Subject: [PATCH 2/2] Add check that the event startFrom and the current job are from the same stage for the startFrom job to be executed right after the current job --- package.json | 2 +- plugins/builds/index.js | 4 +- plugins/builds/triggers/helpers.js | 20 +++----- test/plugins/builds.test.js | 77 +++++++++++++++++++++++++++++- 4 files changed, 86 insertions(+), 17 deletions(-) diff --git a/package.json b/package.json index dd005eb0b..bb908ced1 100644 --- a/package.json +++ b/package.json @@ -124,7 +124,7 @@ "screwdriver-scm-gitlab": "^3.1.0", "screwdriver-scm-router": "^7.1.0", "screwdriver-template-validator": "^8.2.0", - "screwdriver-workflow-parser": "^4.1.1", + "screwdriver-workflow-parser": "^4.4.1", "sqlite3": "^5.1.4", "stream": "0.0.3", "tinytim": "^0.1.1", diff --git a/plugins/builds/index.js b/plugins/builds/index.js index 2239e0e4d..61896f929 100644 --- a/plugins/builds/index.js +++ b/plugins/builds/index.js @@ -36,7 +36,7 @@ const { buildsToRestartFilter, trimJobName, getParallelBuilds, - isStartFromMiddleOfStage, + isStartFromMiddleOfCurrentStage, Status } = require('./triggers/helpers'); @@ -118,7 +118,7 @@ async function triggerNextJobs(config, app) { */ if ( isOrTrigger(currentEvent.workflowGraph, originalCurrentJobName, trimJobName(nextJobName)) || - isStartFromMiddleOfStage(currentJob.name, currentEvent.startFrom, currentEvent.workflowGraph) + isStartFromMiddleOfCurrentStage(currentJob.name, currentEvent.startFrom, currentEvent.workflowGraph) ) { nextBuild = await orTrigger.execute( currentEvent, diff --git a/plugins/builds/triggers/helpers.js b/plugins/builds/triggers/helpers.js index afbd62bbc..9337da08f 100644 --- a/plugins/builds/triggers/helpers.js +++ b/plugins/builds/triggers/helpers.js @@ -1086,22 +1086,16 @@ function getStageName(workflowGraph, jobName) { /** * Check if the current job is a stage setup and the next job is a non-setup job in the same stage - * @param {String} triggerJob Current job - * @param {String} startFrom Next job + * @param {String} currentJobName Current job + * @param {String} eventStartFrom Event StartFrom job * @param {Object} workflowGraph Workflow Graph * @return {Boolean} */ -function isStartFromMiddleOfStage(triggerJob, startFrom, workflowGraph) { - if (isStageSetup(triggerJob)) { - const startFromStageName = getStageName(workflowGraph, triggerJob); - const triggerStageName = getStageName(workflowGraph, triggerJob); +function isStartFromMiddleOfCurrentStage(currentJobName, eventStartFrom, workflowGraph) { + const startFromStageName = getStageName(workflowGraph, eventStartFrom); + const currentStageName = getStageName(workflowGraph, currentJobName); - if (!isStageSetup(startFrom) && startFromStageName === triggerStageName) { - return true; - } - } - - return false; + return isStageSetup(currentJobName) && !isStageSetup(eventStartFrom) && startFromStageName === currentStageName; } module.exports = { @@ -1127,5 +1121,5 @@ module.exports = { extractExternalJoinData, buildsToRestartFilter, trimJobName, - isStartFromMiddleOfStage + isStartFromMiddleOfCurrentStage }; diff --git a/test/plugins/builds.test.js b/test/plugins/builds.test.js index 50a305114..a4805e60c 100644 --- a/test/plugins/builds.test.js +++ b/test/plugins/builds.test.js @@ -1628,7 +1628,7 @@ describe('build plugin test', () => { }); }); - it('triggers the startFrom job after the stage setup job if the startFrom job is a non-setup stage job', () => { + it('triggers the startFrom job after the stage setup job if the startFrom job is a non-setup job in the same stage', () => { const status = 'SUCCESS'; const options = { method: 'PUT', @@ -1703,6 +1703,81 @@ describe('build plugin test', () => { }); }); + it('triggers the startFrom job after the stage setup job if the startFrom job is a non-setup job in a different stage', () => { + const status = 'SUCCESS'; + const options = { + method: 'PUT', + url: `/builds/${id}`, + auth: { + credentials: { + username: id, + scope: ['build'] + }, + strategy: ['token'] + }, + payload: { + status + } + }; + + jobMock = { + id: 1234, + name: 'stage@alpha:setup', + pipelineId, + permutations: [ + { + settings: { + email: 'foo@bar.com' + } + } + ], + pipeline: sinon.stub().resolves(pipelineMock)(), + getLatestBuild: sinon.stub().resolves(buildMock) + }; + buildMock.job = sinon.stub().resolves(jobMock)(); + buildMock.parentBuilds = { + 123: { eventId: '8888', jobs: { '~commit': 7777, C: 7778, D: 7779 } } + }; + eventMock.getBuilds.resolves([ + { + id: 1, + eventId: '8888', + jobId: 1, + status: 'FAILURE' + }, + { + id: 7777, + eventId: '8888', + jobId: 4, + status: 'SUCCESS' + }, + { + id: 7778, + eventId: '8888', + jobId: 5, + status: 'SUCCESS' + }, + { + id: 7779, + eventId: '8888', + jobId: 6, + status: 'SUCCESS' + } + ]); + + eventMock.workflowGraph = testWorkflowGraphWithStages; + eventMock.startFrom = 'beta-certify'; + eventFactoryMock.get.resolves(eventMock); + + return server.inject(options).then(reply => { + assert.equal(reply.statusCode, 200); + assert.calledWith(jobFactoryMock.get, { + name: 'alpha-deploy', + pipelineId + }); + }); + }); + it('triggers next job in the stage workflow if current is successful stage teardown and stageBuild is success', () => { const stageBuildMock = { id: 1,