From 1f189bb91628d4d61cd84d1fc66e73f3d0920b78 Mon Sep 17 00:00:00 2001
From: Thorarinn Sigurdsson <thorarinnsigurdsson@gmail.com>
Date: Wed, 1 Apr 2020 16:33:08 +0200
Subject: [PATCH] fix(core): fix watch task logic for sourceModules (#1756)

Before this fix, if a service with a sourceModule was deployed with hot
reloading enabled, a build task for its source module would be added
when its sources change (in addition to the hot reload task).

This was not the desired behavior, and would (e.g. for helm modules
using a container module as a sourceModule) result in unnecessary
rebuilds of the source module.
---
 garden-service/src/tasks/helpers.ts           | 28 ++++++++-
 garden-service/test/unit/src/tasks/helpers.ts | 59 +++++++++++++++++++
 2 files changed, 85 insertions(+), 2 deletions(-)

diff --git a/garden-service/src/tasks/helpers.ts b/garden-service/src/tasks/helpers.ts
index fb5711df1f..27acecf69a 100644
--- a/garden-service/src/tasks/helpers.ts
+++ b/garden-service/src/tasks/helpers.ts
@@ -38,7 +38,29 @@ export async function getModuleWatchTasks({
 
   const dependants = graph.getDependantsForModule(module, true)
 
-  if (intersection(module.serviceNames, hotReloadServiceNames).length === 0) {
+  const dependantSourceModules = dependants.build.filter((depModule) =>
+    depModule.serviceConfigs.find((s) => s.sourceModuleName === module.name)
+  )
+
+  const dependantSourceModuleServiceNames = flatten(
+    dependantSourceModules.map((depModule) => {
+      return depModule.serviceConfigs.filter((s) => s.sourceModuleName === module.name).map((s) => s.name)
+    })
+  )
+
+  const serviceNamesUsingModule = [...module.serviceNames, ...dependantSourceModuleServiceNames]
+
+  /**
+   * If a service is deployed with hot reloading enabled, we don't rebuild its module
+   * (or its sourceModule, if the service instead uses a sourceModule) when its
+   * sources change.
+   *
+   * Therefore, we skip adding a build task for module if one of its services is in
+   * hotReloadServiceNames, or if one of its build dependants' services is in
+   * hotReloadServiceNames and has module as its sourceModule (in which case we
+   * also don't add a build task for the dependant's module below).
+   */
+  if (intersection(serviceNamesUsingModule, hotReloadServiceNames).length === 0) {
     buildTasks = await BuildTask.factory({
       garden,
       graph,
@@ -48,9 +70,11 @@ export async function getModuleWatchTasks({
     })
   }
 
+  const dependantSourceModuleNames = dependantSourceModules.map((m) => m.name)
+
   const dependantBuildTasks = flatten(
     await Bluebird.map(
-      dependants.build.filter((m) => !m.disabled),
+      dependants.build.filter((m) => !m.disabled && !dependantSourceModuleNames.includes(m.name)),
       (m) =>
         BuildTask.factory({
           garden,
diff --git a/garden-service/test/unit/src/tasks/helpers.ts b/garden-service/test/unit/src/tasks/helpers.ts
index 22f01005f9..7f0b934ef2 100644
--- a/garden-service/test/unit/src/tasks/helpers.ts
+++ b/garden-service/test/unit/src/tasks/helpers.ts
@@ -166,6 +166,65 @@ describe("TaskHelpers", () => {
       expect(sortedBaseKeys(tasks)).to.eql(["build.module-a", "deploy.service-a"])
     })
 
+    it("should not add a build task for a hot-reload-enabled service's sourceModule", async () => {
+      const garden = await makeTestGardenA()
+
+      garden.setModuleConfigs([
+        {
+          apiVersion: DEFAULT_API_VERSION,
+          allowPublish: false,
+          build: { dependencies: [] },
+          disabled: false,
+          name: "module-a",
+          include: [],
+          outputs: {},
+          path: garden.projectRoot,
+          serviceConfigs: [],
+          taskConfigs: [],
+          spec: { services: [] },
+          testConfigs: [],
+          type: "test",
+        },
+        {
+          apiVersion: DEFAULT_API_VERSION,
+          allowPublish: false,
+          build: { dependencies: [{ name: "module-a", copy: [] }] },
+          disabled: false,
+          name: "module-b",
+          include: [],
+          outputs: {},
+          path: garden.projectRoot,
+          serviceConfigs: [],
+          taskConfigs: [],
+          spec: {
+            services: [
+              {
+                name: "service-b",
+                dependencies: [],
+                disabled: false,
+                sourceModule: "module-a", // <---------------
+              },
+            ],
+          },
+          testConfigs: [],
+          type: "test",
+        },
+      ])
+
+      const graph = await garden.getConfigGraph(garden.log)
+      const module = await graph.getModule("module-b", true)
+
+      const tasks = await getModuleWatchTasks({
+        garden,
+        graph,
+        log,
+        module,
+        hotReloadServiceNames: ["service-b"],
+      })
+
+      expect(sortedBaseKeys(tasks)).to.eql(["hot-reload.service-b"])
+    })
+
     context("without hot reloading enabled", () => {
       const expectedBaseKeysByChangedModule = [
         {