From db72f2a88808a4cc9c656ea17dcacbd150763318 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Wed, 3 Jun 2020 12:53:31 +0200 Subject: [PATCH] perf(core): cache provider statuses for faster successive startup We now cache provider statuses for a default of one hour. You can run any command with `--force-refresh` to skip the caching, and can override the cache duration with the `GARDEN_CACHE_TTL=` environment variable. This substantially improves command execution times when run in succession, which will be very useful for day-to-day usage, as well CI performance. Closes #1824 --- docs/reference/commands.md | 1 + garden-service/src/cli/cli.ts | 6 + garden-service/src/config/status.ts | 9 ++ garden-service/src/garden.ts | 19 ++- .../src/plugins/terraform/commands.ts | 14 +- garden-service/src/plugins/terraform/init.ts | 3 +- garden-service/src/tasks/resolve-provider.ts | 115 +++++++++++++- .../plugin/provider/getEnvironmentStatus.ts | 2 + garden-service/src/util/util.ts | 5 +- garden-service/src/vcs/vcs.ts | 2 +- garden-service/test/helpers.ts | 7 +- garden-service/test/unit/src/garden.ts | 2 +- .../unit/src/plugins/terraform/terraform.ts | 7 +- .../test/unit/src/tasks/resolve-provider.ts | 149 ++++++++++++++++++ 14 files changed, 325 insertions(+), 16 deletions(-) diff --git a/docs/reference/commands.md b/docs/reference/commands.md index b064fda2bc..46bf643c54 100644 --- a/docs/reference/commands.md +++ b/docs/reference/commands.md @@ -30,6 +30,7 @@ quiet: suppresses all log output, same as --silent. | `--output` | `-o` | `json` `yaml` | Output command result in specified format (note: disables progress logging and interactive functionality). | `--emoji` | | boolean | Enable emoji in output (defaults to true if the environment supports it). | `--yes` | `-y` | boolean | Automatically approve any yes/no prompts during execution. + | `--force-refresh` | | boolean | Force refresh of any caches, e.g. cached provider statuses. ### garden build diff --git a/garden-service/src/cli/cli.ts b/garden-service/src/cli/cli.ts index 37cd8ddfe7..b2df704682 100644 --- a/garden-service/src/cli/cli.ts +++ b/garden-service/src/cli/cli.ts @@ -164,6 +164,10 @@ export const GLOBAL_OPTIONS = { help: "Automatically approve any yes/no prompts during execution.", defaultValue: false, }), + "force-refresh": new BooleanParameter({ + help: "Force refresh of any caches, e.g. cached provider statuses.", + defaultValue: false, + }), } export type GlobalOptions = typeof GLOBAL_OPTIONS @@ -290,6 +294,7 @@ export class GardenCli { "env": environmentName, silent, output, + "force-refresh": forceRefresh, } = parsedOpts let loggerType = loggerTypeOpt || command.getLoggerType({ opts: parsedOpts, args: parsedArgs }) @@ -323,6 +328,7 @@ export class GardenCli { environmentName, log, sessionId, + forceRefresh, } let garden: Garden diff --git a/garden-service/src/config/status.ts b/garden-service/src/config/status.ts index 1fc1a29951..c123d12939 100644 --- a/garden-service/src/config/status.ts +++ b/garden-service/src/config/status.ts @@ -61,5 +61,14 @@ export const environmentStatusSchema = () => outputs: joiVariables() .meta({ extendable: true }) .description("Output variables that modules and other variables can reference."), + disableCache: joi + .boolean() + .optional() + .description("Set to true to disable caching of the status."), + cached: joi + .boolean() + .optional() + .meta({ internal: true }) + .description("Indicates if the status was retrieved from cache by the framework."), }) .description("Description of an environment's status for a provider.") diff --git a/garden-service/src/garden.ts b/garden-service/src/garden.ts index cd39ced273..9c74348c9f 100644 --- a/garden-service/src/garden.ts +++ b/garden-service/src/garden.ts @@ -95,6 +95,7 @@ export interface GardenOpts { commandInfo?: CommandInfo gardenDirPath?: string environmentName?: string + forceRefresh?: boolean persistent?: boolean log?: LogEntry plugins?: RegisterPluginParam[] @@ -130,6 +131,7 @@ export interface GardenParams { username: string | undefined vcs: VcsHandler workingCopyId: string + forceRefresh?: boolean } @Profile() @@ -182,6 +184,7 @@ export class Garden { public readonly systemNamespace: string public readonly username?: string public readonly version: ModuleVersion + private readonly forceRefresh: boolean constructor(params: GardenParams) { this.buildDir = params.buildDir @@ -212,6 +215,7 @@ export class Garden { this.persistent = !!params.opts.persistent this.username = params.username this.vcs = params.vcs + this.forceRefresh = !!params.forceRefresh // make sure we're on a supported platform const currentPlatform = platform() @@ -386,6 +390,7 @@ export class Garden { log, username: _username, vcs, + forceRefresh: opts.forceRefresh, }) as InstanceType return garden @@ -613,6 +618,7 @@ export class Garden { plugin, config, version: this.version, + forceRefresh: this.forceRefresh, forceInit, }) }) @@ -634,6 +640,8 @@ export class Garden { providers = Object.values(taskResults).map((result) => result!.output) + const gotCachedResult = !!providers.find((p) => p.status.cached) + await Bluebird.map(providers, async (provider) => Bluebird.map(provider.moduleConfigs, async (moduleConfig) => { // Make sure module and all nested entities are scoped to the plugin @@ -646,7 +654,16 @@ export class Garden { this.resolvedProviders[provider.name] = provider } - log.setSuccess({ msg: chalk.green("Done"), append: true }) + if (gotCachedResult) { + log.setSuccess({ msg: chalk.green("Cached"), append: true }) + log.info({ + symbol: "info", + msg: chalk.gray("Run with --force-refresh to force a refresh of provider statuses."), + }) + } else { + log.setSuccess({ msg: chalk.green("Done"), append: true }) + } + this.log.silly(`Resolved providers: ${providers.map((p) => p.name).join(", ")}`) }) diff --git a/garden-service/src/plugins/terraform/commands.ts b/garden-service/src/plugins/terraform/commands.ts index b3d43c901b..a1923752ab 100644 --- a/garden-service/src/plugins/terraform/commands.ts +++ b/garden-service/src/plugins/terraform/commands.ts @@ -14,8 +14,10 @@ import { prepareVariables, tfValidate } from "./common" import { Module } from "../../types/module" import { findByName } from "../../util/util" import { TerraformModule } from "./module" -import { PluginCommand } from "../../types/plugin/command" +import { PluginCommand, PluginCommandParams } from "../../types/plugin/command" import { join } from "path" +import { remove } from "fs-extra" +import { getProviderStatusCachePath } from "../../tasks/resolve-provider" const commandsToWrap = ["apply", "plan"] const initCommand = chalk.bold("terraform init") @@ -32,7 +34,7 @@ function makeRootCommand(commandName: string) { name: commandName + "-root", description: `Runs ${terraformCommand} for the provider root stack, with the provider variables automatically configured as inputs. Positional arguments are passed to the command. If necessary, ${initCommand} is run first.`, title: chalk.bold.magenta(`Running ${chalk.white.bold(terraformCommand)} for project root stack`), - async handler({ ctx, args, log }) { + async handler({ ctx, args, log }: PluginCommandParams) { const provider = ctx.provider as TerraformProvider if (!provider.config.initRoot) { @@ -41,6 +43,14 @@ function makeRootCommand(commandName: string) { }) } + // Clear the provider status cache, to avoid any user confusion + const cachePath = getProviderStatusCachePath({ + gardenDirPath: ctx.gardenDirPath, + pluginName: provider.name, + environmentName: ctx.environmentName, + }) + await remove(cachePath) + const root = join(ctx.projectRoot, provider.config.initRoot) await tfValidate(log, provider, root, provider.config.variables) diff --git a/garden-service/src/plugins/terraform/init.ts b/garden-service/src/plugins/terraform/init.ts index ce2111385e..2edea4dce7 100644 --- a/garden-service/src/plugins/terraform/init.ts +++ b/garden-service/src/plugins/terraform/init.ts @@ -43,7 +43,8 @@ export async function getEnvironmentStatus({ ctx, log }: GetEnvironmentStatusPar `), }) const outputs = await getTfOutputs(log, tfVersion, root) - return { ready: true, outputs } + // Make sure the status is not cached when the stack is not up-to-date + return { ready: true, outputs, disableCache: true } } } else { return { ready: false, outputs: {} } diff --git a/garden-service/src/tasks/resolve-provider.ts b/garden-service/src/tasks/resolve-provider.ts index 4c1311bdc9..eb1e5c155a 100644 --- a/garden-service/src/tasks/resolve-provider.ts +++ b/garden-service/src/tasks/resolve-provider.ts @@ -6,6 +6,7 @@ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ +import stableStringify = require("json-stable-stringify") import chalk from "chalk" import { BaseTask, TaskParams, TaskType } from "./base" import { ProviderConfig, Provider, providerFromConfig, getProviderTemplateReferences } from "../config/provider" @@ -17,18 +18,36 @@ import { ProviderConfigContext } from "../config/config-context" import { ModuleConfig } from "../config/module" import { GardenPlugin } from "../types/plugin/plugin" import { joi } from "../config/common" -import { validateWithPath } from "../config/validation" +import { validateWithPath, validateSchema } from "../config/validation" import Bluebird from "bluebird" -import { defaultEnvironmentStatus } from "../types/plugin/provider/getEnvironmentStatus" +import { defaultEnvironmentStatus, EnvironmentStatus } from "../types/plugin/provider/getEnvironmentStatus" import { getPluginBases, getPluginBaseNames } from "../plugins" import { Profile } from "../util/profiling" +import { join, dirname } from "path" +import { readFile, writeFile, ensureDir } from "fs-extra" +import { deserialize, serialize } from "v8" +import { environmentStatusSchema } from "../config/status" +import { hashString } from "../util/util" interface Params extends TaskParams { plugin: GardenPlugin config: ProviderConfig + forceRefresh: boolean forceInit: boolean } +interface CachedStatus extends EnvironmentStatus { + configHash: string + resolvedAt: Date +} + +const cachedStatusSchema = environmentStatusSchema().keys({ + configHash: joi.string().required(), + resolvedAt: joi.date().required(), +}) + +const defaultCacheTtl = 3600 // 1 hour + /** * Resolves the configuration for the specified provider. */ @@ -38,12 +57,14 @@ export class ResolveProviderTask extends BaseTask { private config: ProviderConfig private plugin: GardenPlugin + private forceRefresh: boolean private forceInit: boolean constructor(params: Params) { super(params) this.config = params.config this.plugin = params.plugin + this.forceRefresh = params.forceRefresh this.forceInit = params.forceInit } @@ -96,6 +117,7 @@ export class ResolveProviderTask extends BaseTask { config, log: this.log, version: this.version, + forceRefresh: this.forceRefresh, forceInit: this.forceInit, }) }) @@ -197,6 +219,72 @@ export class ResolveProviderTask extends BaseTask { return providerFromConfig(resolvedConfig, resolvedProviders, moduleConfigs, status) } + private getCachePath() { + return getProviderStatusCachePath({ + gardenDirPath: this.garden.gardenDirPath, + pluginName: this.plugin.name, + environmentName: this.garden.environmentName, + }) + } + + private hashConfig(config: ProviderConfig) { + return hashString(stableStringify(config)) + } + + private async getCachedStatus(config: ProviderConfig): Promise { + const cachePath = this.getCachePath() + + this.log.silly(`Checking provider status cache for ${this.plugin.name} at ${cachePath}`) + + let cachedStatus: CachedStatus | null = null + + if (!this.forceRefresh) { + try { + const cachedData = deserialize(await readFile(cachePath)) + cachedStatus = validateSchema(cachedData, cachedStatusSchema) + } catch (err) { + // Can't find or read a cached status + this.log.silly(`Unable to find or read provider status from ${cachePath}: ${err.message}`) + } + } + + if (!cachedStatus) { + return null + } + + const configHash = this.hashConfig(config) + + if (cachedStatus.configHash !== configHash) { + this.log.silly(`Cached provider status at ${cachePath} does not match the current config`) + return null + } + + const ttl = process.env.GARDEN_CACHE_TTL ? parseInt(process.env.GARDEN_CACHE_TTL, 10) : defaultCacheTtl + const cacheAge = (new Date().getTime() - cachedStatus?.resolvedAt.getTime()) / 1000 + + if (cacheAge > ttl) { + this.log.silly(`Cached provider status at ${cachePath} is out of date`) + return null + } + + return omit(cachedStatus, ["configHash", "resolvedAt"]) + } + + private async setCachedStatus(config: ProviderConfig, status: EnvironmentStatus) { + const cachePath = this.getCachePath() + this.log.silly(`Caching provider status for ${this.plugin.name} at ${cachePath}`) + + const cachedStatus: CachedStatus = { + ...status, + cached: true, + resolvedAt: new Date(), + configHash: this.hashConfig(config), + } + + await ensureDir(dirname(cachePath)) + await writeFile(cachePath, serialize(cachedStatus)) + } + private async ensurePrepared(tmpProvider: Provider) { const pluginName = tmpProvider.name const actions = await this.garden.getActionRouter() @@ -204,6 +292,13 @@ export class ResolveProviderTask extends BaseTask { this.log.silly(`Getting status for ${pluginName}`) + // Check for cached provider status + const cachedStatus = await this.getCachedStatus(tmpProvider.config) + + if (cachedStatus) { + return cachedStatus + } + // TODO: avoid calling the handler manually (currently doing it to override the plugin context) const handler = await actions["getActionHandler"]({ actionType: "getEnvironmentStatus", @@ -245,6 +340,22 @@ export class ResolveProviderTask extends BaseTask { ) } + if (!status.disableCache) { + await this.setCachedStatus(tmpProvider.config, status) + } + return status } } + +export function getProviderStatusCachePath({ + gardenDirPath, + pluginName, + environmentName, +}: { + gardenDirPath: string + pluginName: string + environmentName: string +}) { + return join(gardenDirPath, "cache", "provider-statuses", `${pluginName}.${environmentName}.json`) +} diff --git a/garden-service/src/types/plugin/provider/getEnvironmentStatus.ts b/garden-service/src/types/plugin/provider/getEnvironmentStatus.ts index 39cd6d7c99..5cb2c32b59 100644 --- a/garden-service/src/types/plugin/provider/getEnvironmentStatus.ts +++ b/garden-service/src/types/plugin/provider/getEnvironmentStatus.ts @@ -17,6 +17,8 @@ export interface EnvironmentStatus { dashboardPages?: DashboardPage[] detail?: D outputs: O + disableCache?: boolean + cached?: boolean } export const defaultEnvironmentStatus: EnvironmentStatus = { diff --git a/garden-service/src/util/util.ts b/garden-service/src/util/util.ts index dccbef92aa..0d0c8f4406 100644 --- a/garden-service/src/util/util.ts +++ b/garden-service/src/util/util.ts @@ -546,10 +546,11 @@ export function findByNames(names: string[], entries: return entries.filter(({ name }) => names.includes(name)) } -export function hashString(s: string, length: number) { +export function hashString(s: string, length?: number): string { const urlHash = createHash("sha256") urlHash.update(s) - return urlHash.digest("hex").slice(0, length) + const str = urlHash.digest("hex") + return length ? str.slice(0, length) : str } /** diff --git a/garden-service/src/vcs/vcs.ts b/garden-service/src/vcs/vcs.ts index a9a73eb116..398e6800c8 100644 --- a/garden-service/src/vcs/vcs.ts +++ b/garden-service/src/vcs/vcs.ts @@ -250,7 +250,7 @@ export function hashVersions(moduleConfig: ModuleConfig, versions: NamedTreeVers return hashStrings([configString, ...versionStrings]) } -export function hashStrings(hashes: string[]) { +function hashStrings(hashes: string[]) { const versionHash = createHash("sha256") versionHash.update(hashes.join(".")) return versionHash.digest("hex").slice(0, 10) diff --git a/garden-service/test/helpers.ts b/garden-service/test/helpers.ts index 8254f85d69..7f5d358f45 100644 --- a/garden-service/test/helpers.ts +++ b/garden-service/test/helpers.ts @@ -399,14 +399,14 @@ export const makeTestGarden = async (projectRoot: string, opts: GardenOpts = {}) } export const makeTestGardenA = async (extraPlugins: RegisterPluginParam[] = []) => { - return makeTestGarden(projectRootA, { plugins: extraPlugins }) + return makeTestGarden(projectRootA, { plugins: extraPlugins, forceRefresh: true }) } export const makeTestGardenTasksFails = async (extraPlugins: RegisterPluginParam[] = []) => { return makeTestGarden(projectTestFailsRoot, { plugins: extraPlugins }) } -export function stubAction( +export async function stubAction( garden: Garden, pluginName: string, type: T, @@ -415,7 +415,8 @@ export function stubAction( if (handler) { handler["pluginName"] = pluginName } - return td.replace(garden["actionHandlers"][type], pluginName, handler) + const actions = await garden.getActionRouter() + return td.replace(actions["actionHandlers"][type], pluginName, handler) } export function stubModuleAction>( diff --git a/garden-service/test/unit/src/garden.ts b/garden-service/test/unit/src/garden.ts index e487308292..4b00e1a039 100644 --- a/garden-service/test/unit/src/garden.ts +++ b/garden-service/test/unit/src/garden.ts @@ -158,7 +158,7 @@ describe("Garden", () => { const projectRoot = join(dataDir, "test-project-templated") - const garden = await makeTestGarden(projectRoot) + const garden = await makeTestGarden(projectRoot, { forceRefresh: true }) delete process.env.TEST_PROVIDER_TYPE delete process.env.TEST_VARIABLE diff --git a/garden-service/test/unit/src/plugins/terraform/terraform.ts b/garden-service/test/unit/src/plugins/terraform/terraform.ts index 1a3b694e94..dbcb0eb3d3 100644 --- a/garden-service/test/unit/src/plugins/terraform/terraform.ts +++ b/garden-service/test/unit/src/plugins/terraform/terraform.ts @@ -41,7 +41,7 @@ describe("Terraform provider", () => { context("autoApply=false", () => { beforeEach(async () => { await reset() - garden = await makeTestGarden(testRoot, { environmentName: "prod" }) + garden = await makeTestGarden(testRoot, { environmentName: "prod", forceRefresh: true }) }) after(async () => { @@ -49,11 +49,12 @@ describe("Terraform provider", () => { }) it("should warn if stack is not up-to-date", async () => { - await garden.resolveProvider("terraform") + const provider = await garden.resolveProvider("terraform") const messages = getLogMessages(garden.log, (e) => e.level === LogLevel.warn) expect(messages).to.include( "Terraform stack is not up-to-date and autoApply is not enabled. Please run garden plugins terraform apply-root to make sure the stack is in the intended state." ) + expect(provider.status.disableCache).to.be.true }) it("should expose outputs to template contexts after applying", async () => { @@ -110,7 +111,7 @@ describe("Terraform provider", () => { context("autoApply=true", () => { before(async () => { await reset() - garden = await makeTestGarden(testRoot, { environmentName: "local" }) + garden = await makeTestGarden(testRoot, { environmentName: "local", forceRefresh: true }) }) after(async () => { diff --git a/garden-service/test/unit/src/tasks/resolve-provider.ts b/garden-service/test/unit/src/tasks/resolve-provider.ts index 5772a98f95..7eeaccb624 100644 --- a/garden-service/test/unit/src/tasks/resolve-provider.ts +++ b/garden-service/test/unit/src/tasks/resolve-provider.ts @@ -10,6 +10,155 @@ import { PluginMap, createGardenPlugin } from "../../../../src/types/plugin/plug import { getPluginBases } from "../../../../src/plugins" import { expect } from "chai" import { sortBy } from "lodash" +import { makeTempDir, TempDirectory, TestGarden, makeTestGarden, stubAction } from "../../../helpers" +import { DEFAULT_API_VERSION } from "../../../../src/constants" +import execa from "execa" +import { ResolveProviderTask } from "../../../../src/tasks/resolve-provider" +import { pathExists, writeFile, remove } from "fs-extra" +import { join } from "path" +import { serialize } from "v8" +import moment from "moment" + +describe("ResolveProviderTask", () => { + let tmpDir: TempDirectory + let garden: TestGarden + let task: ResolveProviderTask + + before(async () => { + tmpDir = await makeTempDir() + const path = tmpDir.path + + await execa("git", ["init"], { cwd: path }) + }) + + after(async () => { + await tmpDir.cleanup() + }) + + beforeEach(async () => { + await remove(join(tmpDir.path, "cache")) + + garden = await makeTestGarden(tmpDir.path, { + config: { + apiVersion: DEFAULT_API_VERSION, + kind: "Project", + name: "test", + path: tmpDir.path, + defaultEnvironment: "default", + dotIgnoreFiles: [], + environments: [{ name: "default", variables: {} }], + providers: [{ name: "test-plugin" }], + variables: {}, + }, + }) + + const plugin = await garden.getPlugin("test-plugin") + const config = garden.getRawProviderConfigs(["test-plugin"])[0] + + task = new ResolveProviderTask({ + garden, + log: garden.log, + plugin, + config, + version: garden.version, + forceRefresh: false, + forceInit: false, + }) + }) + + it("should resolve status if no cached status exists", async () => { + const provider = await task.process({}) + expect(provider.status.cached).to.be.undefined + }) + + it("should cache the provider status", async () => { + await task.process({}) + const cachePath = task["getCachePath"]() + expect(await pathExists(cachePath)).to.be.true + }) + + it("should not cache the provider status if disableCache=true", async () => { + await stubAction(garden, "test-plugin", "getEnvironmentStatus", async () => { + return { ready: true, disableCache: true, outputs: {} } + }) + await task.process({}) + const cachePath = task["getCachePath"]() + expect(await pathExists(cachePath)).to.be.true + }) + + it("should return with cached provider status if the config hash matches and TTL is within range", async () => { + await task.process({}) + const provider = await task.process({}) + expect(provider.status.cached).to.be.true + }) + + it("should not use cached status if the cached data is invalid", async () => { + const cachePath = task["getCachePath"]() + await writeFile(cachePath, serialize({ foo: "bla" })) + + const provider = await task.process({}) + expect(provider.status.cached).to.be.undefined + }) + + it("should not use cached status if the config hash doesn't match", async () => { + let provider = await task.process({}) + + const cachedStatus = await task["getCachedStatus"](provider.config) + + const cachePath = task["getCachePath"]() + await writeFile(cachePath, serialize({ ...cachedStatus, configHash: "abcdef", resolvedAt: new Date() })) + + provider = await task.process({}) + expect(provider.status.cached).to.be.undefined + }) + + it("should use cached status if the cache is just within the TTL", async () => { + let provider = await task.process({}) + + const cachedStatus = await task["getCachedStatus"](provider.config) + + // Just over one hour, which is the default TTL + const resolvedAt = moment() + .subtract(3500, "seconds") + .toDate() + + const configHash = task["hashConfig"](provider.config) + + const cachePath = task["getCachePath"]() + await writeFile(cachePath, serialize({ ...cachedStatus, configHash, resolvedAt })) + + provider = await task.process({}) + expect(provider.status.cached).to.be.true + }) + + it("should not use cached status if the cache is expired", async () => { + let provider = await task.process({}) + + const cachedStatus = await task["getCachedStatus"](provider.config) + + // Just over one hour, which is the default TTL + const resolvedAt = moment() + .subtract(3601, "seconds") + .toDate() + + const configHash = task["hashConfig"](provider.config) + + const cachePath = task["getCachePath"]() + await writeFile(cachePath, serialize({ ...cachedStatus, configHash, resolvedAt })) + + provider = await task.process({}) + expect(provider.status.cached).to.be.undefined + }) + + it("should not use cached status if forceRefresh=true", async () => { + await task.process({}) + + task["forceRefresh"] = true + + const provider = await task.process({}) + expect(provider.status.cached).to.be.undefined + }) +}) describe("getPluginBases", () => { it("should return an empty list if plugin has no base", () => {