Skip to content

Commit

Permalink
fix(k8s): error when test+task result log exceeded 1MB
Browse files Browse the repository at this point in the history
This comes down to a limitation of K8s ConfigMap resources. I've
refactored that code a bit and ensured we limit the log size we store.

I also made sure the ConfigMap keys didn't grow too long by hashing
the key parameters.

Lastly I made the results more easily searchable by adding labels.
  • Loading branch information
edvald committed Jun 25, 2019
1 parent 24168fb commit 04a5a36
Show file tree
Hide file tree
Showing 14 changed files with 166 additions and 84 deletions.
12 changes: 7 additions & 5 deletions garden-service/src/plugins/kubernetes/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -413,13 +413,15 @@ async function getContextConfig(log: LogEntry, context: string): Promise<KubeCon
return kc
}

function wrapError(err) {
function wrapError(err: any) {
if (!err.message) {
const wrapped = new KubernetesError(`Got error from Kubernetes API - ${err.body.message}`, {
body: err.body,
request: omitBy(err.response.request, (v, k) => isObject(v) || k[0] === "_"),
const response = err.response || {}
const body = response.body || err.body
const wrapped = new KubernetesError(`Got error from Kubernetes API - ${body.message}`, {
body,
request: omitBy(response.request, (v, k) => isObject(v) || k[0] === "_"),
})
wrapped.code = err.response.statusCode
wrapped.code = response.statusCode
return wrapped
} else {
return err
Expand Down
2 changes: 2 additions & 0 deletions garden-service/src/plugins/kubernetes/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,5 @@
export const RSYNC_PORT = 873
export const CLUSTER_REGISTRY_PORT = 5000
export const CLUSTER_REGISTRY_DEPLOYMENT_NAME = "garden-docker-registry"
export const MAX_CONFIGMAP_DATA_SIZE = 1024 * 1024 // max ConfigMap data size is 1MB
export const MAX_RUN_RESULT_OUTPUT_LENGTH = 900 * 1024 // max ConfigMap data size is 1MB, so 900kB gives enough margin
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import { configureContainerModule } from "../../container/container"
import { KubernetesProvider } from "../config"
import { ConfigureModuleParams } from "../../../types/plugin/module/configure"
import { getContainerServiceStatus } from "./status"
import { getTestResult } from "../test"
import { getTestResult } from "../test-results"
import { ContainerModule } from "../../container/config"
import { configureMavenContainerModule, MavenContainerModule } from "../../maven-container/maven-container"
import { getTaskResult } from "../task-results"
Expand Down
1 change: 1 addition & 0 deletions garden-service/src/plugins/kubernetes/container/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,7 @@ export async function runContainerTask(
await storeTaskResult({
ctx,
log,
module,
result,
taskVersion,
taskName: task.name,
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/plugins/kubernetes/container/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
import { ContainerModule } from "../../container/config"
import { DEFAULT_TEST_TIMEOUT } from "../../../constants"
import { runContainerModule } from "./run"
import { storeTestResult } from "../test"
import { storeTestResult } from "../test-results"
import { TestModuleParams } from "../../../types/plugin/module/testModule"
import { TestResult } from "../../../types/plugin/module/getTestResult"

Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/plugins/kubernetes/helm/handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { HelmModule, validateHelmModule as configureHelmModule, helmModuleSpecSc
import { buildHelmModule } from "./build"
import { getServiceStatus } from "./status"
import { deployService, deleteService } from "./deployment"
import { getTestResult } from "../test"
import { getTestResult } from "../test-results"
import { runHelmTask, runHelmModule } from "./run"
import { hotReloadHelmChart } from "./hot-reload"
import { getServiceLogs } from "./logs"
Expand Down
10 changes: 9 additions & 1 deletion garden-service/src/plugins/kubernetes/helm/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import { storeTaskResult } from "../task-results"
import { RunModuleParams } from "../../../types/plugin/module/runModule"
import { RunResult } from "../../../types/plugin/base"
import { RunTaskParams, RunTaskResult } from "../../../types/plugin/task/runTask"
import { MAX_RUN_RESULT_OUTPUT_LENGTH } from "../constants"
import { tailString } from "../../../util/string"

export async function runHelmModule(
{
Expand Down Expand Up @@ -78,11 +80,17 @@ export async function runHelmTask(
log,
})

const result = { ...res, taskName: task.name }
const result = {
...res,
// Make sure we don't exceed max length of ConfigMap
output: tailString(res.output, MAX_RUN_RESULT_OUTPUT_LENGTH, true),
taskName: task.name,
}

await storeTaskResult({
ctx,
log,
module,
result,
taskVersion,
taskName: task.name,
Expand Down
2 changes: 1 addition & 1 deletion garden-service/src/plugins/kubernetes/helm/test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { DEFAULT_TEST_TIMEOUT } from "../../../constants"
import { storeTestResult } from "../test"
import { storeTestResult } from "../test-results"
import { HelmModule } from "./config"
import { getAppNamespace } from "../namespace"
import { runPod } from "../run"
Expand Down
61 changes: 30 additions & 31 deletions garden-service/src/plugins/kubernetes/task-results.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,22 @@ import { KubernetesPluginContext, KubernetesProvider } from "./config"
import { KubeApi } from "./api"
import { getMetadataNamespace } from "./namespace"
import { RunTaskResult } from "../../types/plugin/task/runTask"
import { deserializeValues, serializeValues } from "../../util/util"
import { deserializeValues } from "../../util/util"
import { PluginContext } from "../../plugin-context"
import { LogEntry } from "../../logger/log-entry"
import { gardenAnnotationKey, tailString } from "../../util/string"
import { Module } from "../../types/module"
import * as hasha from "hasha"
import { upsertConfigMap } from "./util"
import { MAX_RUN_RESULT_OUTPUT_LENGTH } from "./constants"

export async function getTaskResult(
{ ctx, log, task, taskVersion }: GetTaskResultParams<ContainerModule | HelmModule>,
{ ctx, log, module, task, taskVersion }: GetTaskResultParams<ContainerModule | HelmModule>,
): Promise<RunTaskResult | null> {
const k8sCtx = <KubernetesPluginContext>ctx
const api = await KubeApi.factory(log, k8sCtx.provider.config.context)
const ns = await getMetadataNamespace(k8sCtx, log, k8sCtx.provider)
const resultKey = getTaskResultKey(task.name, taskVersion)
const resultKey = getTaskResultKey(ctx, module, task.name, taskVersion)

try {
const res = await api.core.readNamespacedConfigMap(resultKey, ns)
Expand All @@ -38,13 +43,16 @@ export async function getTaskResult(
}
}

export function getTaskResultKey(taskName: string, version: ModuleVersion) {
return `task-result--${taskName}--${version.versionString}`
export function getTaskResultKey(ctx: PluginContext, module: Module, taskName: string, version: ModuleVersion) {
const key = `${ctx.projectName}--${module.name}--${taskName}--${version.versionString}`
const hash = hasha(key, { algorithm: "sha1" })
return `task-result--${hash.slice(0, 32)}`
}

interface StoreTaskResultParams {
ctx: PluginContext,
log: LogEntry,
module: Module,
taskName: string,
taskVersion: ModuleVersion,
result: RunTaskResult,
Expand All @@ -56,34 +64,25 @@ interface StoreTaskResultParams {
* TODO: Implement a CRD for this.
*/
export async function storeTaskResult(
{ ctx, log, taskName, taskVersion, result }: StoreTaskResultParams,
): Promise<RunTaskResult> {
{ ctx, log, module, taskName, taskVersion, result }: StoreTaskResultParams,
) {
const provider = <KubernetesProvider>ctx.provider
const api = await KubeApi.factory(log, provider.config.context)
const ns = await getMetadataNamespace(ctx, log, provider)
const resultKey = getTaskResultKey(taskName, taskVersion)
const namespace = await getMetadataNamespace(ctx, log, provider)

const body = {
apiVersion: "v1",
kind: "ConfigMap",
metadata: {
name: resultKey,
annotations: {
"garden.io/generated": "true",
},
},
data: serializeValues(result),
}

try {
await api.core.createNamespacedConfigMap(ns, <any>body)
} catch (err) {
if (err.code === 409) {
await api.core.patchNamespacedConfigMap(resultKey, ns, body)
} else {
throw err
}
}
// Make sure the output isn't too large for a ConfigMap
result.output = tailString(result.output, MAX_RUN_RESULT_OUTPUT_LENGTH, true)

return result
await upsertConfigMap({
api,
namespace,
key: getTaskResultKey(ctx, module, taskName, taskVersion),
labels: {
[gardenAnnotationKey("module")]: module.name,
[gardenAnnotationKey("task")]: taskName,
[gardenAnnotationKey("moduleVersion")]: module.version.versionString,
[gardenAnnotationKey("version")]: taskVersion.versionString,
},
data: result,
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
*/

import { ContainerModule } from "../container/config"
import { deserializeValues, serializeValues } from "../../util/util"
import { deserializeValues } from "../../util/util"
import { KubeApi } from "./api"
import { Module } from "../../types/module"
import { ModuleVersion } from "../../vcs/vcs"
Expand All @@ -18,6 +18,9 @@ import { systemMetadataNamespace } from "./system"
import { LogEntry } from "../../logger/log-entry"
import { GetTestResultParams, TestResult } from "../../types/plugin/module/getTestResult"
import { RunResult } from "../../types/plugin/base"
import * as hasha from "hasha"
import { gardenAnnotationKey } from "../../util/string"
import { upsertConfigMap } from "./util"

const testResultNamespace = systemMetadataNamespace

Expand All @@ -41,7 +44,9 @@ export async function getTestResult(
}

export function getTestResultKey(ctx: PluginContext, module: Module, testName: string, version: ModuleVersion) {
return `test-result--${ctx.projectName}--${module.name}--${testName}--${version.versionString}`
const key = `${ctx.projectName}--${module.name}--${testName}--${version.versionString}`
const hash = hasha(key, { algorithm: "sha1" })
return `test-result--${hash.slice(0, 32)}`
}

interface StoreTestResultParams {
Expand Down Expand Up @@ -69,28 +74,18 @@ export async function storeTestResult(
testName,
}

const resultKey = getTestResultKey(k8sCtx, module, testName, testVersion)
const body = {
apiVersion: "v1",
kind: "ConfigMap",
metadata: {
name: resultKey,
annotations: {
"garden.io/generated": "true",
},
await upsertConfigMap({
api,
namespace: testResultNamespace,
key: getTestResultKey(k8sCtx, module, testName, testVersion),
labels: {
[gardenAnnotationKey("module")]: module.name,
[gardenAnnotationKey("test")]: testName,
[gardenAnnotationKey("moduleVersion")]: module.version.versionString,
[gardenAnnotationKey("version")]: testVersion.versionString,
},
data: serializeValues(testResult),
}

try {
await api.core.createNamespacedConfigMap(testResultNamespace, <any>body)
} catch (err) {
if (err.code === 409) {
await api.core.patchNamespacedConfigMap(resultKey, testResultNamespace, body)
} else {
throw err
}
}
data: testResult,
})

return testResult
}
47 changes: 45 additions & 2 deletions garden-service/src/plugins/kubernetes/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,15 @@ const AsyncLock = require("async-lock")
import { V1Pod } from "@kubernetes/client-node"

import { KubernetesResource, KubernetesWorkload, KubernetesPod, KubernetesServerResource } from "./types"
import { splitLast } from "../../util/util"
import { KubeApi } from "./api"
import { splitLast, serializeValues } from "../../util/util"
import { KubeApi, KubernetesError } from "./api"
import { PluginContext } from "../../plugin-context"
import { LogEntry } from "../../logger/log-entry"
import { KubernetesPluginContext } from "./config"
import { kubectl } from "./kubectl"
import { registerCleanupFunction } from "../../util/util"
import { gardenAnnotationKey, base64 } from "../../util/string"
import { MAX_CONFIGMAP_DATA_SIZE } from "./constants"

export const workloadTypes = ["Deployment", "DaemonSet", "ReplicaSet", "StatefulSet"]

Expand Down Expand Up @@ -216,3 +218,44 @@ const suffixTable = {
Gi: 2,
Mi: 1,
}

export async function upsertConfigMap(
{ api, namespace, key, labels, data }:
{ api: KubeApi, namespace: string, key: string, labels: { [key: string]: string }, data: { [key: string]: any } },
) {
const serializedData = serializeValues(data)

if (base64(JSON.stringify(serializedData)).length > MAX_CONFIGMAP_DATA_SIZE) {
throw new KubernetesError(`Attempting to store too much data in ConfigMap ${key}`, {
key,
namespace,
labels,
data,
})
}

const body = {
apiVersion: "v1",
kind: "ConfigMap",
metadata: {
name: key,
annotations: {
[gardenAnnotationKey("generated")]: "true",
// Set all the labels as annotations as well
...labels,
},
labels,
},
data: serializedData,
}

try {
await api.core.createNamespacedConfigMap(namespace, <any>body)
} catch (err) {
if (err.code === 409) {
await api.core.patchNamespacedConfigMap(key, namespace, body)
} else {
throw err
}
}
}
28 changes: 27 additions & 1 deletion garden-service/src/util/string.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,34 @@ export const deline = _deline

const gardenAnnotationPrefix = "garden.io/"

export type GardenAnnotationKey = "generated" | "service" | "version"
export type GardenAnnotationKey = "generated" | "module" | "moduleVersion" | "service" | "task" | "test" | "version"

export function gardenAnnotationKey(key: GardenAnnotationKey) {
return gardenAnnotationPrefix + key
}

/**
* Truncates the first n characters from a string where n equals the number by
* which the string byte length exceeds the `maxLength`.
*
* Optionally scan towards the next line break after trimming the bytes, and trim to there.
*
* Note that a UTF-8 character can be 1-4 bytes so this is a naive but inexpensive approach.
*/
export function tailString(str: string, maxLength: number, nextLine = false) {
const overflow = Buffer.byteLength(str, "utf8") - maxLength
if (overflow > 0) {
if (nextLine) {
const lineBreakIdx = str.indexOf("\n", overflow)
if (lineBreakIdx) {
return str.substr(lineBreakIdx + 1)
}
}
return str.substr(overflow)
}
return str
}

export function base64(str: string) {
return Buffer.from(str).toString("base64")
}
Loading

0 comments on commit 04a5a36

Please sign in to comment.