Merge pull request #1373 from garden-io/fix-build-stage

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

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": {},
     })
   })
 })