fix: fixed another dependency calculation bug

Fixes #407. Before this commit, getDependantsForModule was calculating the dependants of the module's service/task dependencies instead of the dependants of the module's services and tasks. Adapted the task helper tests to cover this case too. Also added some more debug/silly-level log statements.
garden-io · Dec 4, 2018 · 99df5d9 · 99df5d9
1 parent 4a8428c
commit 99df5d9
Show file tree

Hide file tree

Showing 10 changed files with 75 additions and 13 deletions.
diff --git a/garden-service/src/dependency-graph.ts b/garden-service/src/dependency-graph.ts
@@ -151,14 +151,10 @@ export class DependencyGraph {
    * Returns all build and runtime dependants of module and its services & tasks (recursively).
    */
   async getDependantsForModule(module: Module, filterFn?: DependencyRelationFilterFn): Promise<DependencyRelations> {
-    const runtimeDependencies = uniq(module.serviceDependencyNames.concat(module.taskDependencyNames))
-    const serviceNames = runtimeDependencies.filter(d => this.serviceMap[d])
-    const taskNames = runtimeDependencies.filter(d => this.taskMap[d])
-
     return this.mergeRelations(... await Bluebird.all([
       this.getDependants("build", module.name, true, filterFn),
-      this.getDependantsForMany("service", serviceNames, true, filterFn),
-      this.getDependantsForMany("task", taskNames, true, filterFn),
+      this.getDependantsForMany("service", module.serviceNames, true, filterFn),
+      this.getDependantsForMany("task", module.taskNames, true, filterFn),
     ]))
   }
 

diff --git a/garden-service/src/process.ts b/garden-service/src/process.ts
@@ -64,6 +64,9 @@ export async function processServices(
 export async function processModules(
   { garden, log, modules, watch, handler, changeHandler }: ProcessModulesParams,
 ): Promise<ProcessResults> {
+
+  log.debug("Starting processModules")
+
   // Let the user know if any modules are linked to a local path
   const linkedModulesMsg = modules
     .filter(m => isModuleLinked(m, garden))
@@ -95,7 +98,7 @@ export async function processModules(
     changeHandler = handler
   }
 
-  const watcher = new FSWatcher(garden)
+  const watcher = new FSWatcher(garden, log)
 
   const restartPromise = new Promise(async (resolve) => {
     await watcher.watchModules(modules,
@@ -107,7 +110,7 @@ export async function processModules(
         }
 
         if (changedModule) {
-          log.debug({ msg: `Files changed for module ${changedModule.name}` })
+          log.silly({ msg: `Files changed for module ${changedModule.name}` })
 
           await Bluebird.map(changeHandler!(changedModule), (task) => garden.addTask(task))
         }

diff --git a/garden-service/src/task-graph.ts b/garden-service/src/task-graph.ts
@@ -79,6 +79,10 @@ export class TaskGraph {
 
     // this.taskDependencyCache will already have been populated at this point (happens in addTaskInternal).
     for (const node of taskNodes) {
+      /**
+       * We set the list of dependency nodes to the intersection of the set of nodes in this.index with
+       * the node's task's dependencies (from configuration).
+       */
       node.clear()
       const taskDeps = this.taskDependencyCache[node.getKey()] || new Set()
       node.setDependencies(taskNodes.filter(n => taskDeps.has(n.getBaseKey())))

diff --git a/garden-service/src/tasks/helpers.ts b/garden-service/src/tasks/helpers.ts
@@ -68,8 +68,11 @@ export async function getTasksForModule(
   const taskTasks = tasks
     .map(task => new TaskTask({ garden, log, task, force, forceBuild }))
 
-  return [...buildTasks, ...deployTasks, ...taskTasks]
+  const outputTasks = [...buildTasks, ...deployTasks, ...taskTasks]
+  log.silly(`getTasksForModule called for module ${module.name}, returning the following tasks:`)
+  log.silly(`  ${outputTasks.map(t => t.getBaseKey()).join(", ")}`)
 
+  return outputTasks
 }
 
 export async function getHotReloadModuleNames(garden: Garden, hotReloadServiceNames: string[]): Promise<Set<string>> {

diff --git a/garden-service/src/watch.ts b/garden-service/src/watch.ts
@@ -13,13 +13,14 @@ import { Module } from "./types/module"
 import { getIgnorer, scanDirectory } from "./util/util"
 import { MODULE_CONFIG_FILENAME } from "./constants"
 import { Garden } from "./garden"
+import { LogEntry } from "./logger/log-entry"
 
 export type ChangeHandler = (module: Module | null, configChanged: boolean) => Promise<void>
 
 export class FSWatcher {
   private watcher
 
-  constructor(private garden: Garden) {
+  constructor(private garden: Garden, private log: LogEntry) {
   }
 
   async watchModules(modules: Module[], changeHandler: ChangeHandler) {
@@ -53,6 +54,8 @@ export class FSWatcher {
 
     return async (filePath: string) => {
 
+      this.log.debug("Start of changeHandler")
+
       const filename = basename(filePath)
       if (filename === "garden.yml" || filename === ".gitignore" || filename === ".gardenignore") {
         await this.invalidateCachedForAll()

diff --git a/garden-service/test/data/test-project-dependants/service-dependant2/.dockerignore b/garden-service/test/data/test-project-dependants/service-dependant2/.dockerignore
@@ -0,0 +1,4 @@
+node_modules
+Dockerfile
+garden.yml
+app.yaml
diff --git a/garden-service/test/data/test-project-dependants/service-dependant2/Dockerfile b/garden-service/test/data/test-project-dependants/service-dependant2/Dockerfile
@@ -0,0 +1,2 @@
+FROM scratch
+ADD . /
diff --git a/garden-service/test/data/test-project-dependants/service-dependant2/garden.yml b/garden-service/test/data/test-project-dependants/service-dependant2/garden.yml
@@ -0,0 +1,19 @@
+module:
+  description: Service dependant 2
+  name: service-dependant2
+  type: container
+  services:
+    - name: service-dependant2
+      ports:
+        - name: http
+          containerPort: 8080
+      ingresses:
+        - path: /dependant
+          port: http
+      dependencies:
+        - good-morning
+  tasks:
+    - name: dependant-task2
+      command: [echo, dependant-task2]
+      dependencies:
+        - service-dependant2
diff --git a/garden-service/test/src/task-graph.ts b/garden-service/test/src/task-graph.ts
@@ -153,7 +153,7 @@ describe("task-graph", () => {
 
       const garden = await getGarden()
       const graph = new TaskGraph(garden, garden.log)
-      const task = new TestTask(garden, "a", { throwError: true })
+      const task = new TestTask(garden, "a", false, { throwError: true })
 
       await graph.addTask(task)
       const result = await graph.processTasks()
@@ -217,7 +217,8 @@ describe("task-graph", () => {
       const repeatTaskC = new TestTask(garden, "c", true, { ...repeatOpts, dependencies: [repeatTaskB], id: "c2" })
 
       const repeatTaskAforced = new TestTask(garden, "a", true, { ...repeatOpts, dependencies: [], id: "a2f" })
-      const repeatTaskBforced = new TestTask(garden, "b", true, { ...repeatOpts, dependencies: [repeatTaskA], id: "b2f" })
+      const repeatTaskBforced = new TestTask(garden, "b", true,
+        { ...repeatOpts, dependencies: [repeatTaskA], id: "b2f" })
 
       await graph.addTask(repeatTaskBforced)
       await graph.addTask(repeatTaskAforced)

diff --git a/garden-service/test/src/tasks/helpers.ts b/garden-service/test/src/tasks/helpers.ts
@@ -71,6 +71,9 @@ describe("TaskHelpers", () => {
 
             "deploy.service-dependant",
             "task.dependant-task",
+
+            "deploy.service-dependant2",
+            "task.dependant-task2",
           ].sort(),
           withDependencies: [
             "build.build-dependency",
@@ -90,6 +93,11 @@ describe("TaskHelpers", () => {
             "push.service-dependant",
             "deploy.service-dependant",
             "task.dependant-task",
+
+            "build.service-dependant2",
+            "push.service-dependant2",
+            "deploy.service-dependant2",
+            "task.dependant-task2",
           ].sort(),
         },
         {
@@ -104,6 +112,9 @@ describe("TaskHelpers", () => {
 
             "deploy.service-dependant",
             "task.dependant-task",
+
+            "deploy.service-dependant2",
+            "task.dependant-task2",
           ].sort(),
           withDependencies: [
             "build.build-dependency",
@@ -121,6 +132,11 @@ describe("TaskHelpers", () => {
             "push.service-dependant",
             "deploy.service-dependant",
             "task.dependant-task",
+
+            "build.service-dependant2",
+            "push.service-dependant2",
+            "deploy.service-dependant2",
+            "task.dependant-task2",
           ].sort(),
         },
         {
@@ -181,12 +197,23 @@ describe("TaskHelpers", () => {
         },
         {
           moduleName: "good-morning",
-          withoutDependencies: ["deploy.service-dependant", "task.dependant-task"],
+          withoutDependencies: [
+            "deploy.service-dependant",
+            "task.dependant-task",
+
+            "deploy.service-dependant2",
+            "task.dependant-task2",
+          ].sort(),
           withDependencies: [
             "build.service-dependant",
             "push.service-dependant",
             "deploy.service-dependant",
             "task.dependant-task",
+
+            "build.service-dependant2",
+            "push.service-dependant2",
+            "deploy.service-dependant2",
+            "task.dependant-task2",
           ].sort(),
         },
         {