Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(3143): skip jobs between stage setup and startFrom stage job only if they belong to the same stage #3166

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -123,8 +123,8 @@
"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-workflow-parser": "^4.1.1",
"screwdriver-template-validator": "^8.2.0",
sagar1312 marked this conversation as resolved.
Show resolved Hide resolved
"screwdriver-workflow-parser": "^4.4.1",
"sqlite3": "^5.1.4",
"stream": "0.0.3",
"tinytim": "^0.1.1",
Expand Down
4 changes: 2 additions & 2 deletions plugins/builds/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ const {
buildsToRestartFilter,
trimJobName,
getParallelBuilds,
isStartFromMiddleOfStage,
isStartFromMiddleOfCurrentStage,
Status
} = require('./triggers/helpers');

Expand Down Expand Up @@ -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,
Expand Down
26 changes: 14 additions & 12 deletions plugins/builds/triggers/helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -1074,26 +1073,29 @@ 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
* @param {String} triggerJob Current job
* @param {String} startFrom Next 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} currentJobName Current job
* @param {String} eventStartFrom Event StartFrom job
* @param {Object} workflowGraph Workflow Graph
* @return {Boolean}
*/
function isStartFromMiddleOfStage(triggerJob, startFrom, workflowGraph) {
return isStageSetup(triggerJob) && !isStageSetup(startFrom) && isStageJob(workflowGraph, startFrom);
function isStartFromMiddleOfCurrentStage(currentJobName, eventStartFrom, workflowGraph) {
const startFromStageName = getStageName(workflowGraph, eventStartFrom);
const currentStageName = getStageName(workflowGraph, currentJobName);

return isStageSetup(currentJobName) && !isStageSetup(eventStartFrom) && startFromStageName === currentStageName;
}

module.exports = {
Expand All @@ -1119,5 +1121,5 @@ module.exports = {
extractExternalJoinData,
buildsToRestartFilter,
trimJobName,
isStartFromMiddleOfStage
isStartFromMiddleOfCurrentStage
};
77 changes: 76 additions & 1 deletion test/plugins/builds.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -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: '[email protected]'
}
}
],
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,
Expand Down