Skip to content

Commit

Permalink
feat(build): Handle config changes in auto-reload.
Browse files Browse the repository at this point in the history
Before this change, each module directory was watched separately for
changes during auto-reload, and didn't properly handle module- or
project-level configuration changes since the auto-reload watch was
started.

Here, we consolidate the module-level watches into a single
project-level watch, and call the originating command with a fresh
PluginContext when configuration changes. This includes the addition or
removal of modules during the lifetime of the watch.
  • Loading branch information
thsig committed Jun 12, 2018
1 parent 12cb5e1 commit 9d9295f
Show file tree
Hide file tree
Showing 12 changed files with 367 additions and 184 deletions.
11 changes: 7 additions & 4 deletions src/cli/cli.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,10 +206,13 @@ export class GardenCli {
}

const logger = RootLogNode.initialize({ level, writers })
const garden = await Garden.factory(root, { env, logger })

// TODO: enforce that commands always output DeepPrimitiveMap
const result = await command.action(garden.pluginContext, parsedArgs, parsedOpts)
let garden
let result
do {
garden = await Garden.factory(root, { env, logger })
// TODO: enforce that commands always output DeepPrimitiveMap
result = await command.action(garden.pluginContext, parsedArgs, parsedOpts)
} while (result.restartRequired)

// We attach the action result to cli context so that we can process it in the parse method
cliContext.details.result = result
Expand Down
19 changes: 13 additions & 6 deletions src/commands/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
import { PluginContext } from "../plugin-context"
import { TaskResults } from "../task-graph"
import { LoggerType } from "../logger/types"
import { ProcessResults } from "../process"

export class ValidationError extends Error { }

Expand Down Expand Up @@ -128,6 +129,7 @@ export interface CommandConstructor {

export interface CommandResult<T = any> {
result?: T
restartRequired?: boolean
errors?: GardenError[]
}

Expand Down Expand Up @@ -172,19 +174,24 @@ export abstract class Command<T extends Parameters = {}, U extends Parameters =
}

export async function handleTaskResults(
ctx: PluginContext, taskType: string, result: TaskResults,
ctx: PluginContext, taskType: string, results: ProcessResults,
): Promise<CommandResult<TaskResults>> {
const failed = Object.values(result).filter(r => !!r.error).length
const failed = Object.values(results).filter(r => !!r.error).length

if (failed) {
const error = new RuntimeError(`${failed} ${taskType} task(s) failed!`, {
result,
results,
})
return { errors: [error] }
} else {
ctx.log.info("")
}

ctx.log.info("")
if (!results.restartRequired) {
ctx.log.header({ emoji: "heavy_check_mark", command: `Done!` })
return { result }
}
return {
result: results.taskResults,
restartRequired: results.restartRequired,
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
import { BuildTask } from "../tasks/build"
import { TaskResults } from "../task-graph"
import dedent = require("dedent")
import { processModules } from "../process"

export const buildArguments = {
module: new StringParameter({
Expand Down Expand Up @@ -59,8 +60,9 @@ export class BuildCommand extends Command<typeof buildArguments, typeof buildOpt

ctx.log.header({ emoji: "hammer", command: "Build" })

const results = await ctx.processModules({
const results = await processModules({
modules,
pluginContext: ctx,
watch: opts.watch,
process: async (module) => {
return [await BuildTask.factory({ ctx, module, force: opts.force })]
Expand Down
4 changes: 3 additions & 1 deletion src/commands/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {
StringParameter,
} from "./base"
import { TaskResults } from "../task-graph"
import { processServices } from "../process"

export const deployArgs = {
service: new StringParameter({
Expand Down Expand Up @@ -75,9 +76,10 @@ export class DeployCommand extends Command<typeof deployArgs, typeof deployOpts>
const force = opts.force
const forceBuild = opts["force-build"]

const results = await ctx.processServices({
const results = await processServices({
services,
watch,
pluginContext: ctx,
process: async (service) => {
return [await DeployTask.factory({ ctx, service, force, forceBuild })]
},
Expand Down
4 changes: 3 additions & 1 deletion src/commands/dev.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import { join } from "path"
import { STATIC_DIR } from "../constants"
import chalk from "chalk"
import moment = require("moment")
import { processModules } from "../process"

const imgcatPath = join(STATIC_DIR, "imgcat")
const bannerPath = join(STATIC_DIR, "garden-banner-1-half.png")
Expand Down Expand Up @@ -60,8 +61,9 @@ export class DevCommand extends Command {
return {}
}

await ctx.processModules({
await processModules({
modules,
pluginContext: ctx,
watch: true,
process: async (module) => {
const testTasks: Task[] = await module.getTestTasks({})
Expand Down
4 changes: 2 additions & 2 deletions src/commands/push.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,9 +65,9 @@ export class PushCommand extends Command<typeof pushArgs, typeof pushOpts> {
const names = args.module ? args.module.split(",") : undefined
const modules = await ctx.getModules(names)

const result = await pushModules(ctx, modules, !!opts["force-build"], !!opts["allow-dirty"])
const results = await pushModules(ctx, modules, !!opts["force-build"], !!opts["allow-dirty"])

return handleTaskResults(ctx, "push", result)
return handleTaskResults(ctx, "push", { taskResults: results })
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/commands/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
CommandResult,
} from "./base"
import { TaskResults } from "../task-graph"
import { processModules } from "../process"

export const testArgs = {
module: new StringParameter({
Expand Down Expand Up @@ -75,8 +76,9 @@ export class TestCommand extends Command<typeof testArgs, typeof testOpts> {
const force = opts.force
const forceBuild = opts["force-build"]

const results = await ctx.processModules({
const results = await processModules({
modules,
pluginContext: ctx,
watch: opts.watch,
process: async (module) => module.getTestTasks({ name, force, forceBuild }),
})
Expand Down
103 changes: 0 additions & 103 deletions src/plugin-context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
Garden,
} from "./garden"
import { EntryStyle } from "./logger/types"
import { TaskResults } from "./task-graph"
import {
PrimitiveMap,
validate,
Expand Down Expand Up @@ -82,24 +81,13 @@ import {
mapValues,
toPairs,
values,
padEnd,
keyBy,
omit,
flatten,
} from "lodash"
import { Task } from "./types/task"
import {
getNames,
Omit,
registerCleanupFunction,
sleep,
} from "./util"
import { TreeVersion } from "./vcs/base"
import {
autoReloadModules,
computeAutoReloadDependants,
FSWatcher,
} from "./watch"

export type PluginContextGuard = {
readonly [P in keyof (PluginActionParams | ModuleActionParams<any>)]: (...args: any[]) => Promise<any>
Expand Down Expand Up @@ -133,18 +121,6 @@ export type WrappedFromGarden = Pick<Garden,
"addTask" |
"processTasks">

export interface ProcessModulesParams {
modules: Module[],
watch: boolean,
process: (module: Module) => Promise<Task[]>,
}

export interface ProcessServicesParams {
services: Service[],
watch: boolean,
process: (service: Service) => Promise<Task[]>,
}

export interface PluginContext extends PluginContextGuard, WrappedFromGarden {
getEnvironmentStatus: (params: {}) => Promise<EnvironmentStatusMap>
configureEnvironment: (params: { force?: boolean }) => Promise<EnvironmentStatusMap>
Expand Down Expand Up @@ -189,8 +165,6 @@ export interface PluginContext extends PluginContextGuard, WrappedFromGarden {
getModuleVersion: (module: Module, force?: boolean) => Promise<TreeVersion>
stageBuild: (moduleName: string) => Promise<void>
getStatus: () => Promise<ContextStatus>
processModules: (params: ProcessModulesParams) => Promise<TaskResults>
processServices: (params: ProcessServicesParams) => Promise<TaskResults>
}

export function createPluginContext(garden: Garden): PluginContext {
Expand Down Expand Up @@ -524,83 +498,6 @@ export function createPluginContext(garden: Garden): PluginContext {
}
},

processModules: async ({ modules, watch, process }: ProcessModulesParams) => {
// TODO: log errors as they happen, instead of after processing all tasks
const logErrors = (taskResults: TaskResults) => {
for (const result of values(taskResults).filter(r => !!r.error)) {
const divider = padEnd("", 80, "—")

ctx.log.error(`\nFailed ${result.description}. Here is the output:`)
ctx.log.error(divider)
ctx.log.error(result.error + "")
ctx.log.error(divider + "\n")
}
}

for (const module of modules) {
const tasks = await process(module)
await Bluebird.map(tasks, ctx.addTask)
}

const results = await ctx.processTasks()
logErrors(results)

if (!watch) {
return results
}

const modulesToWatch = await autoReloadModules(modules)
const autoReloadDependants = await computeAutoReloadDependants(ctx)

async function handleChanges(module: Module) {
const tasks = await process(module)
await Bluebird.map(tasks, ctx.addTask)

const dependantsForModule = autoReloadDependants[module.name]
if (!dependantsForModule) {
return
}

for (const dependant of dependantsForModule) {
await handleChanges(dependant)
}
}

const watcher = new FSWatcher(ctx)

// TODO: should the prefix here be different or set explicitly per run?
await watcher.watchModules(modulesToWatch, "addTasksForAutoReload/",
async (changedModule) => {
ctx.log.debug({ msg: `Files changed for module ${changedModule.name}` })
await handleChanges(changedModule)
logErrors(await ctx.processTasks())
})

registerCleanupFunction("clearAutoReloadWatches", () => {
watcher.end()
ctx.log.info({ msg: "\nDone!" })
})

while (true) {
await sleep(1000)
}
},

processServices: async ({ services, watch, process }: ProcessServicesParams) => {
const serviceNames = getNames(services)
const modules = Array.from(new Set(services.map(s => s.module)))

return ctx.processModules({
modules,
watch,
process: async (module) => {
const moduleServices = await module.getServices()
const servicesToDeploy = moduleServices.filter(s => serviceNames.includes(s.name))
return flatten(await Bluebird.map(servicesToDeploy, process))
},
})
},

//endregion
}

Expand Down
6 changes: 4 additions & 2 deletions src/plugins/kubernetes/local.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ import {
isSystemGarden,
} from "./system"
import { readFile } from "fs-extra"
import { processServices } from "../../process"

// TODO: split this into separate plugins to handle Docker for Mac and Minikube

Expand Down Expand Up @@ -96,15 +97,16 @@ async function configureLocalEnvironment(

const services = await sysCtx.getServices(provider.config._systemServices)

const results = await sysCtx.processServices({
const results = await processServices({
services,
pluginContext: ctx,
watch: false,
process: async (service) => {
return [await DeployTask.factory({ ctx: sysCtx, service, force, forceBuild: false })]
},
})

const failed = values(results).filter(r => !!r.error).length
const failed = values(results.taskResults).filter(r => !!r.error).length

if (failed) {
throw new PluginError(`local-kubernetes: ${failed} errors occurred when configuring environments`, {
Expand Down
Loading

0 comments on commit 9d9295f

Please sign in to comment.