From 1d5e05ab2782916f0b8a1d831c680857dd4fe3e9 Mon Sep 17 00:00:00 2001 From: Nathan Good Date: Wed, 20 Sep 2023 15:30:41 -0500 Subject: [PATCH] fix: added missing auto-sequence to tasks and fixed duplicate tasks in multidoc YAML. Signed-off-by: Nathan Good --- src/builders.ts | 49 +++++++--- .../pipelinebuilder.test.ts.snap | 95 +++++++++++++++++++ test/pipelinebuilder.test.ts | 39 +++++++- 3 files changed, 169 insertions(+), 14 deletions(-) diff --git a/src/builders.ts b/src/builders.ts index 281b868..f6e9c08 100644 --- a/src/builders.ts +++ b/src/builders.ts @@ -661,8 +661,12 @@ export class PipelineBuilder { const pipelineParams = new Map(); const pipelineWorkspaces = new Map(); const pipelineTasks: PipelineTask[] = new Array(); + // For making a list to make sure that tasks aren't duplicated when doing + // the build. Not that it really hurts anything, but it makes the multidoc + // YAML file bigger and more complex than it needs to be. + const taskList: string[] = new Array(); - this._tasks?.forEach(t => { + this._tasks?.forEach((t, i) => { const taskParams: TaskParam[] = new Array(); const taskWorkspaces: PipelineTaskWorkspace[] = new Array(); @@ -702,19 +706,20 @@ export class PipelineBuilder { }); }); - pipelineTasks.push({ - name: t.logicalID, - taskRef: { - name: t.name, - }, - params: taskParams, - workspaces: taskWorkspaces, - }); + const pt = createOrderedPipelineTask(t, ((i > 0) ? this._tasks![i - 1].logicalID : ''), taskParams, taskWorkspaces); + + pipelineTasks.push(pt); if (opts.includeDependencies) { // Build the task if the user has asked for the dependencies to be - // built along with the pipeline. - t.buildTask(); + // built along with the pipeline, but only if we haven't already + // built the task yet. + if (!taskList.find(it => { + return it == t.name; + })) { + t.buildTask(); + } + taskList.push(t.name!); } }); @@ -732,3 +737,25 @@ export class PipelineBuilder { }); } } + +function createOrderedPipelineTask(t: TaskBuilder, after: string, params: TaskParam[], ws: TaskWorkspace[]): PipelineTask { + if (after) { + return { + name: t.logicalID, + taskRef: { + name: t.name, + }, + runAfter: [after], + params: params, + workspaces: ws, + }; + } + return { + name: t.logicalID, + taskRef: { + name: t.name, + }, + params: params, + workspaces: ws, + }; +} diff --git a/test/__snapshots__/pipelinebuilder.test.ts.snap b/test/__snapshots__/pipelinebuilder.test.ts.snap index a00565c..66faf71 100644 --- a/test/__snapshots__/pipelinebuilder.test.ts.snap +++ b/test/__snapshots__/pipelinebuilder.test.ts.snap @@ -158,6 +158,98 @@ exports[`PipelineBuilderTest PipelineBuilderWithComplexTasks 1`] = ` ] `; +exports[`PipelineBuilderTest PipelineBuilderWithDuplicateTasks 1`] = ` +[ + { + "apiVersion": "tekton.dev/v1beta1", + "kind": "Task", + "metadata": { + "name": "fetch-source", + }, + "spec": { + "description": undefined, + "params": [ + { + "default": "", + "description": "", + "name": "url", + }, + ], + "steps": [], + "workspaces": [ + { + "description": "The files cloned by the task", + "name": "output", + }, + ], + }, + }, + { + "apiVersion": "tekton.dev/v1beta1", + "kind": "Pipeline", + "metadata": { + "name": "clone-build-push", + }, + "spec": { + "description": "This pipeline closes a repository, builds a Docker image, etc.", + "params": [ + { + "name": "repo-url", + "type": "string", + }, + ], + "tasks": [ + { + "name": "git-clone", + "params": [ + { + "name": "url", + "value": "$(params.repo-url)", + }, + ], + "taskRef": { + "name": "fetch-source", + }, + "workspaces": [ + { + "name": "output", + "workspace": "shared-data", + }, + ], + }, + { + "name": "git-clone", + "params": [ + { + "name": "url", + "value": "$(params.repo-url)", + }, + ], + "runAfter": [ + "git-clone", + ], + "taskRef": { + "name": "fetch-source", + }, + "workspaces": [ + { + "name": "output", + "workspace": "shared-data", + }, + ], + }, + ], + "workspaces": [ + { + "description": "The files cloned by the task", + "name": "shared-data", + }, + ], + }, + }, +] +`; + exports[`PipelineBuilderTest PipelineBuilderWithDuplicates 1`] = ` [ { @@ -201,6 +293,9 @@ exports[`PipelineBuilderTest PipelineBuilderWithDuplicates 1`] = ` "value": "$(params.repo-url)", }, ], + "runAfter": [ + "git-clone", + ], "taskRef": { "name": "print-readme", }, diff --git a/test/pipelinebuilder.test.ts b/test/pipelinebuilder.test.ts index c807975..1f35b1b 100644 --- a/test/pipelinebuilder.test.ts +++ b/test/pipelinebuilder.test.ts @@ -2,7 +2,7 @@ import { Chart, Testing } from 'cdk8s'; import { ChartProps } from 'cdk8s/lib/chart'; import { Construct } from 'constructs'; // @ts-ignore -import { Pipeline, PipelineBuilder, PipelineTaskBuilder, PipelineTaskDef, TaskRef, TaskBuilder, WorkspaceBuilder, ParameterBuilder } from '../src'; +import { ParameterBuilder, Pipeline, PipelineBuilder, PipelineTaskBuilder, PipelineTaskDef, TaskBuilder, TaskRef, WorkspaceBuilder } from '../src'; class MyTestChart extends Chart { constructor(scope: Construct, id: string, props?: ChartProps) { @@ -51,7 +51,7 @@ class MySecondTestChart extends Chart { } } -class MyTestChartWithDups extends Chart { +class MyTestChartWithDuplicateParams extends Chart { constructor(scope: Construct, id: string, props?: ChartProps) { super(scope, id, props); @@ -107,6 +107,32 @@ class MyTestChartWithStaticOverride extends Chart { } } +class MyTestChartWithDuplicateTasks extends Chart { + constructor(scope: Construct, id: string, props?: ChartProps) { + super(scope, id, props); + + const myWorkspace = new WorkspaceBuilder('output') + .withDescription('The files cloned by the task') + .withName('shared-data'); + + const urlParam = new ParameterBuilder('url') + .withPiplineParameter('repo-url', ''); + + const myTask = new TaskBuilder(this, 'git-clone') + .withName('fetch-source') + .withWorkspace(myWorkspace) + .withStringParam(urlParam) + ; + + new PipelineBuilder(this, 'my-pipeline') + .withName('clone-build-push') + .withDescription('This pipeline closes a repository, builds a Docker image, etc.') + .withTask(myTask) + .withTask(myTask) + .buildPipeline({ includeDependencies: true }); + } +} + describe('PipelineBuilderTest', () => { test('PipelineBuilder', () => { const app = Testing.app(); @@ -124,7 +150,7 @@ describe('PipelineBuilderTest', () => { test('PipelineBuilderWithDuplicates', () => { const app = Testing.app(); - const chart = new MyTestChartWithDups(app, 'test-chart'); + const chart = new MyTestChartWithDuplicateParams(app, 'test-chart'); const results = Testing.synth(chart); expect(results).toMatchSnapshot(); }); @@ -135,4 +161,11 @@ describe('PipelineBuilderTest', () => { const results = Testing.synth(chart); expect(results).toMatchSnapshot(); }); + + test('PipelineBuilderWithDuplicateTasks', () => { + const app = Testing.app(); + const chart = new MyTestChartWithDuplicateTasks(app, 'test-chart'); + const results = Testing.synth(chart); + expect(results).toMatchSnapshot(); + }); });