fix(core): ensure builds are staged when module has no build handler

examples/vote-helm/base-chart/values.yaml

@@ -5,13 +5,16 @@
 replicaCount: 1
 
 image:
-  repository:
-  tag:
+  repository: busybox
+  tag: latest
 
-fullnameOverride: ""
+name: "base-chart"
 
 args: []
 
+service:
+  type: ClusterIP
+
 servicePort: 80
 
 ingress:

garden-service/src/config-graph.ts

@@ -11,7 +11,7 @@ import toposort from "toposort"
 import { flatten, pick, uniq, find, sortBy } from "lodash"
 import { Garden } from "./garden"
 import { BuildDependencyConfig, ModuleConfig } from "./config/module"
-import { Module, getModuleKey, moduleFromConfig } from "./types/module"
+import { Module, getModuleKey, moduleFromConfig, moduleNeedsBuild } from "./types/module"
 import { Service, serviceFromConfig } from "./types/service"
 import { Task, taskFromConfig } from "./types/task"
 import { TestConfig } from "./config/test"
@@ -159,6 +159,7 @@ export class ConfigGraph {
 
     for (const moduleConfig of moduleConfigs) {
       const type = moduleTypes[moduleConfig.type]
+      const needsBuild = moduleNeedsBuild(moduleConfig, type)
 
       const moduleKey = this.keyForModule(moduleConfig)
       this.moduleConfigs[moduleKey] = moduleConfig
@@ -170,15 +171,15 @@ export class ConfigGraph {
         }
       }
 
-      if (type.needsBuild) {
+      if (needsBuild) {
         addBuildDeps(this.getNode("build", moduleKey, moduleKey))
       }
 
       // Service dependencies
       for (const serviceConfig of moduleConfig.serviceConfigs) {
         const serviceNode = this.getNode("deploy", serviceConfig.name, moduleKey)
 
-        if (type.needsBuild) {
+        if (needsBuild) {
           // The service needs its own module to be built
           this.addRelation(serviceNode, "build", moduleKey, moduleKey)
         } else {
@@ -199,7 +200,7 @@ export class ConfigGraph {
       for (const taskConfig of moduleConfig.taskConfigs) {
         const taskNode = this.getNode("run", taskConfig.name, moduleKey)
 
-        if (type.needsBuild) {
+        if (needsBuild) {
           // The task needs its own module to be built
           this.addRelation(taskNode, "build", moduleKey, moduleKey)
         } else {
@@ -224,7 +225,7 @@ export class ConfigGraph {
 
         const testNode = this.getNode("test", testConfigName, moduleKey)
 
-        if (type.needsBuild) {
+        if (needsBuild) {
           // The test needs its own module to be built
           this.addRelation(testNode, "build", moduleKey, moduleKey)
         } else {

garden-service/src/task-graph.ts

@@ -105,7 +105,7 @@ export class TaskGraph {
     // to return the latest result for each requested task.
     const resultKeys = tasks.map((t) => t.getKey())
 
-    const results = this.opQueue.add(() => this.processTasksInternal(tasksToProcess, resultKeys, opts))
+    const results = await this.opQueue.add(() => this.processTasksInternal(tasksToProcess, resultKeys, opts))
 
     if (opts && opts.throwOnError) {
       const failed = Object.entries(results).filter(([_, result]) => result && result.error)
@@ -114,7 +114,7 @@ export class TaskGraph {
         throw new TaskGraphError(
           dedent`
             ${failed.length} task(s) failed:
-            ${failed.map(([key, result]) => `- ${key}: ${result.error.toString()}`).join("\n")}
+            ${failed.map(([key, result]) => `- ${key}: ${result?.error?.toString()}`).join("\n")}
           `,
           { results }
         )

garden-service/src/tasks/build.ts

@@ -14,97 +14,82 @@ import { BaseTask, TaskType } from "../tasks/base"
 import { Garden } from "../garden"
 import { LogEntry } from "../logger/log-entry"
 import { StageBuildTask } from "./stage-build"
-import { some, flatten } from "lodash"
+import { flatten } from "lodash"
 
 export interface BuildTaskParams {
   garden: Garden
   log: LogEntry
   module: Module
   force: boolean
-  fromWatch?: boolean
-  hotReloadServiceNames?: string[]
 }
 
 export class BuildTask extends BaseTask {
   type: TaskType = "build"
 
   private module: Module
-  private fromWatch: boolean
-  private hotReloadServiceNames: string[]
-
-  constructor({
-    garden,
-    log,
-    module,
-    force,
-    fromWatch = false,
-    hotReloadServiceNames = [],
-  }: BuildTaskParams & { _guard: true }) {
+
+  constructor({ garden, log, module, force }: BuildTaskParams & { _guard: true }) {
     // Note: The _guard attribute is to prevent accidentally bypassing the factory method
     super({ garden, log, force, version: module.version })
     this.module = module
-    this.fromWatch = fromWatch
-    this.hotReloadServiceNames = hotReloadServiceNames
   }
 
   static async factory(params: BuildTaskParams): Promise<BaseTask[]> {
     // We need to see if a build step is necessary for the module. If it is, return a build task for the module.
     // Otherwise, return a build task for each of the module's dependencies.
     // We do this to avoid displaying no-op build steps in the stack graph.
-
-    const { garden, module } = params
-
-    // We need to build if there is a copy statement on any of the build dependencies.
-    let needsBuild = some(module.build.dependencies, (d) => d.copy && d.copy.length > 0)
-
-    if (!needsBuild) {
-      // We also need to build if there is a build handler for the module type
-      const actions = await garden.getActionRouter()
-      try {
-        await actions.getModuleActionHandler({
-          actionType: "build",
-          moduleType: module.type,
-        })
-
-        needsBuild = true
-      } catch {
-        // No build handler for the module type.
-      }
-    }
+    const { garden, log, force } = params
 
     const buildTask = new BuildTask({ ...params, _guard: true })
 
-    if (needsBuild) {
+    if (params.module.needsBuild) {
       return [buildTask]
     } else {
-      return buildTask.getDependencies()
+      const buildTasks = await Bluebird.map(
+        Object.values(params.module.buildDependencies),
+        (module) =>
+          new BuildTask({
+            garden,
+            log,
+            module,
+            force,
+            _guard: true,
+          })
+      )
+      const stageBuildTask = new StageBuildTask({
+        garden,
+        log,
+        module: params.module,
+        force,
+        dependencies: buildTasks,
+      })
+      return [stageBuildTask, ...buildTasks]
     }
   }
 
   async getDependencies() {
     const dg = await this.garden.getConfigGraph(this.log)
     const deps = (await dg.getDependencies("build", this.getName(), false)).build
 
-    const stageBuildTask = new StageBuildTask({
-      garden: this.garden,
-      log: this.log,
-      module: this.module,
-      force: this.force,
-    })
-
     const buildTasks = flatten(
       await Bluebird.map(deps, async (m: Module) => {
         return BuildTask.factory({
           garden: this.garden,
           log: this.log,
           module: m,
           force: this.force,
-          fromWatch: this.fromWatch,
-          hotReloadServiceNames: this.hotReloadServiceNames,
         })
       })
     )
 
+    const stageBuildTask = new StageBuildTask({
+      garden: this.garden,
+      log: this.log,
+      module: this.module,
+      force: this.force,
+      dependencies: buildTasks,
+    })
+
     return [stageBuildTask, ...buildTasks]
   }
 

garden-service/src/tasks/deploy.ts

@@ -134,8 +134,6 @@ export class DeployTask extends BaseTask {
         log: this.log,
         module: this.service.module,
         force: this.forceBuild,
-        fromWatch: this.fromWatch,
-        hotReloadServiceNames: this.hotReloadServiceNames,
       })
 
       return [statusTask, ...deployTasks, ...taskTasks, ...buildTasks]

garden-service/src/tasks/helpers.ts

@@ -49,8 +49,6 @@ export async function getDependantTasksForModule({
         log,
         module,
         force: forceBuild,
-        fromWatch,
-        hotReloadServiceNames,
       }))
     )
     services = await graph.getServices(module.serviceNames)
@@ -75,8 +73,6 @@ export async function getDependantTasksForModule({
           log,
           module,
           force: true,
-          fromWatch,
-          hotReloadServiceNames,
         }))
       )
       dependantBuildModules = dependants.build
@@ -91,8 +87,6 @@ export async function getDependantTasksForModule({
         log,
         module: m,
         force: forceBuild,
-        fromWatch,
-        hotReloadServiceNames,
       })
     )
   )

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

@@ -20,30 +20,35 @@ export interface StageBuildTaskParams {
   log: LogEntry
   module: Module
   force: boolean
+  dependencies?: BaseTask[]
 }
 
 export class StageBuildTask extends BaseTask {
   type: TaskType = "stage-build"
 
   private module: Module
+  private extraDependencies: BaseTask[]
 
-  constructor({ garden, log, module, force }: StageBuildTaskParams) {
+  constructor({ garden, log, module, force, dependencies }: StageBuildTaskParams) {
     super({ garden, log, force, version: module.version })
     this.module = module
+    this.extraDependencies = dependencies || []
   }
 
   async getDependencies() {
     const dg = await this.garden.getConfigGraph(this.log)
     const deps = (await dg.getDependencies("build", this.getName(), false)).build
 
-    return Bluebird.map(deps, async (m: Module) => {
+    const stageDeps = await Bluebird.map(deps, async (m: Module) => {
       return new StageBuildTask({
         garden: this.garden,
         log: this.log,
         module: m,
         force: this.force,
       })
     })
+
+    return [...stageDeps, ...this.extraDependencies]
   }
 
   getName() {

garden-service/src/types/module.ts

@@ -6,7 +6,7 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/.
  */
 
-import { flatten, uniq, cloneDeep, keyBy } from "lodash"
+import { flatten, uniq, cloneDeep, keyBy, some } from "lodash"
 import { getNames } from "../util/util"
 import { TestSpec } from "../config/test"
 import { ModuleSpec, ModuleConfig, moduleConfigSchema } from "../config/module"
@@ -20,6 +20,7 @@ import { ConfigGraph } from "../config-graph"
 import Bluebird from "bluebird"
 import { getConfigFilePath } from "../util/fs"
 import { getModuleTypeBases } from "../plugins"
+import { ModuleType } from "./plugin/plugin"
 
 export interface FileCopySpec {
   source: string
@@ -38,6 +39,7 @@ export interface Module<
   buildPath: string
   buildMetadataPath: string
   configPath: string
+  needsBuild: boolean
 
   version: ModuleVersion
 
@@ -73,6 +75,12 @@ export const moduleSchema = moduleConfigSchema.keys({
   buildDependencies: joiIdentifierMap(joi.lazy(() => moduleSchema))
     .required()
     .description("A map of all modules referenced under `build.dependencies`."),
+  needsBuild: joi
+    .boolean()
+    .required()
+    .description(
+      "Indicate whether the module needs to be built (i.e. has a build handler or needs to copy dependencies)."
+    ),
   serviceNames: joiArray(joiIdentifier())
     .required()
     .description("The names of the services that the module provides."),
@@ -109,6 +117,7 @@ export async function moduleFromConfig(garden: Garden, graph: ConfigGraph, confi
     configPath,
 
     version,
+    needsBuild: moduleNeedsBuild(config, moduleTypes[config.type]),
 
     buildDependencies: {},
 
@@ -134,6 +143,10 @@ export async function moduleFromConfig(garden: Garden, graph: ConfigGraph, confi
   return module
 }
 
+export function moduleNeedsBuild(moduleConfig: ModuleConfig, moduleType: ModuleType) {
+  return moduleType.needsBuild || some(moduleConfig.build.dependencies, (d) => d.copy && d.copy.length > 0)
+}
+
 export function getModuleCacheContext(config: ModuleConfig) {
   return pathToCacheContext(config.path)
 }

garden-service/test/integ/src/plugins/kubernetes/container/container.ts

@@ -386,7 +386,7 @@ describe("kubernetes container module handlers", () => {
           version: task.module.version,
         })
 
-        result = await garden.processTasks([testTask], { throwOnError: true })
+        result = await garden.processTasks([testTask])
 
         const key = "task.missing-sh-task"
 
@@ -412,7 +412,7 @@ describe("kubernetes container module handlers", () => {
           version: task.module.version,
         })
 
-        result = await garden.processTasks([testTask], { throwOnError: true })
+        result = await garden.processTasks([testTask])
 
         const key = "task.missing-tar-task"
 
@@ -508,7 +508,7 @@ describe("kubernetes container module handlers", () => {
           version: module.version,
         })
 
-        result = await garden.processTasks([testTask], { throwOnError: true })
+        result = await garden.processTasks([testTask])
 
         const key = "test.missing-sh.missing-sh-test"
         expect(result).to.have.property(key)
@@ -534,7 +534,7 @@ describe("kubernetes container module handlers", () => {
           version: module.version,
         })
 
-        result = await garden.processTasks([testTask], { throwOnError: true })
+        result = await garden.processTasks([testTask])
 
         const key = "test.missing-tar.missing-tar-test"
         expect(result).to.have.property(key)