diff --git a/src/commands/build.ts b/src/commands/build.ts index 4bae8bce77..d3b5132a75 100644 --- a/src/commands/build.ts +++ b/src/commands/build.ts @@ -40,7 +40,7 @@ export class BuildCommand extends Command { - await ctx.addTask(new BuildTask(ctx, module, opts.force)) + await ctx.addTask(await BuildTask.factory({ ctx, module, force: opts.force })) }) ctx.log.info("") diff --git a/src/commands/deploy.ts b/src/commands/deploy.ts index d283e6ef13..f07dd56997 100644 --- a/src/commands/deploy.ts +++ b/src/commands/deploy.ts @@ -57,7 +57,7 @@ export class DeployCommand extends Command const result = await ctx.processModules(modules, watch, async (module) => { const servicesToDeploy = await module.getServices() for (const service of servicesToDeploy) { - await ctx.addTask(new DeployTask(ctx, service, force, forceBuild)) + await ctx.addTask(await DeployTask.factory({ ctx, service, force, forceBuild })) } }) diff --git a/src/commands/dev.ts b/src/commands/dev.ts index 2c928ae8cc..5f101ee850 100644 --- a/src/commands/dev.ts +++ b/src/commands/dev.ts @@ -53,7 +53,7 @@ export class DevCommand extends Command { const tasks = testTasks.concat(deployTasks) if (tasks.length === 0) { - await ctx.addTask(new BuildTask(ctx, module, false)) + await ctx.addTask(await BuildTask.factory({ ctx, module, force: false })) } else { await Bluebird.map(tasks, ctx.addTask) } diff --git a/src/commands/push.ts b/src/commands/push.ts index 41ca6d18c2..2d858fc9f7 100644 --- a/src/commands/push.ts +++ b/src/commands/push.ts @@ -72,7 +72,7 @@ export async function pushModules( ) } - const task = new PushTask(ctx, module, forceBuild) + const task = await PushTask.factory({ ctx, module, forceBuild }) await ctx.addTask(task) } diff --git a/src/commands/run/module.ts b/src/commands/run/module.ts index 29daecf798..f0cf9e73f6 100644 --- a/src/commands/run/module.ts +++ b/src/commands/run/module.ts @@ -64,7 +64,7 @@ export class RunModuleCommand extends Command { await ctx.configureEnvironment() - const buildTask = new BuildTask(ctx, module, opts["force-build"]) + const buildTask = await BuildTask.factory({ ctx, module, force: opts["force-build"] }) await ctx.addTask(buildTask) await ctx.processTasks() diff --git a/src/commands/run/service.ts b/src/commands/run/service.ts index 5496d8b539..258a933781 100644 --- a/src/commands/run/service.ts +++ b/src/commands/run/service.ts @@ -51,7 +51,7 @@ export class RunServiceCommand extends Command { await ctx.configureEnvironment() - const buildTask = new BuildTask(ctx, module, opts["force-build"]) + const buildTask = await BuildTask.factory({ ctx, module, force: opts["force-build"] }) await ctx.addTask(buildTask) await ctx.processTasks() diff --git a/src/commands/run/test.ts b/src/commands/run/test.ts index 693468782a..ed93c57ed4 100644 --- a/src/commands/run/test.ts +++ b/src/commands/run/test.ts @@ -71,7 +71,7 @@ export class RunTestCommand extends Command { await ctx.configureEnvironment() - const buildTask = new BuildTask(ctx, module, opts["force-build"]) + const buildTask = await BuildTask.factory({ ctx, module, force: opts["force-build"] }) await ctx.addTask(buildTask) await ctx.processTasks() diff --git a/src/commands/test.ts b/src/commands/test.ts index 306729b446..0bb722297c 100644 --- a/src/commands/test.ts +++ b/src/commands/test.ts @@ -57,7 +57,7 @@ export class TestCommand extends Command { const results = await ctx.processModules(modules, opts.watch, async (module) => { const tasks = await module.getTestTasks({ group, force, forceBuild }) - await Bluebird.map(tasks, ctx.addTask) + return Bluebird.map(tasks, ctx.addTask) }) const failed = values(results).filter(r => !!r.error).length diff --git a/src/plugin-context.ts b/src/plugin-context.ts index b1b74a9856..0aba0daf92 100644 --- a/src/plugin-context.ts +++ b/src/plugin-context.ts @@ -6,6 +6,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import Bluebird = require("bluebird") import chalk from "chalk" import { Stream } from "ts-stream" import { NotFoundError } from "./exceptions" @@ -48,7 +49,6 @@ import { RuntimeContext, ServiceStatus, } from "./types/service" -import Bluebird = require("bluebird") import { mapValues, toPairs, @@ -367,10 +367,10 @@ export function createPluginContext(garden: Garden): PluginContext { deployServices: async ({ names, force = false, forceBuild = false, logEntry }) => { const services = await ctx.getServices(names) - for (const service of services) { - const task = new DeployTask(ctx, service, force, forceBuild, logEntry) + await Bluebird.map(services, async (service) => { + const task = await DeployTask.factory({ ctx, service, force, forceBuild, logEntry }) await ctx.addTask(task) - } + }) return ctx.processTasks() }, diff --git a/src/task-graph.ts b/src/task-graph.ts index aa3d3da113..34d42430f9 100644 --- a/src/task-graph.ts +++ b/src/task-graph.ts @@ -8,7 +8,7 @@ import * as Bluebird from "bluebird" import chalk from "chalk" -import { pick } from "lodash" +import { merge, pick } from "lodash" import { Task, TaskDefinitionError } from "./types/task" import { EntryStyle, LogSymbolType } from "./logger/types" @@ -55,12 +55,14 @@ export class TaskGraph { private inProgress: TaskNodeMap private logEntryMap: LogEntryMap + private resultCache: ResultCache private opQueue: OperationQueue constructor(private ctx: PluginContext, private concurrency: number = DEFAULT_CONCURRENCY) { this.roots = new TaskNodeMap() this.index = new TaskNodeMap() this.inProgress = new TaskNodeMap() + this.resultCache = new ResultCache() this.opQueue = new OperationQueue(this) this.logEntryMap = {} } @@ -70,7 +72,6 @@ export class TaskGraph { } async addTaskInternal(task: Task) { - // TODO: Detect circular dependencies. const predecessor = this.getPredecessor(task) let node = this.getNode(task) @@ -137,6 +138,7 @@ export class TaskGraph { this.initLogging() return Bluebird.map(batch, async (node: TaskNode) => { + const task = node.task const type = node.getType() const baseKey = node.getBaseKey() const description = node.getDescription() @@ -145,15 +147,21 @@ export class TaskGraph { this.logTask(node) this.logEntryMap.inProgress.setState(inProgressToStr(this.inProgress.getNodes())) - const dependencyBaseKeys = (await node.task.getDependencies()) - .map(task => task.getBaseKey()) + const dependencyBaseKeys = (await task.getDependencies()) + .map(dep => dep.getBaseKey()) - const dependencyResults = pick(results, dependencyBaseKeys) + const dependencyResults = merge( + this.resultCache.pick(dependencyBaseKeys), + pick(results, dependencyBaseKeys)) + let result try { - results[baseKey] = await node.process(dependencyResults) + result = await node.process(dependencyResults) } catch (error) { - results[baseKey] = { type, description, error } + result = { type, description, error } + } finally { + results[baseKey] = result + this.resultCache.put(baseKey, task.version.versionString, result) } } finally { this.completeTask(node) @@ -197,9 +205,15 @@ export class TaskGraph { private async addDependencies(node: TaskNode) { const task = node.task for (const d of await task.getDependencies()) { + + if (this.resultCache.get(d.getBaseKey(), d.version.versionString)) { + continue + } + const dependency = this.getPredecessor(d) || this.getNode(d) this.index.addNode(dependency) node.addDependency(dependency) + } } @@ -390,6 +404,56 @@ class TaskNode { } } +interface CachedResult { + result: TaskResult, + versionString: string +} + +class ResultCache { + /* + By design, at most one TaskResult (the most recently processed) is cached for a given baseKey. + + Invariant: No concurrent calls are made to this class' instance methods, since they + only happen within TaskGraph's addTaskInternal and processTasksInternal methods, + which are never executed concurrently, since they are executed sequentially by the + operation queue. + */ + private cache: { [key: string]: CachedResult } + + constructor() { + this.cache = {} + } + + put(baseKey: string, versionString: string, result: TaskResult): void { + this.cache[baseKey] = { result, versionString } + } + + get(baseKey: string, versionString: string): TaskResult | null { + const r = this.cache[baseKey] + return (r && r.versionString === versionString && !r.result.error) ? r.result : null + } + + getNewest(baseKey: string): TaskResult | null { + const r = this.cache[baseKey] + return (r && !r.result.error) ? r.result : null + } + + // Returns newest cached results, if any, for baseKeys + pick(baseKeys: string[]): TaskResults { + const results: TaskResults = {} + + for (const baseKey of baseKeys) { + const cachedResult = this.getNewest(baseKey) + if (cachedResult) { + results[baseKey] = cachedResult + } + } + + return results + } + +} + // TODO: Add more typing to this class. /* diff --git a/src/tasks/build.ts b/src/tasks/build.ts index bd3ee0a9ee..9fd0dbca80 100644 --- a/src/tasks/build.ts +++ b/src/tasks/build.ts @@ -6,24 +6,49 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import * as Bluebird from "bluebird" import chalk from "chalk" import { round } from "lodash" import { PluginContext } from "../plugin-context" import { Module } from "../types/module" import { EntryStyle } from "../logger/types" import { BuildResult } from "../types/plugin" -import { Task } from "../types/task" +import { Task, TaskParams, TaskVersion } from "../types/task" -export class BuildTask extends Task { +export interface BuildTaskParams extends TaskParams { + ctx: PluginContext + module: Module + force: boolean +} + +export class BuildTask extends Task { type = "build" - constructor(private ctx: PluginContext, private module: T, private force: boolean) { - super() + private ctx: PluginContext + private module: Module + private force: boolean + + constructor(initArgs: BuildTaskParams & TaskVersion) { + super(initArgs) + this.ctx = initArgs.ctx + this.module = initArgs.module + this.force = initArgs.force } - async getDependencies() { + /* + TODO: Replace with a generic factory method on the Task class to avoid repetition. This applies equally to other + child classes of Task that implement an equivalent factory method. + */ + static async factory(initArgs: BuildTaskParams): Promise { + initArgs.version = await initArgs.module.getVersion() + return new BuildTask(initArgs) + } + + async getDependencies(): Promise { const deps = await this.module.getBuildDependencies() - return deps.map((m: M) => new BuildTask(this.ctx, m, this.force)) + return Bluebird.map(deps, async (m: Module) => { + return BuildTask.factory({ ctx: this.ctx, module: m, force: this.force }) + }) } protected getName() { diff --git a/src/tasks/deploy.ts b/src/tasks/deploy.ts index 6a1e70f83c..ae4fb69457 100644 --- a/src/tasks/deploy.ts +++ b/src/tasks/deploy.ts @@ -6,37 +6,62 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import * as Bluebird from "bluebird" +import chalk from "chalk" import { LogEntry } from "../logger" import { PluginContext } from "../plugin-context" import { BuildTask } from "./build" -import { Task } from "../types/task" +import { Task, TaskParams, TaskVersion } from "../types/task" import { Service, ServiceStatus, } from "../types/service" import { EntryStyle } from "../logger/types" -import chalk from "chalk" -export class DeployTask> extends Task { +export interface DeployTaskParams extends TaskParams { + ctx: PluginContext + service: Service + force: boolean + forceBuild: boolean + logEntry?: LogEntry +} + +export class DeployTask extends Task { type = "deploy" - constructor( - private ctx: PluginContext, - private service: T, - private force: boolean, - private forceBuild: boolean, - private logEntry?: LogEntry) { - super() + private ctx: PluginContext + private service: Service + private force: boolean + private forceBuild: boolean + private logEntry?: LogEntry + + constructor(initArgs: DeployTaskParams & TaskVersion) { + super(initArgs) + this.ctx = initArgs.ctx + this.service = initArgs.service + this.force = initArgs.force + this.forceBuild = initArgs.forceBuild + this.logEntry = initArgs.logEntry + } + + static async factory(initArgs: DeployTaskParams): Promise { + initArgs.version = await initArgs.service.module.getVersion() + return new DeployTask(initArgs) } async getDependencies() { const serviceDeps = this.service.config.dependencies const services = await this.ctx.getServices(serviceDeps) - const deps: Task[] = services.map((s) => { - return new DeployTask(this.ctx, s, this.force, this.forceBuild) + const deps: Task[] = await Bluebird.map(services, async (service) => { + return DeployTask.factory({ + service, + ctx: this.ctx, + force: this.force, + forceBuild: this.forceBuild, + }) }) - deps.push(new BuildTask(this.ctx, this.service.module, this.forceBuild)) + deps.push(await BuildTask.factory({ ctx: this.ctx, module: this.service.module, force: this.forceBuild })) return deps } diff --git a/src/tasks/push.ts b/src/tasks/push.ts index 637c5aad03..bb34c1625a 100644 --- a/src/tasks/push.ts +++ b/src/tasks/push.ts @@ -12,23 +12,38 @@ import { BuildTask } from "./build" import { Module } from "../types/module" import { EntryStyle } from "../logger/types" import { PushResult } from "../types/plugin" -import { Task } from "../types/task" +import { Task, TaskParams, TaskVersion } from "../types/task" -export class PushTask> extends Task { +export interface PushTaskParams extends TaskParams { + ctx: PluginContext + module: Module + forceBuild: boolean +} + +export class PushTask extends Task { type = "push" - constructor( - private ctx: PluginContext, - private module: T, - private forceBuild: boolean) { - super() + private ctx: PluginContext + private module: Module + private forceBuild: boolean + + constructor(initArgs: PushTaskParams & TaskVersion) { + super(initArgs) + this.ctx = initArgs.ctx + this.module = initArgs.module + this.forceBuild = initArgs.forceBuild + } + + static async factory(initArgs: PushTaskParams): Promise { + initArgs.version = await initArgs.module.getVersion() + return new PushTask(initArgs) } async getDependencies() { if (!(await this.module.getConfig()).allowPush) { return [] } - return [new BuildTask(this.ctx, this.module, this.forceBuild)] + return [await BuildTask.factory({ ctx: this.ctx, module: this.module, force: this.forceBuild })] } getName() { diff --git a/src/tasks/test.ts b/src/tasks/test.ts index 581776b206..acb132ada1 100644 --- a/src/tasks/test.ts +++ b/src/tasks/test.ts @@ -6,14 +6,15 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import * as Bluebird from "bluebird" +import chalk from "chalk" import { PluginContext } from "../plugin-context" import { Module, TestSpec } from "../types/module" import { BuildTask } from "./build" import { DeployTask } from "./deploy" import { TestResult } from "../types/plugin" -import { Task } from "../types/task" +import { Task, TaskParams, TaskVersion } from "../types/task" import { EntryStyle } from "../logger/types" -import chalk from "chalk" class TestError extends Error { toString() { @@ -21,15 +22,35 @@ class TestError extends Error { } } -export class TestTask extends Task { +export interface TestTaskParams extends TaskParams { + ctx: PluginContext + module: Module + testSpec: TestSpec + force: boolean + forceBuild: boolean +} + +export class TestTask extends Task { type = "test" - constructor( - private ctx: PluginContext, - private module: T, private testSpec: TestSpec, - private force: boolean, private forceBuild: boolean, - ) { - super() + private ctx: PluginContext + private module: Module + private testSpec: TestSpec + private force: boolean + private forceBuild: boolean + + constructor(initArgs: TestTaskParams & TaskVersion) { + super(initArgs) + this.ctx = initArgs.ctx + this.module = initArgs.module + this.testSpec = initArgs.testSpec + this.force = initArgs.force + this.forceBuild = initArgs.forceBuild + } + + static async factory(initArgs: TestTaskParams): Promise { + initArgs.version = await initArgs.module.getVersion() + return new TestTask(initArgs) } async getDependencies() { @@ -39,15 +60,23 @@ export class TestTask extends Task { return [] } - const deps: Task[] = [new BuildTask(this.ctx, this.module, this.forceBuild)] - const services = await this.ctx.getServices(this.testSpec.dependencies) + const deps: Promise[] = [BuildTask.factory({ + ctx: this.ctx, module: this.module, + force: this.forceBuild, + })] + for (const service of services) { - deps.push(new DeployTask(this.ctx, service, false, this.forceBuild)) + deps.push(DeployTask.factory({ + service, + ctx: this.ctx, + force: false, + forceBuild: this.forceBuild, + })) } - return deps + return Bluebird.all(deps) } getName() { diff --git a/src/types/module.ts b/src/types/module.ts index 69c30854c9..18e477de38 100644 --- a/src/types/module.ts +++ b/src/types/module.ts @@ -6,6 +6,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import * as Bluebird from "bluebird" import { existsSync, readFileSync, @@ -26,7 +27,6 @@ import { validate, } from "./common" import { ConfigurationError } from "../exceptions" -import Bluebird = require("bluebird") import { extend, set, @@ -237,27 +237,35 @@ export class Module { async getDeployTasks( { force = false, forceBuild = false }: { force?: boolean, forceBuild?: boolean }, - ): Promise[]> { + ): Promise { const services = await this.getServices() const module = this - return services.map(s => new DeployTask(module.ctx, s, force, forceBuild)) + return Bluebird.map(services, async (service) => { + return DeployTask.factory({ ctx: module.ctx, service, force, forceBuild }) + }) } async getTestTasks( { group, force = false, forceBuild = false }: { group?: string, force?: boolean, forceBuild?: boolean }, ) { - const tasks: TestTask>[] = [] + const tasks: Promise[] = [] const config = await this.getConfig() for (const test of config.test) { if (group && test.name !== group) { continue } - tasks.push(new TestTask>(this.ctx, this, test, force, forceBuild)) + tasks.push(TestTask.factory({ + force, + forceBuild, + testSpec: test, + ctx: this.ctx, + module: this, + })) } - return tasks + return Bluebird.all(tasks) } async prepareRuntimeContext(dependencies: Service[], extraEnvVars: PrimitiveMap = {}): Promise { diff --git a/src/types/task.ts b/src/types/task.ts index 9e11d53258..48c20c2632 100644 --- a/src/types/task.ts +++ b/src/types/task.ts @@ -7,19 +7,30 @@ */ import { TaskResults } from "../task-graph" +import { TreeVersion } from "../vcs/base" import { v1 as uuidv1 } from "uuid" export class TaskDefinitionError extends Error { } +export interface TaskVersion { + version: TreeVersion +} + +export interface TaskParams { + version?: TreeVersion +} + export abstract class Task { abstract type: string id: string + version: TreeVersion dependencies: Task[] - constructor() { + constructor(initArgs: TaskParams & TaskVersion) { this.dependencies = [] this.id = uuidv1() // uuidv1 is timestamp-based + this.version = initArgs.version } async getDependencies(): Promise { @@ -40,10 +51,3 @@ export abstract class Task { abstract async process(dependencyResults: TaskResults): Promise } - -// Ensures that the task's version has been computed before it is used further. -export async function makeTask(taskClass, initArgs) { - const task = new taskClass(...initArgs) - await task.getVersion() - return task -} diff --git a/test/src/build-dir.ts b/test/src/build-dir.ts index 240019ec0d..9132ca538b 100644 --- a/test/src/build-dir.ts +++ b/test/src/build-dir.ts @@ -1,3 +1,4 @@ +import * as Bluebird from "bluebird" const nodetree = require("nodetree") import { join } from "path" import { pathExists, readdir } from "fs-extra" @@ -69,9 +70,11 @@ describe("BuildDir", () => { await garden.clearBuilds() const modules = await garden.getModules() - for (const module of modules) { - await garden.addTask(new BuildTask(garden.pluginContext, module, false)) - } + await Bluebird.map(modules, async (module) => { + return garden.addTask(await BuildTask.factory({ + module, ctx: garden.pluginContext, force: false, + })) + }) await garden.processTasks() const modulesByName = keyBy(modules, "name") diff --git a/test/src/task-graph.ts b/test/src/task-graph.ts index 598be9357b..251c7cec72 100644 --- a/test/src/task-graph.ts +++ b/test/src/task-graph.ts @@ -21,7 +21,13 @@ class TestTask extends Task { private callback?: (name: string, result: any) => Promise, id: string = "", ) { - super() + super({ + version: { + versionString: "12345#6789", + latestCommit: "12345", + dirtyTimestamp: 6789, + }, + }) this.name = name this.id = id @@ -42,6 +48,10 @@ class TestTask extends Task { return this.id ? `${this.name}.${this.id}` : this.name } + async getVersion() { + return this.version + } + getDescription() { return this.getKey() } diff --git a/test/src/tasks/deploy.ts b/test/src/tasks/deploy.ts index eaf6fe8748..8974ba1349 100644 --- a/test/src/tasks/deploy.ts +++ b/test/src/tasks/deploy.ts @@ -23,7 +23,7 @@ describe("DeployTask", () => { const serviceA = await ctx.getService("service-a") const serviceB = await ctx.getService("service-b") - const task = new DeployTask(ctx, serviceB, false, false) + const task = await DeployTask.factory({ctx, service: serviceB, force: false, forceBuild: false}) let actionParams: any = {} stubModuleAction(