Skip to content

Commit 26068e4

Browse files
yakanechiyakanechi
authored
fix: Reduce the number of fetches for job templates (#184)
1 parent 863d31d commit 26068e4

11 files changed

Lines changed: 343 additions & 45 deletions

‎lib/phase/flatten.js‎

Lines changed: 47 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -352,8 +352,8 @@ function handleMergeSharedStepsAnnotation({ sharedConfig, jobConfig, template })
352352
* Retrieve template and merge into job config
353353
*
354354
* @method mergeTemplateIntoJob
355-
* @param {String} jobName Job name
356-
* @param {Object} jobConfig Job config
355+
* @param {String[]} jobNames Job names
356+
* @param {Object[]} jobConfigs Job configs
357357
* @param {Object} newJobs Object with all the jobs
358358
* @param {TemplateFactory} templateFactory Template Factory to get templates
359359
* @param {Object} sharedConfig Shared configuration
@@ -362,23 +362,26 @@ function handleMergeSharedStepsAnnotation({ sharedConfig, jobConfig, template })
362362
* @return {Promise}
363363
*/
364364
async function mergeTemplateIntoJob({
365-
jobName,
366-
jobConfig,
365+
jobNames,
366+
jobConfigs,
367367
newJobs,
368368
templateFactory,
369369
sharedConfig,
370370
pipelineParameters,
371371
isPipelineTemplate
372372
}) {
373-
let oldJob = jobConfig;
373+
const template = await templateFactory.getTemplate(jobConfigs[0].template);
374374

375375
// Try to get the template
376-
return templateFactory.getTemplate(jobConfig.template).then(template => {
376+
return jobNames.map((jobName, i) => {
377+
const jobConfig = jobConfigs[i];
378+
let oldJob = jobConfig;
379+
377380
if (!template) {
378381
throw new Error(`Template ${jobConfig.template} does not exist`);
379382
}
380383

381-
const newJob = template.config;
384+
const newJob = clone(template.config);
382385
const environment = newJob.environment || {};
383386

384387
// Construct full template name
@@ -453,52 +456,60 @@ async function mergeTemplateIntoJob({
453456
*/
454457
async function flattenTemplates(doc, templateFactory, isPipelineTemplate) {
455458
const newJobs = {};
456-
const templates = [];
459+
const orderWarnings = [];
457460
const { jobs, shared, parameters } = doc;
458-
const jobNames = Object.keys(jobs);
461+
const allJobNames = Object.keys(jobs);
459462

460-
jobNames.forEach(jobName => {
461-
const jobConfig = clone(jobs[jobName]);
462-
const templateConfig = jobConfig.template;
463-
const order = jobConfig.order || [];
464-
let warnMessages = [];
463+
const jobGroupByTemplate = {};
465464

466-
// Validate order is used with template
467-
if (order.length > 0 && templateConfig === undefined) {
468-
warnMessages = warnMessages.concat(`"order" in ${jobName} job cannot be used without "template"`);
465+
allJobNames.forEach(jobName => {
466+
const templateName = jobs[jobName].template;
469467

470-
templates.push(warnMessages);
471-
}
468+
if (!templateName) {
469+
const jobConfig = clone(jobs[jobName]);
470+
const order = jobConfig.order || [];
472471

473-
// If template is specified, then merge
474-
if (templateConfig) {
475-
templates.push(
476-
mergeTemplateIntoJob({
477-
jobName,
478-
jobConfig,
479-
newJobs,
480-
templateFactory,
481-
sharedConfig: shared,
482-
pipelineParameters: parameters,
483-
isPipelineTemplate
484-
})
485-
);
472+
// Validate order is used with template
473+
if (order.length > 0 && templateName === undefined) {
474+
orderWarnings.push(`"order" in ${jobName} job cannot be used without "template"`);
475+
}
476+
newJobs[jobName] = jobConfig;
477+
478+
return;
479+
}
480+
if (jobGroupByTemplate[templateName] === undefined) {
481+
jobGroupByTemplate[templateName] = [jobName];
486482
} else {
487-
newJobs[jobName] = jobConfig; // Otherwise just use jobConfig
483+
jobGroupByTemplate[templateName].push(jobName);
488484
}
489485
});
490486

487+
const promises = Object.values(jobGroupByTemplate).map(jobNames => {
488+
const jobConfigs = jobNames.map(jobName => clone(jobs[jobName]));
489+
490+
// If template is specified, then merge
491+
return mergeTemplateIntoJob({
492+
jobNames,
493+
jobConfigs,
494+
newJobs,
495+
templateFactory,
496+
sharedConfig: shared,
497+
pipelineParameters: parameters,
498+
isPipelineTemplate
499+
});
500+
});
501+
491502
// Wait until all promises are resolved
492503
// Inserting newJobs with template is delayed, hence the order of newJobs.keys is incompatible to doc.jobs.keys
493-
return Promise.all(templates).then(warnings => {
504+
return Promise.all(promises).then(warnings => {
494505
const sortedNewJobs = {};
495506

496-
jobNames.forEach(jobName => {
507+
allJobNames.forEach(jobName => {
497508
sortedNewJobs[jobName] = newJobs[jobName];
498509
});
499510

500511
return {
501-
warnings: [].concat(...warnings),
512+
warnings: [].concat(...orderWarnings, ...warnings.flat()),
502513
newJobs: sortedNewJobs
503514
};
504515
});

‎test/data/basic-job-with-parameters-and-template-without-parameters.json‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
],
1616
"description": "This is the main description",
1717
"environment": {
18+
"BAR": "foo",
19+
"FOO": "from yourtemplate",
1820
"SD_TEMPLATE_FULLNAME": "yourtemplate",
1921
"SD_TEMPLATE_NAME": "yourtemplate",
2022
"SD_TEMPLATE_NAMESPACE": "",

‎test/data/basic-job-with-same-template-different-version.json‎

Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
{
2+
"annotations": {},
3+
"parameters": {},
4+
"jobs": {
5+
"main": [{
6+
"annotations": {},
7+
"image": "node:4",
8+
"commands": [
9+
{
10+
"name": "install",
11+
"command": "npm install"
12+
},
13+
{
14+
"name": "test",
15+
"command": "npm test"
16+
}
17+
],
18+
"environment": {
19+
"FOO": "overwritten by job",
20+
"BAR": "foo",
21+
"SD_TEMPLATE_FULLNAME": "mytemplate",
22+
"SD_TEMPLATE_NAME": "mytemplate",
23+
"SD_TEMPLATE_NAMESPACE": "",
24+
"SD_TEMPLATE_VERSION": "1.2.3"
25+
},
26+
"secrets": [
27+
"GIT_KEY"
28+
],
29+
"settings": {
30+
"email": "foo@example.com"
31+
},
32+
"requires": [
33+
"~pr",
34+
"~commit"
35+
],
36+
"templateId": 7754
37+
}],
38+
"publish1": [{
39+
"annotations": {},
40+
"image": "node:4",
41+
"commands": [
42+
{
43+
"name": "publish",
44+
"command": "npm publish"
45+
}
46+
],
47+
"environment": {
48+
"BAR": "foo",
49+
"FOO": "from yourtemplate",
50+
"SD_TEMPLATE_FULLNAME": "yourtemplate",
51+
"SD_TEMPLATE_NAME": "yourtemplate",
52+
"SD_TEMPLATE_NAMESPACE": "",
53+
"SD_TEMPLATE_VERSION": "2"
54+
},
55+
"secrets": [],
56+
"settings": {},
57+
"requires": [
58+
"main"
59+
],
60+
"templateId": 1234
61+
}],
62+
"publish2": [{
63+
"annotations": {},
64+
"image": "node:4",
65+
"commands": [
66+
{
67+
"name": "publish",
68+
"command": "npm publish"
69+
}
70+
],
71+
"environment": {
72+
"SD_TEMPLATE_FULLNAME": "yourtemplate",
73+
"SD_TEMPLATE_NAME": "yourtemplate",
74+
"SD_TEMPLATE_NAMESPACE": "",
75+
"SD_TEMPLATE_VERSION": "3"
76+
},
77+
"secrets": [],
78+
"settings": {},
79+
"requires": [
80+
"main"
81+
],
82+
"templateId": 1234
83+
}]
84+
},
85+
"workflowGraph": {
86+
"nodes": [
87+
{ "name": "~pr" },
88+
{ "name": "~commit" },
89+
{ "name": "main" },
90+
{ "name": "publish1" },
91+
{ "name": "publish2" }
92+
],
93+
"edges": [
94+
{ "src": "~pr", "dest": "main" },
95+
{ "src": "~commit", "dest": "main" },
96+
{ "src": "main", "dest": "publish1", "join": true },
97+
{ "src": "main", "dest": "publish2", "join": true }
98+
]
99+
},
100+
"subscribe": {}
101+
}

‎test/data/basic-job-with-same-template-different-version.yaml‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
jobs:
2+
main:
3+
template: mytemplate@1.2.3
4+
environment:
5+
FOO: 'overwritten by job'
6+
requires:
7+
- ~pr
8+
- ~commit
9+
publish1:
10+
template: yourtemplate@2
11+
requires:
12+
- main
13+
14+
publish2:
15+
template: yourtemplate@3
16+
requires:
17+
- main

‎test/data/basic-job-with-same-template.json‎

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,105 @@
1+
{
2+
"annotations": {},
3+
"parameters": {},
4+
"jobs": {
5+
"main": [{
6+
"annotations": {},
7+
"image": "node:4",
8+
"commands": [
9+
{
10+
"name": "install",
11+
"command": "npm install"
12+
},
13+
{
14+
"name": "test",
15+
"command": "npm test"
16+
}
17+
],
18+
"environment": {
19+
"FOO": "overwritten by job",
20+
"BAR": "foo",
21+
"SD_TEMPLATE_FULLNAME": "mytemplate",
22+
"SD_TEMPLATE_NAME": "mytemplate",
23+
"SD_TEMPLATE_NAMESPACE": "",
24+
"SD_TEMPLATE_VERSION": "1.2.3"
25+
},
26+
"secrets": [
27+
"GIT_KEY"
28+
],
29+
"settings": {
30+
"email": "foo@example.com"
31+
},
32+
"requires": [
33+
"~pr",
34+
"~commit"
35+
],
36+
"templateId": 7754
37+
}],
38+
"publish1": [{
39+
"annotations": {},
40+
"image": "node:4",
41+
"commands": [
42+
{
43+
"name": "publish",
44+
"command": "npm publish"
45+
}
46+
],
47+
"environment": {
48+
"FOO": "from yourtemplate",
49+
"BAR": "bar1",
50+
"BAZ": "baz1",
51+
"SD_TEMPLATE_FULLNAME": "yourtemplate",
52+
"SD_TEMPLATE_NAME": "yourtemplate",
53+
"SD_TEMPLATE_NAMESPACE": "",
54+
"SD_TEMPLATE_VERSION": "2"
55+
},
56+
"secrets": [],
57+
"settings": {},
58+
"requires": [
59+
"main"
60+
],
61+
"templateId": 1234
62+
}],
63+
"publish2": [{
64+
"annotations": {},
65+
"image": "node:4",
66+
"commands": [
67+
{
68+
"name": "publish",
69+
"command": "npm publish"
70+
}
71+
],
72+
"environment": {
73+
"FOO": "from yourtemplate",
74+
"BAR": "foo",
75+
"BAZ": "baz2",
76+
"SD_TEMPLATE_FULLNAME": "yourtemplate",
77+
"SD_TEMPLATE_NAME": "yourtemplate",
78+
"SD_TEMPLATE_NAMESPACE": "",
79+
"SD_TEMPLATE_VERSION": "2"
80+
},
81+
"secrets": [],
82+
"settings": {},
83+
"requires": [
84+
"main"
85+
],
86+
"templateId": 1234
87+
}]
88+
},
89+
"workflowGraph": {
90+
"nodes": [
91+
{ "name": "~pr" },
92+
{ "name": "~commit" },
93+
{ "name": "main" },
94+
{ "name": "publish1" },
95+
{ "name": "publish2" }
96+
],
97+
"edges": [
98+
{ "src": "~pr", "dest": "main" },
99+
{ "src": "~commit", "dest": "main" },
100+
{ "src": "main", "dest": "publish1", "join": true },
101+
{ "src": "main", "dest": "publish2", "join": true }
102+
]
103+
},
104+
"subscribe": {}
105+
}

‎test/data/basic-job-with-same-template.yaml‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
jobs:
2+
main:
3+
template: mytemplate@1.2.3
4+
environment:
5+
FOO: 'overwritten by job'
6+
requires:
7+
- ~pr
8+
- ~commit
9+
publish1:
10+
template: yourtemplate@2
11+
environment:
12+
BAR: "bar1"
13+
BAZ: "baz1"
14+
requires:
15+
- main
16+
17+
publish2:
18+
template: yourtemplate@2
19+
environment:
20+
BAZ: "baz2"
21+
requires:
22+
- main

0 commit comments

Comments
 (0)