chore: set default timeouts (#4066)

BREAKING CHANGE

Now all timeout configurations have default values.
This can affect existing pipelines with long-running tests and tasks. The old value was 60000 seconds. The new values for  `Build`, `Run`, and `Test` actions are 600 seconds.

* refactor: move constant `defaultBuildTimeout` to `constants.ts`
* refactor: rename timeout constants for clarity
* chore: set the default `Build` action timeout in joi schema
* chore: set the default `Run` action timeout in joi schema
* chore: set default `Test` action timeout in joi schema
* chore: change default run and test action timeouts to 600s
* chore: remove the duplicate constant for `Run` action timeout. Use `DEFAULT_RUN_TIMEOUT_SEC` everywhere.
* chore: default timeout value for module-based tasks and tests
* chore: set the minimal timeout value for module builds
* refactor: non-optional timeout in `BaseTestSpec`
* refactor: non-optional timeout in `BaseTaskSpec`
* refactor: non-optional timeout in `BaseBuildSpec`
* chore: replace `DEFAULT_BUILD_TIMEOUT` by `DEFAULT_BUILD_TIMEOUT_SEC`. Constant `DEFAULT_BUILD_TIMEOUT` was used only in tests, and was a duplicate of `DEFAULT_BUILD_TIMEOUT_SEC`.
* chore: require integer as a k8s deployment timeout
* chore: non-optional timeout in `BaseRunParams`
* chore: set the default timeout for exec service
* chore: set the default timeout for exec `Deploy` action
* refactor: make `timeout` non-optional in `KubernetesTypeCommonDeploySpec`
* refactor: add optional `timeout` field in `BaseActionConfig`
* chore: make `timeout` required in `runBaseParams`. Non-optional value is always set from outside.
* test: updated tests and fixed assertions

core/src/actions/build.ts

@@ -38,6 +38,7 @@ import {
 import { ResolvedConfigGraph } from "../graph/config-graph"
 import { ActionVersion } from "../vcs/vcs"
 import { Memoize } from "typescript-memoize"
+import { DEFAULT_BUILD_TIMEOUT_SEC } from "../constants"
 
 export interface BuildCopyFrom {
   build: string
@@ -51,7 +52,6 @@ export interface BuildActionConfig<T extends string = string, S extends object =
   allowPublish?: boolean
   buildAtSource?: boolean
   copyFrom?: BuildCopyFrom[]
-  timeout?: number
 }
 
 export const copyFromSchema = createSchema({
@@ -136,6 +136,8 @@ export const buildActionConfigSchema = () =>
     timeout: joi
       .number()
       .integer()
+      .min(1)
+      .default(DEFAULT_BUILD_TIMEOUT_SEC)
       .description("Set a timeout for the build to complete, in seconds.")
       .meta({ templateContext: ActionConfigContext }),
   })
@@ -203,6 +205,7 @@ export class ResolvedBuildAction<
     this._config.spec = params.spec
     this._config.internal.inputs = params.inputs
   }
+
   getExecutedDependencies() {
     return this.executedDependencies
   }

core/src/actions/run.ts

@@ -16,16 +16,20 @@ import {
   RuntimeAction,
 } from "./base"
 import { Action, BaseActionConfig } from "./types"
+import { DEFAULT_RUN_TIMEOUT_SEC } from "../constants"
 
 export interface RunActionConfig<N extends string = any, S extends object = any>
-  extends BaseRuntimeActionConfig<"Run", N, S> {
-  timeout?: number
-}
+  extends BaseRuntimeActionConfig<"Run", N, S> {}
 
 export const runActionConfigSchema = memoize(() =>
   baseRuntimeActionConfigSchema().keys({
     kind: joi.string().allow("Run").only(),
-    timeout: joi.number().integer().description("Set a timeout for the run to complete, in seconds."),
+    timeout: joi
+      .number()
+      .integer()
+      .min(1)
+      .default(DEFAULT_RUN_TIMEOUT_SEC)
+      .description("Set a timeout for the run to complete, in seconds."),
   })
 )
 

core/src/actions/test.ts

@@ -16,16 +16,20 @@ import {
   RuntimeAction,
 } from "./base"
 import { Action, BaseActionConfig } from "./types"
+import { DEFAULT_TEST_TIMEOUT_SEC } from "../constants"
 
 export interface TestActionConfig<N extends string = any, S extends object = any>
-  extends BaseRuntimeActionConfig<"Test", N, S> {
-  timeout?: number
-}
+  extends BaseRuntimeActionConfig<"Test", N, S> {}
 
 export const testActionConfigSchema = memoize(() =>
   baseRuntimeActionConfigSchema().keys({
     kind: joi.string().allow("Test").only(),
-    timeout: joi.number().integer().description("Set a timeout for the test to complete, in seconds."),
+    timeout: joi
+      .number()
+      .integer()
+      .min(1)
+      .default(DEFAULT_TEST_TIMEOUT_SEC)
+      .description("Set a timeout for the test to complete, in seconds."),
   })
 )
 

core/src/actions/types.ts

@@ -79,6 +79,8 @@ export interface BaseActionConfig<K extends ActionKind = ActionKind, T = string,
   include?: string[]
   exclude?: string[]
 
+  timeout?: number
+
   // Variables
   // -> Templating with ActionConfigContext allowed
   variables?: DeepPrimitiveMap

core/src/config/base.ts

@@ -14,7 +14,7 @@ import { pathExists, readFile } from "fs-extra"
 import { omit, isPlainObject, isArray } from "lodash"
 import { coreModuleSpecSchema, baseModuleSchemaKeys, BuildDependencyConfig, ModuleConfig } from "./module"
 import { ConfigurationError, FilesystemError, ParameterError } from "../exceptions"
-import { DEFAULT_API_VERSION, PREVIOUS_API_VERSION } from "../constants"
+import { DEFAULT_API_VERSION, DEFAULT_BUILD_TIMEOUT_SEC, PREVIOUS_API_VERSION } from "../constants"
 import { ProjectConfig, ProjectResource } from "../config/project"
 import { validateWithPath } from "./validation"
 import { defaultDotIgnoreFile, listDirectory } from "../util/fs"
@@ -379,6 +379,7 @@ export function prepareModuleResource(spec: any, configPath: string, projectRoot
     allowPublish: spec.allowPublish,
     build: {
       dependencies,
+      timeout: spec.build?.timeout || DEFAULT_BUILD_TIMEOUT_SEC,
     },
     configPath,
     description: spec.description,

core/src/config/module.ts

@@ -9,24 +9,23 @@
 import { memoize } from "lodash"
 import { ServiceConfig, serviceConfigSchema } from "./service"
 import {
+  apiVersionSchema,
+  createSchema,
+  DeepPrimitiveMap,
+  includeGuideLink,
+  joi,
   joiArray,
   joiIdentifier,
   joiRepositoryUrl,
+  joiSparseArray,
   joiUserIdentifier,
-  joi,
-  includeGuideLink,
-  apiVersionSchema,
-  DeepPrimitiveMap,
   joiVariables,
-  joiSparseArray,
-  createSchema,
 } from "./common"
 import { TestConfig, testConfigSchema } from "./test"
 import { TaskConfig, taskConfigSchema } from "./task"
 import { dedent, stableStringify } from "../util/string"
 import { configTemplateKind, varfileDescription } from "./base"
-
-export const defaultBuildTimeout = 1200
+import { DEFAULT_BUILD_TIMEOUT_SEC } from "../constants"
 
 interface BuildCopySpec {
   source: string
@@ -71,7 +70,7 @@ export const buildDependencySchema = createSchema({
 
 export interface BaseBuildSpec {
   dependencies: BuildDependencyConfig[]
-  timeout?: number
+  timeout: number
 }
 
 export interface GenerateFileSpec {
@@ -157,7 +156,8 @@ export const baseBuildSpecSchema = createSchema({
     timeout: joi
       .number()
       .integer()
-      .default(defaultBuildTimeout)
+      .min(1)
+      .default(DEFAULT_BUILD_TIMEOUT_SEC)
       .description("Maximum time in seconds to wait for build to finish."),
   }),
   default: () => ({ dependencies: [] }),

core/src/config/task.ts

@@ -9,6 +9,7 @@
 import { joiUserIdentifier, joi, joiSparseArray, createSchema } from "./common"
 import { deline, dedent } from "../util/string"
 import { memoize } from "lodash"
+import { DEFAULT_RUN_TIMEOUT_SEC } from "../constants"
 
 export interface TaskSpec {}
 
@@ -17,7 +18,7 @@ export interface BaseTaskSpec extends TaskSpec {
   dependencies: string[]
   description?: string
   disabled: boolean
-  timeout: number | null
+  timeout: number
 }
 
 export const cacheResultSchema = memoize(() =>
@@ -54,9 +55,9 @@ export const baseTaskSpecSchema = createSchema({
       ),
     timeout: joi
       .number()
-      .optional()
-      .allow(null)
-      .default(null)
+      .integer()
+      .min(1)
+      .default(DEFAULT_RUN_TIMEOUT_SEC)
       .description("Maximum duration (in seconds) of the task's execution."),
   }),
 })

core/src/config/test.ts

@@ -8,12 +8,13 @@
 
 import { joiUserIdentifier, joi, joiSparseArray, createSchema } from "./common"
 import { deline, dedent } from "../util/string"
+import { DEFAULT_TEST_TIMEOUT_SEC } from "../constants"
 
 export interface BaseTestSpec {
   name: string
   dependencies: string[]
   disabled: boolean
-  timeout: number | null
+  timeout: number
 }
 
 export const baseTestSpecSchema = createSchema({
@@ -35,7 +36,12 @@ export const baseTestSpecSchema = createSchema({
         specific environments, e.g. only during CI.
       `
       ),
-    timeout: joi.number().allow(null).default(null).description("Maximum duration (in seconds) of the test run."),
+    timeout: joi
+      .number()
+      .integer()
+      .min(1)
+      .default(DEFAULT_TEST_TIMEOUT_SEC)
+      .description("Maximum duration (in seconds) of the test run."),
   }),
 })
 

core/src/constants.ts

@@ -7,7 +7,7 @@
  */
 
 import env from "env-var"
-import { resolve, join } from "path"
+import { join, resolve } from "path"
 import { homedir } from "os"
 
 export const isPkg = !!(<any>process).pkg
@@ -26,8 +26,9 @@ export const DEFAULT_PORT_PROTOCOL = "TCP"
 export const PREVIOUS_API_VERSION = "garden.io/v0"
 export const DEFAULT_API_VERSION = "garden.io/v1"
 
-export const DEFAULT_TEST_TIMEOUT = 60 * 1000
-export const DEFAULT_RUN_TIMEOUT = 60 * 1000
+export const DEFAULT_BUILD_TIMEOUT_SEC = 600
+export const DEFAULT_TEST_TIMEOUT_SEC = 600
+export const DEFAULT_RUN_TIMEOUT_SEC = 600
 
 export type SupportedPlatform = "linux" | "darwin" | "win32"
 export const SUPPORTED_PLATFORMS: SupportedPlatform[] = ["linux", "darwin", "win32"]

core/src/logger/log-entry.ts

@@ -455,7 +455,7 @@ export class ActionLog extends Log<ActionLogContext> {
   showDuration = true
 
   /**
-   * Create a new ActioLog instance. The new instance inherits the parent context.
+   * Create a new ActionLog instance. The new instance inherits the parent context.
    */
   createLog(params: CreateLogParams = {}): ActionLog {
     return new ActionLog(this.makeLogConfig(params))

core/src/plugin/base.ts

@@ -114,7 +114,7 @@ export const moduleActionParamsSchema = () =>
 export const runBaseParams = () => ({
   interactive: joi.boolean().description("Whether to run interactively (i.e. attach to the terminal)."),
   silent: joi.boolean().description("Set to false if the output should not be logged to the console."),
-  timeout: joi.number().optional().description("If set, how long to run the command before timing out."),
+  timeout: joi.number().required().description("If set, how long to run the command before timing out."),
 })
 
 // TODO-0.13.0: update this schema in 0.13.0

core/src/plugin/handlers/base/base.ts

@@ -55,5 +55,5 @@ export interface BaseRunParams {
   command?: string[]
   args: string[]
   interactive: boolean
-  timeout?: number
+  timeout: number
 }

core/src/plugins/container/container.ts

@@ -252,7 +252,7 @@ function convertContainerModuleRuntimeActions(
       disabled: task.disabled,
       build: buildAction?.name,
       dependencies: prepareRuntimeDependencies(task.spec.dependencies, buildAction),
-      timeout: task.spec.timeout || undefined,
+      timeout: task.spec.timeout,
 
       spec: {
         ...omit(task.spec, ["name", "dependencies", "disabled", "timeout"]),
@@ -271,7 +271,7 @@ function convertContainerModuleRuntimeActions(
       disabled: test.disabled,
       build: buildAction?.name,
       dependencies: prepareRuntimeDependencies(test.spec.dependencies, buildAction),
-      timeout: test.spec.timeout ? test.spec.timeout : undefined,
+      timeout: test.spec.timeout,
 
       spec: {
         ...omit(test.spec, ["name", "dependencies", "disabled", "timeout"]),

core/src/plugins/container/helpers.ts

@@ -40,8 +40,6 @@ interface DockerVersion {
   server?: string
 }
 
-export const DEFAULT_BUILD_TIMEOUT = 600
-
 export const minDockerVersion: DockerVersion = {
   client: "19.03.0",
   server: "17.07.0",

core/src/plugins/exec/config.ts

@@ -22,6 +22,7 @@ import {
 } from "../../config/common"
 import { ArtifactSpec } from "../../config/validation"
 import { dedent } from "../../util/string"
+import { DEFAULT_RUN_TIMEOUT_SEC } from "../../constants"
 
 const execPathDoc = dedent`
   Note that if a Build is referenced in the \`build\` field, the command will be run from the build directory for that Build action. If that Build has \`buildAtSource: true\` set, the command will be run from the source directory of the Build action. If no \`build\` reference is set, the command is run from the source directory of this action.
@@ -53,13 +54,15 @@ const execCommonSchema = createSchema({
   `),
   }),
 })
+
 // BUILD //
 
 export interface ExecBuildActionSpec extends CommonKeys {
   command?: string[] // This needs to be optional to support "dummy" builds
   timeout?: number
   env: StringMap
 }
+
 export type ExecBuildConfig = BuildActionConfig<"exec", ExecBuildActionSpec>
 export type ExecBuild = BuildAction<ExecBuildConfig, ExecOutputs>
 
@@ -97,7 +100,7 @@ export interface ExecDeployActionSpec extends CommonKeys {
   cleanupCommand?: string[]
   deployCommand: string[]
   statusCommand?: string[]
-  timeout?: number
+  timeout: number
   statusTimeout: number
   env: StringMap
 }
@@ -159,8 +162,7 @@ export const execDeployActionSchema = createSchema({
         ${execPathDoc}
         `
       ),
-    // TODO: Set a default in v0.13.
-    timeout: joi.number().description(dedent`
+    timeout: joi.number().integer().min(1).default(DEFAULT_RUN_TIMEOUT_SEC).description(dedent`
       The maximum duration (in seconds) to wait for \`deployCommand\` to exit. Ignored if \`persistent: false\`.
     `),
     statusTimeout: joi.number().default(defaultStatusTimeout).description(dedent`
@@ -204,6 +206,7 @@ export const execRunActionSchema = createSchema({
 // TEST //
 
 export interface ExecTestActionSpec extends ExecRunActionSpec {}
+
 export type ExecTestConfig = TestActionConfig<"exec", ExecTestActionSpec>
 export type ExecTest = TestAction<ExecTestConfig, ExecOutputs>
 

core/src/plugins/exec/convert.ts

@@ -126,7 +126,7 @@ export async function convertExecModule(params: ConvertModuleParams<ExecModule>)
       disabled: task.disabled,
       build: buildAction ? buildAction.name : undefined,
       dependencies: prepRuntimeDeps(task.spec.dependencies),
-      timeout: task.spec.timeout ? task.spec.timeout : undefined,
+      timeout: task.spec.timeout,
 
       spec: {
         shell: true, // This keeps the old pre-0.13 behavior
@@ -147,7 +147,7 @@ export async function convertExecModule(params: ConvertModuleParams<ExecModule>)
       disabled: test.disabled,
       build: buildAction ? buildAction.name : undefined,
       dependencies: prepRuntimeDeps(test.spec.dependencies),
-      timeout: test.spec.timeout ? test.spec.timeout : undefined,
+      timeout: test.spec.timeout,
 
       spec: {
         shell: true, // This keeps the old pre-0.13 behavior

core/src/plugins/exec/moduleConfig.ts

@@ -23,6 +23,7 @@ import { artifactsSchema, ExecSyncModeSpec } from "./config"
 import { ConfigureModuleParams, ConfigureModuleResult } from "../../plugin/handlers/Module/configure"
 import { ConfigurationError } from "../../exceptions"
 import { omit } from "lodash"
+import { DEFAULT_RUN_TIMEOUT_SEC } from "../../constants"
 
 const execPathDoc = dedent`
   By default, the command is run inside the Garden build directory (under .garden/build/<module-name>).
@@ -87,7 +88,7 @@ export interface ExecServiceSpec extends CommonServiceSpec {
   deployCommand: string[]
   statusCommand?: string[]
   syncMode?: ExecSyncModeSpec
-  timeout?: number
+  timeout: number
   env: { [key: string]: string }
 }
 
@@ -130,8 +131,7 @@ export const execServiceSchema = () =>
           ${execPathDoc}
           `
         ),
-      // TODO: Set a default in v0.13.
-      timeout: joi.number().description(dedent`
+      timeout: joi.number().integer().min(1).default(DEFAULT_RUN_TIMEOUT_SEC).description(dedent`
         The maximum duration (in seconds) to wait for a local script to exit.
       `),
       env: joiEnvVars().description("Environment variables to set when running the deploy and status commands."),