From 9b35d744752147cf598aaf2a01a028890e16717a Mon Sep 17 00:00:00 2001 From: Thorarinn Sigurdsson Date: Wed, 19 Feb 2020 12:43:42 +0100 Subject: [PATCH] fix(core): fix to task deps for DeployTask `DeployTask`'s `getDependencies` method now always returns task dependencies initialized with `force = false`. The underlying semantic is that unless the task dependency's module sources have changed, there's no need to re-run them when force-deploying the dependant service. I.e. the decision to run or not run the task dependency is delegated to the `TaskTask` in the normal way. --- garden-service/src/commands/dev.ts | 75 +++++-- garden-service/src/tasks/deploy.ts | 2 +- garden-service/test/unit/src/commands/dev.ts | 41 +++- garden-service/test/unit/src/tasks/deploy.ts | 211 ++++++++++++------- 4 files changed, 222 insertions(+), 107 deletions(-) diff --git a/garden-service/src/commands/dev.ts b/garden-service/src/commands/dev.ts index a24c4b56c7..d42ffdb871 100644 --- a/garden-service/src/commands/dev.ts +++ b/garden-service/src/commands/dev.ts @@ -34,6 +34,8 @@ import { getHotReloadServiceNames, validateHotReloadServiceNames } from "./helpe import { GardenServer, startServer } from "../server/server" import { BuildTask } from "../tasks/build" import { DeployTask } from "../tasks/deploy" +import { Garden } from "../garden" +import { LogEntry } from "../logger/log-entry" const ansiBannerPath = join(STATIC_DIR, "garden-banner-2.txt") @@ -184,34 +186,15 @@ export class DevCommand extends Command { watch: true, initialTasks, changeHandler: async (updatedGraph: ConfigGraph, module: Module) => { - const tasks = await getModuleWatchTasks({ + return getDevCommandWatchTasks({ garden, log, - graph: updatedGraph, + updatedGraph, module, hotReloadServiceNames, + testNames: opts["test-names"], + skipTests, }) - - if (!skipTests) { - const filterNames = opts["test-names"] - const testModules: Module[] = await updatedGraph.withDependantModules([module]) - tasks.push( - ...flatten( - await Bluebird.map(testModules, (m) => - getTestTasks({ - garden, - log, - module: m, - graph: updatedGraph, - filterNames, - hotReloadServiceNames, - }) - ) - ) - ) - } - - return tasks }, }) @@ -219,6 +202,52 @@ export class DevCommand extends Command { } } +export async function getDevCommandWatchTasks({ + garden, + log, + updatedGraph, + module, + hotReloadServiceNames, + testNames, + skipTests, +}: { + garden: Garden + log: LogEntry + updatedGraph: ConfigGraph + module: Module + hotReloadServiceNames: string[] + testNames: string[] | undefined + skipTests: boolean +}) { + const tasks = await getModuleWatchTasks({ + garden, + log, + graph: updatedGraph, + module, + hotReloadServiceNames, + }) + + if (!skipTests) { + const testModules: Module[] = await updatedGraph.withDependantModules([module]) + tasks.push( + ...flatten( + await Bluebird.map(testModules, (m) => + getTestTasks({ + garden, + log, + module: m, + graph: updatedGraph, + filterNames: testNames, + hotReloadServiceNames, + }) + ) + ) + ) + } + + return tasks +} + function getGreetingTime() { const m = moment() diff --git a/garden-service/src/tasks/deploy.ts b/garden-service/src/tasks/deploy.ts index 92d9747b43..ac2d621309 100644 --- a/garden-service/src/tasks/deploy.ts +++ b/garden-service/src/tasks/deploy.ts @@ -124,7 +124,7 @@ export class DeployTask extends BaseTask { garden: this.garden, log: this.log, graph: this.graph, - force: this.force, + force: false, forceBuild: this.forceBuild, }) }) diff --git a/garden-service/test/unit/src/commands/dev.ts b/garden-service/test/unit/src/commands/dev.ts index 650bf2bd9e..b7e1d17f01 100644 --- a/garden-service/test/unit/src/commands/dev.ts +++ b/garden-service/test/unit/src/commands/dev.ts @@ -8,7 +8,7 @@ import pEvent from "p-event" import { expect } from "chai" -import { DevCommand, DevCommandArgs, DevCommandOpts } from "../../../../src/commands/dev" +import { DevCommand, DevCommandArgs, DevCommandOpts, getDevCommandWatchTasks } from "../../../../src/commands/dev" import { makeTestGardenA, withDefaultGlobalOpts, TestGarden } from "../../../helpers" import { ParameterValues } from "../../../../src/commands/base" import { GlobalOptions } from "../../../../src/cli/cli" @@ -233,3 +233,42 @@ describe("DevCommand", () => { return promise }) }) + +describe("getDevCommandWatchTasks", () => { + it("should deploy, run and test appropriately on watch change", async () => { + const garden = await makeTestGardenA() + const log = garden.log + const graph = await garden.getConfigGraph(log) + + const watchTasks = await getDevCommandWatchTasks({ + garden, + log, + updatedGraph: graph, + module: await graph.getModule("module-b"), + hotReloadServiceNames: [], + testNames: undefined, + skipTests: false, + }) + + const results = await garden.processTasks(watchTasks) + expect(Object.keys(results).sort()).to.eql([ + "build.module-a", + "build.module-b", + "build.module-c", + "deploy.service-a", + "deploy.service-b", + "deploy.service-c", + "get-service-status.service-a", + "get-service-status.service-b", + "get-service-status.service-c", + "get-task-result.task-c", + "stage-build.module-a", + "stage-build.module-b", + "stage-build.module-c", + "task.task-c", + "test.module-b.unit", + "test.module-c.integ", + "test.module-c.unit", + ]) + }) +}) diff --git a/garden-service/test/unit/src/tasks/deploy.ts b/garden-service/test/unit/src/tasks/deploy.ts index f25ac662bf..70128faa71 100644 --- a/garden-service/test/unit/src/tasks/deploy.ts +++ b/garden-service/test/unit/src/tasks/deploy.ts @@ -12,7 +12,8 @@ import execa from "execa" import { ProjectConfig } from "../../../../src/config/project" import { DEFAULT_API_VERSION } from "../../../../src/constants" import { Garden } from "../../../../src/garden" -import { createGardenPlugin } from "../../../../src/types/plugin/plugin" +import { ConfigGraph } from "../../../../src/config-graph" +import { createGardenPlugin, GardenPlugin } from "../../../../src/types/plugin/plugin" import { joi } from "../../../../src/config/common" import { ServiceState } from "../../../../src/types/service" import { DeployTask } from "../../../../src/tasks/deploy" @@ -22,7 +23,10 @@ import { expect } from "chai" describe("DeployTask", () => { let tmpDir: tmp.DirectoryResult + let garden: Garden + let graph: ConfigGraph let config: ProjectConfig + let testPlugin: GardenPlugin before(async () => { tmpDir = await tmp.dir({ unsafeCleanup: true }) @@ -40,98 +44,141 @@ describe("DeployTask", () => { providers: [{ name: "test" }], variables: {}, } - }) - after(async () => { - await tmpDir.cleanup() - }) + testPlugin = createGardenPlugin({ + name: "test", + createModuleTypes: [ + { + name: "test", + docs: "test", + serviceOutputsSchema: joi.object().keys({ log: joi.string() }), + handlers: { + build: async () => ({}), + getServiceStatus: async () => { + return { + state: "missing", + detail: {}, + outputs: {}, + } + }, + deployService: async ({ service }: DeployServiceParams) => { + return { + state: "ready", + detail: {}, + outputs: { log: service.spec.log }, + } + }, + runTask: async ({ task }: RunTaskParams) => { + const log = task.spec.log - describe("process", () => { - it("should correctly resolve runtime outputs from tasks", async () => { - const testPlugin = createGardenPlugin({ + return { + taskName: task.name, + moduleName: task.module.name, + success: true, + outputs: { log }, + command: [], + log, + startedAt: new Date(), + completedAt: new Date(), + version: task.module.version.versionString, + } + }, + }, + }, + ], + }) + + garden = await Garden.factory(tmpDir.path, { config, plugins: [testPlugin] }) + + garden["moduleConfigs"] = { + test: { + apiVersion: DEFAULT_API_VERSION, name: "test", - createModuleTypes: [ + type: "test", + allowPublish: false, + disabled: false, + build: { dependencies: [] }, + outputs: {}, + path: tmpDir.path, + serviceConfigs: [ + { + name: "test-service", + dependencies: ["test-task"], + disabled: false, + hotReloadable: false, + spec: { + log: "${runtime.tasks.test-task.outputs.log}", + }, + }, + ], + taskConfigs: [ { - name: "test", - docs: "test", - serviceOutputsSchema: joi.object().keys({ log: joi.string() }), - handlers: { - build: async () => ({}), - getServiceStatus: async () => { - return { - state: "missing", - detail: {}, - outputs: {}, - } - }, - deployService: async ({ service }: DeployServiceParams) => { - return { - state: "ready", - detail: {}, - outputs: { log: service.spec.log }, - } - }, - runTask: async ({ task }: RunTaskParams) => { - const log = task.spec.log - - return { - taskName: task.name, - moduleName: task.module.name, - success: true, - outputs: { log }, - command: [], - log, - startedAt: new Date(), - completedAt: new Date(), - version: task.module.version.versionString, - } - }, + name: "test-task", + cacheResult: true, + dependencies: [], + disabled: false, + spec: { + log: "test output", }, + timeout: 10, }, ], + testConfigs: [], + spec: { bla: "fla" }, + }, + } + + graph = await garden.getConfigGraph(garden.log) + }) + + after(async () => { + await tmpDir.cleanup() + }) + + describe("getDependencies", () => { + it("should always return task dependencies having force = false", async () => { + const testService = await graph.getService("test-service") + + const forcedDeployTask = new DeployTask({ + garden, + graph, + service: testService, + force: true, + forceBuild: false, + fromWatch: false, + log: garden.log, }) - const garden = await Garden.factory(tmpDir.path, { config, plugins: [testPlugin] }) + expect((await forcedDeployTask.getDependencies()).find((dep) => dep.type === "task")!.force).to.be.false - garden["moduleConfigs"] = { - test: { - apiVersion: DEFAULT_API_VERSION, - name: "test", - type: "test", - allowPublish: false, - disabled: false, - build: { dependencies: [] }, - outputs: {}, - path: tmpDir.path, - serviceConfigs: [ - { - name: "test-service", - dependencies: ["test-task"], - disabled: false, - hotReloadable: false, - spec: { - log: "${runtime.tasks.test-task.outputs.log}", - }, - }, - ], - taskConfigs: [ - { - name: "test-task", - cacheResult: true, - dependencies: [], - disabled: false, - spec: { - log: "test output", - }, - timeout: 10, - }, - ], - testConfigs: [], - spec: { bla: "fla" }, - }, - } + const unforcedDeployTask = new DeployTask({ + garden, + graph, + service: testService, + force: false, + forceBuild: false, + fromWatch: false, + log: garden.log, + }) + + expect((await unforcedDeployTask.getDependencies()).find((dep) => dep.type === "task")!.force).to.be.false + + const deployTaskFromWatch = new DeployTask({ + garden, + graph, + service: testService, + force: false, + forceBuild: false, + fromWatch: true, + log: garden.log, + }) - const graph = await garden.getConfigGraph(garden.log) + expect((await deployTaskFromWatch.getDependencies()).find((dep) => dep.type === "task")!.force).to.be.false + }) + }) + + describe("process", () => { + it("should correctly resolve runtime outputs from tasks", async () => { const testService = await graph.getService("test-service") const deployTask = new DeployTask({