Skip to content

Commit

Permalink
improvement(core): more granular version hashes
Browse files Browse the repository at this point in the history
This change addresses a long-standing issue where changes to runtime
configuration (e.g. services, test and tasks) would also impact build
versions of container images.

We now tackle versioning more specifically for each entity within any
given module. Module types can control which parts of a configuration
should affect the build version, which is applied in different ways by
different module type handlers.

This change does mean that service, task and test versions are _always_
different from module (i.e. build) versions. This is because the
versions are now computed with the service/test/task configuration
hashed on top of the module configuration, even if there is no natural
distinction between the build version and a runtime version for that
particular module.

This generally has no practical downsides except that users might be
expecting versions to match, which they would previously _sometimes_ do,
e.g. a container service would have the same version as its module—which
is in fact the problem we're solving here.

I've added a documentation section and an FAQ entry, but we should also
mention this in release notes and in the community when announcing the
change.
  • Loading branch information
edvald authored and thsig committed Mar 19, 2021
1 parent 49b9982 commit d6f1373
Show file tree
Hide file tree
Showing 124 changed files with 1,390 additions and 1,133 deletions.
1 change: 1 addition & 0 deletions core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,7 @@
"build": "gulp pegjs",
"check-package-lock": "git diff-index --quiet HEAD -- yarn.lock || (echo 'yarn.lock is dirty!' && exit 1)",
"clean": "shx rm -rf build",
"dev": "gulp pegjs-watch",
"fix-format": "prettier --write \"{src,test}/**/*.ts\"",
"lint": "tslint -p .",
"migration:generate": "typeorm migration:generate --config ormconfig.js -n",
Expand Down
16 changes: 8 additions & 8 deletions core/src/actions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ import { HotReloadServiceParams, HotReloadServiceResult } from "./types/plugin/s
import { RunServiceParams } from "./types/plugin/service/runService"
import { GetTaskResultParams } from "./types/plugin/task/getTaskResult"
import { RunTaskParams, RunTaskResult } from "./types/plugin/task/runTask"
import { ServiceStatus, ServiceStatusMap, ServiceState, Service } from "./types/service"
import { ServiceStatus, ServiceStatusMap, ServiceState, GardenService } from "./types/service"
import { Omit, getNames } from "./util/util"
import { DebugInfoMap } from "./types/plugin/provider/getDebugInfo"
import { PrepareEnvironmentParams, PrepareEnvironmentResult } from "./types/plugin/provider/prepareEnvironment"
Expand All @@ -92,7 +92,7 @@ import { getServiceStatuses } from "./tasks/base"
import { getRuntimeTemplateReferences, resolveTemplateStrings } from "./template-string"
import { getPluginBases, getPluginDependencies, getModuleTypeBases } from "./plugins"
import { ConfigureProviderParams, ConfigureProviderResult } from "./types/plugin/provider/configureProvider"
import { Task } from "./types/task"
import { GardenTask } from "./types/task"
import { ConfigureModuleParams, ConfigureModuleResult } from "./types/plugin/module/configure"
import { PluginContext } from "./plugin-context"
import { DeleteServiceTask, deletedServiceStatuses } from "./tasks/delete-service"
Expand Down Expand Up @@ -376,7 +376,7 @@ export class ActionRouter implements TypeGuard {
try {
const result = await this.callModuleHandler({ params: { ...params, artifactsPath }, actionType: "testModule" })
this.garden.events.emit("testStatus", {
testName: params.testConfig.name,
testName: params.test.name,
moduleName: params.module.name,
status: runStatus(result),
})
Expand All @@ -387,7 +387,7 @@ export class ActionRouter implements TypeGuard {
await this.copyArtifacts(
params.log,
artifactsPath,
getArtifactKey("test", params.testConfig.name, params.module.version.versionString)
getArtifactKey("test", params.test.name, params.test.version)
)
} finally {
await tmpDir.cleanup()
Expand All @@ -404,7 +404,7 @@ export class ActionRouter implements TypeGuard {
defaultHandler: async () => null,
})
this.garden.events.emit("testStatus", {
testName: params.testName,
testName: params.test.name,
moduleName: params.module.name,
status: runStatus(result),
})
Expand Down Expand Up @@ -437,7 +437,7 @@ export class ActionRouter implements TypeGuard {
return result
}

private validateServiceOutputs(service: Service, result: ServiceStatus) {
private validateServiceOutputs(service: GardenService, result: ServiceStatus) {
const spec = this.moduleTypes[service.module.type]

if (spec.serviceOutputsSchema) {
Expand Down Expand Up @@ -547,7 +547,7 @@ export class ActionRouter implements TypeGuard {
await this.copyArtifacts(
params.log,
artifactsPath,
getArtifactKey("task", params.task.name, params.task.module.version.versionString)
getArtifactKey("task", params.task.name, params.task.version)
)
} finally {
await tmpDir.cleanup()
Expand All @@ -569,7 +569,7 @@ export class ActionRouter implements TypeGuard {
return result
}

private validateTaskOutputs(task: Task, result: RunTaskResult) {
private validateTaskOutputs(task: GardenTask, result: RunTaskResult) {
const spec = this.moduleTypes[task.module.type]

if (spec.taskOutputsSchema) {
Expand Down
14 changes: 5 additions & 9 deletions core/src/commands/get/get-status.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import { deepFilter } from "../../util/util"
import { Command, CommandResult, CommandParams } from "../base"
import { Garden } from "../../garden"
import { ConfigGraph } from "../../config-graph"
import { getTaskVersion } from "../../tasks/task"
import { LogEntry } from "../../logger/log-entry"
import { getTestVersion } from "../../tasks/test"
import { runStatus, RunStatus } from "../../types/plugin/base"
import chalk from "chalk"
import { deline } from "../../util/string"
Expand All @@ -23,6 +21,7 @@ import { ServiceStatus, serviceStatusSchema } from "../../types/service"
import { joi, joiIdentifierMap, joiStringMap } from "../../config/common"
import { environmentStatusSchema } from "../../config/status"
import { printHeader } from "../../logger/util"
import { testFromConfig } from "../../types/test"

export interface TestStatuses {
[testKey: string]: RunStatus
Expand Down Expand Up @@ -116,19 +115,17 @@ export class GetStatusCommand extends Command {
}

async function getTestStatuses(garden: Garden, configGraph: ConfigGraph, log: LogEntry) {
const modules = await configGraph.getModules()
const modules = configGraph.getModules()
const actions = await garden.getActionRouter()

return fromPairs(
flatten(
await Bluebird.map(modules, async (module) => {
return Bluebird.map(module.testConfigs, async (testConfig) => {
const testVersion = await getTestVersion(garden, configGraph, module, testConfig)
const result = await actions.getTestResult({
module,
log,
testVersion,
testName: testConfig.name,
test: testFromConfig(module, testConfig),
})
return [`${module.name}.${testConfig.name}`, runStatus(result)]
})
Expand All @@ -138,13 +135,12 @@ async function getTestStatuses(garden: Garden, configGraph: ConfigGraph, log: Lo
}

async function getTaskStatuses(garden: Garden, configGraph: ConfigGraph, log: LogEntry): Promise<TaskStatuses> {
const tasks = await configGraph.getTasks()
const tasks = configGraph.getTasks()
const actions = await garden.getActionRouter()

return fromPairs(
await Bluebird.map(tasks, async (task) => {
const taskVersion = await getTaskVersion(garden, configGraph, task)
const result = await actions.getTaskResult({ task, taskVersion, log })
const result = await actions.getTaskResult({ task, log })
return [task.name, runStatus(result)]
})
)
Expand Down
4 changes: 1 addition & 3 deletions core/src/commands/get/get-task-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import { ConfigGraph } from "../../config-graph"
import { Command, CommandResult, CommandParams } from "../base"
import { printHeader } from "../../logger/util"
import { getTaskVersion } from "../../tasks/task"
import { RunTaskResult } from "../../types/plugin/task/runTask"
import chalk from "chalk"
import { getArtifactFileList, getArtifactKey } from "../../util/artifacts"
Expand Down Expand Up @@ -64,14 +63,13 @@ export class GetTaskResultCommand extends Command<Args> {
const taskResult = await actions.getTaskResult({
log,
task,
taskVersion: await getTaskVersion(garden, graph, task),
})

let result: GetTaskResultCommandResult = null

if (taskResult) {
const artifacts = await getArtifactFileList({
key: getArtifactKey("task", task.name, task.module.version.versionString),
key: getArtifactKey("task", task.name, task.version),
artifactsPath: garden.artifactsPath,
log: garden.log,
})
Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/get/get-tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import indentString from "indent-string"
import { sortBy, omit, uniq } from "lodash"
import { Command, CommandResult, CommandParams } from "../base"
import { printHeader } from "../../logger/util"
import { Task } from "../../types/task"
import { GardenTask } from "../../types/task"
import { StringsParameter } from "../../cli/params"

const getTasksArgs = {
Expand All @@ -22,7 +22,7 @@ const getTasksArgs = {

type Args = typeof getTasksArgs

export function prettyPrintTask(task: Task): string {
export function prettyPrintTask(task: GardenTask): string {
let out = `${chalk.cyan.bold(task.name)}`

if (task.spec.args || task.spec.args === null) {
Expand Down
22 changes: 4 additions & 18 deletions core/src/commands/get/get-test-result.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,13 @@
*/

import { Command, CommandResult, CommandParams } from "../base"
import { NotFoundError } from "../../exceptions"
import { TestResult, testResultSchema } from "../../types/plugin/module/getTestResult"
import { getTestVersion } from "../../tasks/test"
import { findByName, getNames } from "../../util/util"
import { printHeader } from "../../logger/util"
import chalk from "chalk"
import { getArtifactFileList, getArtifactKey } from "../../util/artifacts"
import { joi, joiArray } from "../../config/common"
import { StringParameter } from "../../cli/params"
import { testFromModule } from "../../types/test"

const getTestResultArgs = {
module: new StringParameter({
Expand Down Expand Up @@ -71,31 +69,19 @@ export class GetTestResultCommand extends Command<Args> {
const actions = await garden.getActionRouter()

const module = graph.getModule(moduleName)

const testConfig = findByName(module.testConfigs, testName)

if (!testConfig) {
throw new NotFoundError(`Could not find test "${testName}" in module "${moduleName}"`, {
moduleName,
testName,
availableTests: getNames(module.testConfigs),
})
}

const testVersion = await getTestVersion(garden, graph, module, testConfig)
const test = testFromModule(module, testName)

const testResult = await actions.getTestResult({
log,
testName,
test,
module,
testVersion,
})

let result: GetTestResultCommandResult = null

if (testResult) {
const artifacts = await getArtifactFileList({
key: getArtifactKey("test", testName, module.version.versionString),
key: getArtifactKey("test", testName, test.version),
artifactsPath: garden.artifactsPath,
log: garden.log,
})
Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { ConfigGraph } from "../config-graph"
import { Service } from "../types/service"
import { GardenService } from "../types/service"

export async function getHotReloadServiceNames(namesFromOpt: string[] | undefined, configGraph: ConfigGraph) {
const names = namesFromOpt || []
Expand Down Expand Up @@ -44,6 +44,6 @@ export async function validateHotReloadServiceNames(
return null
}

function supportsHotReloading(service: Service) {
function supportsHotReloading(service: GardenService) {
return service.config.hotReloadable
}
4 changes: 2 additions & 2 deletions core/src/commands/logs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import chalk from "chalk"
import { maxBy } from "lodash"
import { ServiceLogEntry } from "../types/plugin/service/getServiceLogs"
import Bluebird = require("bluebird")
import { Service } from "../types/service"
import { GardenService } from "../types/service"
import Stream from "ts-stream"
import { LoggerType } from "../logger/logger"
import dedent = require("dedent")
Expand Down Expand Up @@ -106,7 +106,7 @@ export class LogsCommand extends Command<Args, Opts> {
const actions = await garden.getActionRouter()
const voidLog = log.placeholder({ level: LogLevel.silly, childEntriesInheritLevel: true })

await Bluebird.map(services, async (service: Service<any>) => {
await Bluebird.map(services, async (service: GardenService<any>) => {
const status = await actions.getServiceStatus({
hotReload: false,
log: voidLog,
Expand Down
3 changes: 2 additions & 1 deletion core/src/commands/run/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,8 @@ export class RunModuleCommand extends Command<Args, Opts> {
garden,
graph,
dependencies,
version: module.version,
version: module.version.versionString,
moduleVersion: module.version.versionString,
serviceStatuses: {},
taskResults: {},
})
Expand Down
4 changes: 2 additions & 2 deletions core/src/commands/run/service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ export class RunServiceCommand extends Command<Args, Opts> {
const serviceName = args.service
const graph = await garden.getConfigGraph(log)
const service = graph.getService(serviceName, true)
const module = service.module

if (service.disabled && !opts.force) {
throw new CommandError(
Expand Down Expand Up @@ -107,7 +106,8 @@ export class RunServiceCommand extends Command<Args, Opts> {
garden,
graph,
dependencies,
version: module.version,
version: service.version,
moduleVersion: service.module.version.versionString,
serviceStatuses,
taskResults,
})
Expand Down
2 changes: 1 addition & 1 deletion core/src/commands/run/task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class RunTaskCommand extends Command<Args, Opts> {
)
}

const taskTask = await TaskTask.factory({ garden, graph, task, log, force: true, forceBuild: opts["force-build"] })
const taskTask = new TaskTask({ garden, graph, task, log, force: true, forceBuild: opts["force-build"] })
const graphResults = await garden.processTasks([taskTask])

return handleTaskResult({ log, actionDescription: "task", graphResults, key: taskTask.getKey() })
Expand Down
11 changes: 5 additions & 6 deletions core/src/commands/run/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,14 +130,13 @@ export class RunTestCommand extends Command<Args, Opts> {
const interactive = opts.interactive

// Make sure all dependencies are ready and collect their outputs for the runtime context
const testTask = await TestTask.factory({
const testTask = new TestTask({
force: true,
forceBuild: opts["force-build"],
garden,
graph,
log,
module,
testConfig,
test,
})

const dependencyResults = await garden.processTasks(await testTask.resolveDependencies())
Expand All @@ -151,7 +150,8 @@ export class RunTestCommand extends Command<Args, Opts> {
garden,
graph,
dependencies,
version: module.version,
version: test.version,
moduleVersion: module.version.versionString,
serviceStatuses,
taskResults,
})
Expand All @@ -168,8 +168,7 @@ export class RunTestCommand extends Command<Args, Opts> {
silent: false,
interactive,
runtimeContext,
testConfig,
testVersion: testTask.version,
test,
})

return handleRunResult({ log, actionDescription: "test", result, interactive, graphResults: dependencyResults })
Expand Down
21 changes: 15 additions & 6 deletions core/src/config-graph.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ import toposort from "toposort"
import { flatten, pick, uniq, sortBy, pickBy } from "lodash"
import { BuildDependencyConfig } from "./config/module"
import { GardenModule, getModuleKey, moduleNeedsBuild } from "./types/module"
import { Service, serviceFromConfig } from "./types/service"
import { Task, taskFromConfig } from "./types/task"
import { GardenService, serviceFromConfig } from "./types/service"
import { GardenTask, taskFromConfig } from "./types/task"
import { TestConfig } from "./config/test"
import { uniqByName, pickKeys } from "./util/util"
import { ConfigurationError } from "./exceptions"
Expand All @@ -22,15 +22,16 @@ import { TaskConfig } from "./config/task"
import { makeTestTaskName } from "./tasks/helpers"
import { TaskType, makeBaseKey } from "./tasks/base"
import { ModuleTypeMap } from "./types/plugin/plugin"
import { testFromModule, GardenTest } from "./types/test"

// Each of these types corresponds to a Task class (e.g. BuildTask, DeployTask, ...).
export type DependencyGraphNodeType = "build" | "deploy" | "run" | "test"

// The primary output type (for dependencies and dependants).
export type DependencyRelations = {
build: GardenModule[]
deploy: Service[]
run: Task[]
deploy: GardenService[]
run: GardenTask[]
test: TestConfig[]
}

Expand Down Expand Up @@ -308,17 +309,25 @@ export class ConfigGraph {
/**
* Returns the Service with the specified name. Throws error if it doesn't exist.
*/
getService(name: string, includeDisabled?: boolean): Service {
getService(name: string, includeDisabled?: boolean): GardenService {
return this.getServices({ names: [name], includeDisabled })[0]
}

/**
* Returns the Task with the specified name. Throws error if it doesn't exist.
*/
getTask(name: string, includeDisabled?: boolean): Task {
getTask(name: string, includeDisabled?: boolean): GardenTask {
return this.getTasks({ names: [name], includeDisabled })[0]
}

/**
* Returns the `testName` test from the `moduleName` module. Throws if either is not found.
*/
getTest(moduleName: string, testName: string, includeDisabled?: boolean): GardenTest {
const module = this.getModule(moduleName, includeDisabled)
return testFromModule(module, testName)
}

/*
Returns all modules defined in this configuration graph, or the ones specified.
*/
Expand Down

0 comments on commit d6f1373

Please sign in to comment.