fix: docs, comments & various fixes

* Fixed bug in task execution.

* Added more comments to DependencyGraph's methods and fixed formatting.

* Use healthCheck for postgres service in the tasks example project.

* Added a README for the tasks example project.

* Clarify task helper tests.

docs/reference/commands.md

@@ -498,7 +498,7 @@ Examples:
 
 Run a task (in the context of its parent module).
 
-This is useful for re-running tasks on the go, for example after writing/modifying database migrations.
+This is useful for re-running tasks ad-hoc, for example after writing/modifying database migrations.
 
 Examples:
 

docs/reference/config.md

@@ -684,7 +684,7 @@ module:
   #
   # Optional.
   tasks: 
-    # The task specification for a generic module.
+    # A task that can be run in this module.
     #
     # Optional.
     - # The name of the task.

examples/tasks/README.md

@@ -0,0 +1,43 @@
+# Tasks example project
+
+This example uses dependency-aware database migrations to demonstrate Garden's _tasks_ functionality.
+
+Tasks are defined under `tasks` in a module's `garden.yml`. They're currently only supported for `container` modules, and consist of a `name`, a `command` and `dependencies`.
+
+In short, a task is a _command that is run inside an ad-hoc container instance of the module_.
+
+Tasks may depend on other tasks having been run and/or other services being deployed before they themselves are run (the names of which are listed under the task's `dependencies` field).
+
+## Structure of this project
+
+This project consists of three modules:
+
+- `postgres` — a minimally configured PostgreSQL service with a simple health check
+- `hello` — a simple JS/Node service
+- `user` — a simple Ruby/Sinatra service
+
+There are two tasks defined in this project:
+- `node-migration` (defined in `hello`), which creates a `users` table, and
+- `ruby-migration` (defined in `user`), which inserts a few records into the `users` table.
+
+Before `node-migration` can be run, the database has to be up and running, therefore `postgres` is a service dependency of `node-migration`. And before `ruby-migration` can insert records into the `users` table, that table has to exist. `ruby-migration` also requires the database to be up and running, but that's already required by its dependency, `node-migration`, so there's no need for `ruby-migration` to directly depend on `postgres`.
+
+Garden takes care of deploying the project's services and running the project's tasks in the specified dependency order:
+
+When this project is `garden deploy`-ed, `node-migration` is run once `postgres` is up.
+
+Once `node-migration` finishes, `hello` is deployed and `ruby-migration` is run. When ruby-migration finishes, `user` is deployed.
+
+## Usage
+
+The simplest way to see this in action is to run `garden deploy` or `garden dev` in the project's top-level directory.
+
+Run `garden call hello`, and you should see the following output:
+```sh
+$ garden call hello
+✔ Sending HTTP GET request to http://tasks.local.app.garden/hello
+
+200 OK
+
+Hello from Node! Usernames: John, Paul, George, Ringo
+```

examples/tasks/hello-service/Dockerfile → examples/tasks/hello/Dockerfile

@@ -1,4 +1,4 @@
-FROM node:9-alpine
+FROM node:10-alpine
 
 ENV PORT=8080
 EXPOSE ${PORT}

examples/tasks/hello-service/app.js → examples/tasks/hello/app.js

@@ -2,7 +2,7 @@ const express = require("express")
 const knex = require("knex")({
   client: "postgresql",
   connection: {
-    host: "postgres-service",
+    host: "postgres",
     port: 5432,
     database: "postgres",
     user: "postgres",

examples/tasks/hello-service/garden.yml → examples/tasks/hello/garden.yml

@@ -1,9 +1,9 @@
 module:
-  name: hello-service
+  name: hello
   description: Greeting service
   type: container
   services:
-    - name: hello-service
+    - name: hello
       command: [npm, start]
       ports:
         - name: http
@@ -18,7 +18,6 @@ module:
       command: [npm, test]
   tasks:
     - name: node-migration
-      # sleep to give postgres time to get ready
-      command: ["sleep 5 && knex migrate:latest"]
+      command: [knex, migrate:latest]
       dependencies:
-        - postgres-service
+        - postgres

examples/tasks/hello-service/knexfile.js → examples/tasks/hello/knexfile.js

@@ -2,7 +2,7 @@ module.exports = {
   development: {
     client: "postgresql",
     connection: {
-      host: "postgres-service",
+      host: "postgres",
       port: 5432,
       database: "postgres",
       user: "postgres",

examples/tasks/postgres/garden.yml

@@ -4,10 +4,12 @@ module:
   name: postgres
   image: postgres:9.4
   services:
-    - name: postgres-service
+    - name: postgres
       volumes:
         - name: data
           containerPath: /db-data
       ports:
         - name: db
           containerPort: 5432
+      healthCheck:
+        command: [psql, -w, -U, postgres, -d, postgres, -c, "SELECT 1"]

examples/tasks/user-service/config/database.yml → examples/tasks/user/config/database.yml

@@ -1,6 +1,6 @@
 development:
   adapter: postgresql
   port: 5432
-  host: postgres-service
+  host: postgres
   database: postgres
   username: postgres

examples/tasks/user-service/garden.yml → examples/tasks/user/garden.yml

@@ -1,9 +1,9 @@
 module:
-  name: user-service
+  name: user
   description: User-listing service written in Ruby
   type: container
   services:
-    - name: user-service
+    - name: user
       command: [ruby, app.rb]
       ports:
         - name: http

garden-service/src/commands/run/task.ts

@@ -7,7 +7,6 @@
  */
 
 import chalk from "chalk"
-import { RunResult } from "../../types/plugin/outputs"
 import {
   BooleanParameter,
   Command,
@@ -23,6 +22,7 @@ import { printRuntimeContext } from "./run"
 import dedent = require("dedent")
 import { prepareRuntimeContext } from "../../types/service"
 import { TaskTask } from "../../tasks/task"
+import { TaskResult } from "../../task-graph"
 
 const runArgs = {
   task: new StringParameter({
@@ -44,7 +44,7 @@ export class RunTaskCommand extends Command<Args, Opts> {
   help = "Run a task (in the context of its parent module)."
 
   description = dedent`
-    This is useful for re-running tasks on the go, for example after writing/modifying database migrations.
+    This is useful for re-running tasks ad-hoc, for example after writing/modifying database migrations.
 
     Examples:
 
@@ -54,7 +54,7 @@ export class RunTaskCommand extends Command<Args, Opts> {
   arguments = runArgs
   options = runOpts
 
-  async action({ garden, args, opts }: CommandParams<Args, Opts>): Promise<CommandResult<RunResult>> {
+  async action({ garden, args, opts }: CommandParams<Args, Opts>): Promise<CommandResult<TaskResult>> {
     const task = await garden.getTask(args.task)
     const module = task.module
 
@@ -66,12 +66,9 @@ export class RunTaskCommand extends Command<Args, Opts> {
     })
 
     await garden.actions.prepareEnvironment({})
-
     const taskTask = new TaskTask({ garden, task, force: true, forceBuild: opts["force-build"] })
-    for (const depTask of await taskTask.getDependencies()) {
-      await garden.addTask(depTask)
-    }
-    await garden.processTasks()
+    await garden.addTask(taskTask)
+    const result = (await garden.processTasks())[taskTask.getBaseKey()]
 
     // combine all dependencies for all services in the module, to be sure we have all the context we need
     const depNames = uniq(flatten(module.serviceConfigs.map(s => s.dependencies)))
@@ -82,11 +79,7 @@ export class RunTaskCommand extends Command<Args, Opts> {
     printRuntimeContext(garden, runtimeContext)
 
     garden.log.info("")
-
-    const result = await garden.actions.runTask({ task, runtimeContext, interactive: true })
-
-    garden.log.info(chalk.white(result.output))
-
+    garden.log.info(chalk.white(result.output.output))
     garden.log.info("")
     garden.log.header({ emoji: "heavy_check_mark", command: `Done!` })
 

garden-service/src/dependency-graph.ts

@@ -16,6 +16,7 @@ import { Task } from "./types/task"
 import { TestConfig } from "./config/test"
 import { uniqByName } from "./util/util"
 
+// Each of these types corresponds to a Task class (e.g. BuildTask, DeployTask, ...).
 export type DependencyGraphNodeType = "build" | "service" | "task" | "test"
   | "push" | "publish" // these two types are currently not represented in DependencyGraph
 
@@ -36,6 +37,10 @@ type DependencyRelationNames = {
 
 export type DependencyRelationFilterFn = (DependencyGraphNode) => boolean
 
+/**
+ * A graph data structure that facilitates querying (recursive or non-recursive) of the project's dependency and
+ * dependant relationships.
+ */
 export class DependencyGraph {
 
   index: { [key: string]: DependencyGraphNode }
@@ -126,14 +131,13 @@ export class DependencyGraph {
     return getModuleKey(module.name, module.plugin)
   }
 
-  /**
+  /*
    * If filterFn is provided to any of the methods below that accept it, matching nodes
    * (and their dependencies/dependants, if recursive = true) are ignored.
    */
 
   /**
-   * Returns the set union of modules with the set union of their dependants (across all dependency types).
-   * Recursive.
+   * Returns the set union of modules with the set union of their dependants (across all dependency types, recursively).
    */
   async withDependantModules(modules: Module[], filterFn?: DependencyRelationFilterFn): Promise<Module[]> {
     const dependants = flatten(await Bluebird.map(modules, m => this.getDependantsForModule(m, filterFn)))
@@ -143,39 +147,62 @@ export class DependencyGraph {
     return this.garden.getModules(uniq(modules.concat(dependantModules).map(m => m.name)))
   }
 
-  // Recursive.
+  /**
+   * 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("build", module.build.dependencies.map(d => d.name), true, filterFn),
       this.getDependantsForMany("service", serviceNames, true, filterFn),
       this.getDependantsForMany("task", taskNames, true, filterFn),
     ]))
   }
 
+  /**
+   * Returns all dependencies of a node in DependencyGraph. As noted above, each DependencyGraphNodeType corresponds
+   * to a Task class (e.g. BuildTask, DeployTask, ...), and name corresponds to the value returned by its getName
+   * instance method.
+   *
+   * If recursive = true, also includes those dependencies' dependencies, etc.
+   */
   async getDependencies(
     nodeType: DependencyGraphNodeType, name: string, recursive: boolean, filterFn?: DependencyRelationFilterFn,
   ): Promise<DependencyRelations> {
     return this.toRelations(this.getDependencyNodes(nodeType, name, recursive, filterFn))
   }
 
+  /**
+   * Returns all dependants of a node in DependencyGraph. As noted above, each DependencyGraphNodeType corresponds
+   * to a Task class (e.g. BuildTask, DeployTask, ...), and name corresponds to the value returned by its getName
+   * instance method.
+   *
+   * If recursive = true, also includes those dependants' dependants, etc.
+   */
   async getDependants(
     nodeType: DependencyGraphNodeType, name: string, recursive: boolean, filterFn?: DependencyRelationFilterFn,
   ): Promise<DependencyRelations> {
     return this.toRelations(this.getDependantNodes(nodeType, name, recursive, filterFn))
   }
 
+  /**
+   * Same as getDependencies above, but returns the set union of the dependencies of the nodes in the graph
+   * having type = nodeType and name = name (computed recursively or shallowly for all).
+   */
   async getDependenciesForMany(
     nodeType: DependencyGraphNodeType, names: string[], recursive: boolean, filterFn?: DependencyRelationFilterFn,
   ): Promise<DependencyRelations> {
     return this.toRelations(flatten(
       names.map(name => this.getDependencyNodes(nodeType, name, recursive, filterFn))))
   }
 
+  /**
+   * Same as getDependants above, but returns the set union of the dependants of the nodes in the graph
+   * having type = nodeType and name = name (computed recursively or shallowly for all).
+   */
   async getDependantsForMany(
     nodeType: DependencyGraphNodeType, names: string[], recursive: boolean, filterFn?: DependencyRelationFilterFn,
   ): Promise<DependencyRelations> {
@@ -184,8 +211,7 @@ export class DependencyGraph {
   }
 
   /**
-   * Computes the set union for each node type across relationArr (i.e. concatenates
-   * and deduplicates for each key).
+   * Returns the set union for each node type across relationArr (i.e. concatenates and deduplicates for each key).
    */
   async mergeRelations(...relationArr: DependencyRelations[]): Promise<DependencyRelations> {
     const names = {}
@@ -201,6 +227,9 @@ export class DependencyGraph {
     })
   }
 
+  /**
+   * Returns the (unique by name) list of modules represented in relations.
+   */
   async modulesForRelations(relations: DependencyRelations): Promise<Module[]> {
     const moduleNames = uniq(flatten([
       relations.build,

garden-service/src/plugins/generic.ts

@@ -62,7 +62,7 @@ export const genericTaskSpecSchema = baseTaskSpecSchema
     command: Joi.array().items(Joi.string())
       .description("The command to run in the module build context."),
   })
-  .description("The task specification for a generic module.")
+  .description("A task that can be run in this module.")
 
 export interface GenericModuleSpec extends ModuleSpec {
   env: { [key: string]: string },
@@ -173,14 +173,8 @@ export async function runGenericTask(params: RunTaskParams): Promise<RunTaskResu
   const command = task.spec.command
   const startedAt = new Date()
 
-  const result = {
-    moduleName: module.name,
-    taskName: task.name,
-    command,
-    version: module.version,
-    success: true,
-    startedAt,
-  }
+  let completedAt
+  let output
 
   if (command && command.length) {
     const commandResult = await execa.shell(
@@ -191,14 +185,23 @@ export async function runGenericTask(params: RunTaskParams): Promise<RunTaskResu
       },
     )
 
-    result["completedAt"] = new Date()
-    result["output"] = commandResult.stdout + commandResult.stderr
+    completedAt = new Date()
+    output = commandResult.stdout + commandResult.stderr
   } else {
-    result["completedAt"] = startedAt
-    result["output"] = ""
+    completedAt = startedAt
+    output = ""
   }
 
-  return <RunTaskResult>{ ...result }
+  return <RunTaskResult>{
+    moduleName: module.name,
+    taskName: task.name,
+    command,
+    version: module.version,
+    success: true,
+    output,
+    startedAt,
+    completedAt,
+  }
 }
 
 export async function getGenericTaskStatus(): Promise<TaskStatus> {

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

@@ -70,7 +70,7 @@ export class Kubectl {
       opts.push("--tty")
     }
 
-    return args.concat(opts)
+    return opts.concat(args)
   }
 }