fix(build): fix intermittent concurrency issues when staging build

Because both the BuildTask and GetServiceTask need to stage builds,
they could in some cases end up stepping on each other because one
doesn't depend on the other. I resolved that by adding a StageBuildTask
that both depend on.

garden-service/src/plugins/kubernetes/system.ts

@@ -27,6 +27,7 @@ import { combineStates } from "../../types/service"
 import { KubernetesResource } from "./types"
 import { defaultDotIgnoreFiles } from "../../util/fs"
 import { HelmServiceStatus } from "./helm/status"
+import { LogLevel } from "../../logger/log-node"
 
 const GARDEN_VERSION = getPackageVersion()
 const SYSTEM_NAMESPACE_MIN_VERSION = "0.9.0"
@@ -156,7 +157,10 @@ export async function getSystemServiceStatus({
 
   const actions = await sysGarden.getActionRouter()
 
-  const serviceStatuses = await actions.getServiceStatuses({ log, serviceNames })
+  const serviceStatuses = await actions.getServiceStatuses({
+    log: log.placeholder(LogLevel.verbose, true),
+    serviceNames,
+  })
   const state = combineStates(values(serviceStatuses).map((s) => (s && s.state) || "unknown"))
 
   // Add the Kubernetes dashboard to the Garden dashboard

garden-service/src/tasks/base.ts

@@ -25,6 +25,7 @@ export type TaskType =
   | "hot-reload"
   | "publish"
   | "resolve-provider"
+  | "stage-build"
   | "task"
   | "test"
 

garden-service/src/tasks/build.ts

@@ -8,12 +8,12 @@
 
 import Bluebird from "bluebird"
 import chalk from "chalk"
-import pluralize = require("pluralize")
 import { Module, getModuleKey } from "../types/module"
 import { BuildResult } from "../types/plugin/module/build"
 import { BaseTask, TaskType } from "../tasks/base"
 import { Garden } from "../garden"
 import { LogEntry } from "../logger/log-entry"
+import { StageBuildTask } from "./stage-build"
 
 export interface BuildTaskParams {
   garden: Garden
@@ -42,7 +42,14 @@ export class BuildTask extends BaseTask {
     const dg = await this.garden.getConfigGraph()
     const deps = (await dg.getDependencies("build", this.getName(), false)).build
 
-    return Bluebird.map(deps, async (m: Module) => {
+    const stageBuildTask = new StageBuildTask({
+      garden: this.garden,
+      log: this.log,
+      module: this.module,
+      force: this.force,
+    })
+
+    const buildTasks = await Bluebird.map(deps, async (m: Module) => {
       return new BuildTask({
         garden: this.garden,
         log: this.log,
@@ -52,6 +59,8 @@ export class BuildTask extends BaseTask {
         hotReloadServiceNames: this.hotReloadServiceNames,
       })
     })
+
+    return [stageBuildTask, ...buildTasks]
   }
 
   getName() {
@@ -66,27 +75,24 @@ export class BuildTask extends BaseTask {
     const module = this.module
     const actions = await this.garden.getActionRouter()
 
-    const log = this.log.info({
-      section: this.getName(),
-      msg: `Preparing build (${pluralize("file", module.version.files.length, true)})...`,
-      status: "active",
-    })
+    let log: LogEntry
 
     const logSuccess = () => {
-      log.setSuccess({
-        msg: chalk.green(`Done (took ${log.getDuration(1)} sec)`),
-        append: true,
-      })
+      if (log) {
+        log.setSuccess({
+          msg: chalk.green(`Done (took ${log.getDuration(1)} sec)`),
+          append: true,
+        })
+      }
     }
 
-    const graph = await this.garden.getConfigGraph()
-    await this.garden.buildDir.syncFromSrc(this.module, log)
-    await this.garden.buildDir.syncDependencyProducts(this.module, graph, log)
-
     if (!this.force) {
-      log.setState({
+      log = this.log.info({
+        section: this.getName(),
         msg: `Getting build status for ${module.version.versionString}...`,
+        status: "active",
       })
+
       const status = await actions.getBuildStatus({ log: this.log, module })
 
       if (status.ready) {
@@ -95,7 +101,11 @@ export class BuildTask extends BaseTask {
       }
     }
 
-    log.setState({ msg: `Building version ${module.version.versionString}...` })
+    log = this.log.info({
+      section: this.getName(),
+      msg: `Building version ${module.version.versionString}...`,
+      status: "active",
+    })
 
     let result: BuildResult
     try {

garden-service/src/tasks/delete-service.ts

@@ -12,6 +12,7 @@ import { Service, ServiceStatus } from "../types/service"
 import { Garden } from "../garden"
 import { ConfigGraph } from "../config-graph"
 import { TaskResults, TaskResult } from "../task-graph"
+import { StageBuildTask } from "./stage-build"
 
 export interface DeleteServiceTaskParams {
   garden: Garden
@@ -36,14 +37,21 @@ export class DeleteServiceTask extends BaseTask {
   }
 
   async getDependencies() {
+    const stageBuildTask = new StageBuildTask({
+      garden: this.garden,
+      log: this.log,
+      module: this.service.module,
+      force: this.force,
+    })
+
     if (!this.includeDependants) {
-      return []
+      return [stageBuildTask]
     }
 
     // Note: We delete in _reverse_ dependency order, so we query for dependants
     const deps = await this.graph.getDependants("service", this.getName(), false)
 
-    return deps.service.map((service) => {
+    const dependants = deps.service.map((service) => {
       return new DeleteServiceTask({
         garden: this.garden,
         graph: this.graph,
@@ -52,6 +60,8 @@ export class DeleteServiceTask extends BaseTask {
         includeDependants: this.includeDependants,
       })
     })
+
+    return [stageBuildTask, ...dependants]
   }
 
   getName() {

garden-service/src/tasks/deploy.ts

@@ -71,32 +71,28 @@ export class DeployTask extends BaseTask {
       (depNode) => !(depNode.type === "service" && includes(this.hotReloadServiceNames, depNode.name))
     )
 
-    const tasks: BaseTask[] = deps.service.map((service) => {
-      return new DeployTask({
-        garden: this.garden,
-        graph: this.graph,
-        log: this.log,
-        service,
-        force: false,
-        forceBuild: this.forceBuild,
-        fromWatch: this.fromWatch,
-        hotReloadServiceNames: this.hotReloadServiceNames,
-      })
+    const statusTask = new GetServiceStatusTask({
+      garden: this.garden,
+      graph: this.graph,
+      log: this.log,
+      service: this.service,
+      force: false,
+      hotReloadServiceNames: this.hotReloadServiceNames,
     })
 
-    tasks.push(
-      new GetServiceStatusTask({
-        garden: this.garden,
-        graph: this.graph,
-        log: this.log,
-        service: this.service,
-        force: false,
-        hotReloadServiceNames: this.hotReloadServiceNames,
-      })
-    )
-
     if (this.fromWatch && includes(this.hotReloadServiceNames, this.service.name)) {
       // Only need to get existing statuses and results when hot-reloading
+      const dependencyStatusTasks = deps.service.map((service) => {
+        return new GetServiceStatusTask({
+          garden: this.garden,
+          graph: this.graph,
+          log: this.log,
+          service,
+          force: false,
+          hotReloadServiceNames: this.hotReloadServiceNames,
+        })
+      })
+
       const taskResultTasks = await Bluebird.map(deps.task, async (task) => {
         return new GetTaskResultTask({
           garden: this.garden,
@@ -107,8 +103,21 @@ export class DeployTask extends BaseTask {
         })
       })
 
-      return [...tasks, ...taskResultTasks]
+      return [statusTask, ...dependencyStatusTasks, ...taskResultTasks]
     } else {
+      const deployTasks = deps.service.map((service) => {
+        return new DeployTask({
+          garden: this.garden,
+          graph: this.graph,
+          log: this.log,
+          service,
+          force: false,
+          forceBuild: this.forceBuild,
+          fromWatch: this.fromWatch,
+          hotReloadServiceNames: this.hotReloadServiceNames,
+        })
+      })
+
       const taskTasks = await Bluebird.map(deps.task, (task) => {
         return TaskTask.factory({
           task,
@@ -129,7 +138,7 @@ export class DeployTask extends BaseTask {
         hotReloadServiceNames: this.hotReloadServiceNames,
       })
 
-      return [...tasks, ...taskTasks, buildTask]
+      return [statusTask, ...deployTasks, ...taskTasks, buildTask]
     }
   }
 

garden-service/src/tasks/get-service-status.ts

@@ -16,6 +16,7 @@ import { TaskResults } from "../task-graph"
 import { prepareRuntimeContext } from "../runtime-context"
 import { getTaskVersion, TaskTask } from "./task"
 import Bluebird from "bluebird"
+import { StageBuildTask } from "./stage-build"
 
 export interface GetServiceStatusTaskParams {
   garden: Garden
@@ -43,6 +44,13 @@ export class GetServiceStatusTask extends BaseTask {
   async getDependencies() {
     const deps = await this.graph.getDependencies("service", this.getName(), false)
 
+    const stageBuildTask = new StageBuildTask({
+      garden: this.garden,
+      log: this.log,
+      module: this.service.module,
+      force: this.force,
+    })
+
     const statusTasks = deps.service.map((service) => {
       return new GetServiceStatusTask({
         garden: this.garden,
@@ -66,7 +74,7 @@ export class GetServiceStatusTask extends BaseTask {
       })
     })
 
-    return [...statusTasks, ...taskTasks]
+    return [stageBuildTask, ...statusTasks, ...taskTasks]
   }
 
   getName() {
@@ -98,11 +106,6 @@ export class GetServiceStatusTask extends BaseTask {
 
     const actions = await this.garden.getActionRouter()
 
-    // Some handlers expect builds to have been staged when resolving services statuses.
-    const graph = await this.garden.getConfigGraph()
-    await this.garden.buildDir.syncFromSrc(this.service.module, log)
-    await this.garden.buildDir.syncDependencyProducts(this.service.module, graph, log)
-
     let status = await actions.getServiceStatus({
       service: this.service,
       log,

garden-service/src/tasks/stage-build.ts

@@ -0,0 +1,81 @@
+/*
+ * Copyright (C) 2018 Garden Technologies, Inc. <[email protected]>
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ */
+
+import Bluebird from "bluebird"
+import chalk from "chalk"
+import pluralize from "pluralize"
+import { Module, getModuleKey } from "../types/module"
+import { BuildResult } from "../types/plugin/module/build"
+import { BaseTask, TaskType } from "../tasks/base"
+import { Garden } from "../garden"
+import { LogEntry } from "../logger/log-entry"
+
+export interface StageBuildTaskParams {
+  garden: Garden
+  log: LogEntry
+  module: Module
+  force: boolean
+}
+
+export class StageBuildTask extends BaseTask {
+  type: TaskType = "stage-build"
+
+  private module: Module
+
+  constructor({ garden, log, module, force }: StageBuildTaskParams) {
+    super({ garden, log, force, version: module.version })
+    this.module = module
+  }
+
+  async getDependencies() {
+    const dg = await this.garden.getConfigGraph()
+    const deps = (await dg.getDependencies("build", this.getName(), false)).build
+
+    return Bluebird.map(deps, async (m: Module) => {
+      return new StageBuildTask({
+        garden: this.garden,
+        log: this.log,
+        module: m,
+        force: this.force,
+      })
+    })
+  }
+
+  getName() {
+    return getModuleKey(this.module.name, this.module.plugin)
+  }
+
+  getDescription() {
+    return `staging build for ${this.getName()}`
+  }
+
+  async process(): Promise<BuildResult> {
+    let log: LogEntry | undefined = undefined
+
+    if (this.module.version.files.length > 0) {
+      log = this.log.info({
+        section: this.getName(),
+        msg: `Syncing module sources (${pluralize("file", this.module.version.files.length, true)})...`,
+        status: "active",
+      })
+    }
+
+    const graph = await this.garden.getConfigGraph()
+    await this.garden.buildDir.syncFromSrc(this.module, log || this.log)
+    await this.garden.buildDir.syncDependencyProducts(this.module, graph, log || this.log)
+
+    if (log) {
+      log.setSuccess({
+        msg: chalk.green(`Done (took ${log.getDuration(1)} sec)`),
+        append: true,
+      })
+    }
+
+    return {}
+  }
+}

garden-service/test/unit/src/commands/build.ts

@@ -23,6 +23,9 @@ describe("commands.build", () => {
       "build.module-a": { fresh: true, buildLog: "A" },
       "build.module-b": { fresh: true, buildLog: "B" },
       "build.module-c": {},
+      "stage-build.module-a": {},
+      "stage-build.module-b": {},
+      "stage-build.module-c": {},
     })
   })
 
@@ -44,6 +47,8 @@ describe("commands.build", () => {
     expect(taskResultOutputs(result!)).to.eql({
       "build.module-a": { fresh: true, buildLog: "A" },
       "build.module-b": { fresh: true, buildLog: "B" },
+      "stage-build.module-a": {},
+      "stage-build.module-b": {},
     })
   })
 })