From 8b9bbfeecff3ac9e11f58b566aa6ec8bb83182b8 Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Thu, 31 Oct 2019 21:07:02 +0100 Subject: [PATCH] fix(k8s): hostPath is now relative to module source path Previously we implicitly expected an absolute path. Now we explicitly make it relative to the module path, but also support absolute paths. Also added some docs, warning against the usage of hostPath. --- docs/reference/module-types/container.md | 14 ++++++++++++++ docs/reference/module-types/maven-container.md | 14 ++++++++++++++ garden-service/src/plugins/container/config.ts | 12 +++++++++++- .../src/plugins/kubernetes/container/deployment.ts | 9 +++++---- .../src/plugins/local/local-docker-swarm.ts | 3 ++- 5 files changed, 46 insertions(+), 6 deletions(-) diff --git a/docs/reference/module-types/container.md b/docs/reference/module-types/container.md index 5d9d3ab2e2..4383e2b9d6 100644 --- a/docs/reference/module-types/container.md +++ b/docs/reference/module-types/container.md @@ -813,10 +813,24 @@ The path where the volume should be mounted in the container. [services](#services) > [volumes](#servicesvolumes) > hostPath +_NOTE: Usage of hostPath is generally discouraged, since it doesn't work reliably across different platforms +and providers. Some providers may not support it at all._ + +A local path or path on the node that's running the container, to mount in the container, relative to the +module source path (or absolute). + | Type | Required | | -------- | -------- | | `string` | No | +Example: + +```yaml +services: + - volumes: + - hostPath: "/some/dir" +``` + ### `tests` A list of tests to run in the module. diff --git a/docs/reference/module-types/maven-container.md b/docs/reference/module-types/maven-container.md index b02f23f4ea..0e44a07012 100644 --- a/docs/reference/module-types/maven-container.md +++ b/docs/reference/module-types/maven-container.md @@ -818,10 +818,24 @@ The path where the volume should be mounted in the container. [services](#services) > [volumes](#servicesvolumes) > hostPath +_NOTE: Usage of hostPath is generally discouraged, since it doesn't work reliably across different platforms +and providers. Some providers may not support it at all._ + +A local path or path on the node that's running the container, to mount in the container, relative to the +module source path (or absolute). + | Type | Required | | -------- | -------- | | `string` | No | +Example: + +```yaml +services: + - volumes: + - hostPath: "/some/dir" +``` + ### `tests` A list of tests to run in the module. diff --git a/garden-service/src/plugins/container/config.ts b/garden-service/src/plugins/container/config.ts index f8f3df2d78..d752bffd0c 100644 --- a/garden-service/src/plugins/container/config.ts +++ b/garden-service/src/plugins/container/config.ts @@ -25,6 +25,7 @@ import { CommonServiceSpec, ServiceConfig, baseServiceSpecSchema } from "../../c import { baseTaskSpecSchema, BaseTaskSpec } from "../../config/task" import { baseTestSpecSchema, BaseTestSpec } from "../../config/test" import { joiStringMap } from "../../config/common" +import { dedent } from "../../util/string" export const defaultContainerLimits: ServiceLimitSpec = { cpu: 1000, // = 1000 millicpu = 1 CPU @@ -285,10 +286,19 @@ const volumeSchema = joi.object() .required() .description("The name of the allocated volume."), containerPath: joi.string() + .posixPath() .required() .description("The path where the volume should be mounted in the container."), hostPath: joi.string() - .meta({ deprecated: true }), + .posixPath() + .description(dedent` + _NOTE: Usage of hostPath is generally discouraged, since it doesn't work reliably across different platforms + and providers. Some providers may not support it at all._ + + A local path or path on the node that's running the container, to mount in the container, relative to the + module source path (or absolute). + `) + .example("/some/dir"), }) const serviceSchema = baseServiceSpecSchema diff --git a/garden-service/src/plugins/kubernetes/container/deployment.ts b/garden-service/src/plugins/kubernetes/container/deployment.ts index 8874882fc1..15a2c86400 100644 --- a/garden-service/src/plugins/kubernetes/container/deployment.ts +++ b/garden-service/src/plugins/kubernetes/container/deployment.ts @@ -30,6 +30,7 @@ import { DeleteServiceParams } from "../../../types/plugin/service/deleteService import { millicpuToString, kilobytesToString, prepareEnvVars, workloadTypes } from "../util" import { gardenAnnotationKey } from "../../../util/string" import { RuntimeContext } from "../../../runtime-context" +import { resolve } from "path" export const DEFAULT_CPU_REQUEST = "10m" export const DEFAULT_MEMORY_REQUEST = "64Mi" @@ -288,7 +289,7 @@ export async function createWorkloadResource( } if (spec.volumes && spec.volumes.length) { - configureVolumes(deployment, container, spec) + configureVolumes(service.module, deployment, container, spec) } const ports = spec.ports @@ -467,7 +468,7 @@ function configureHealthCheck(container, spec): void { } -function configureVolumes(deployment, container, spec): void { +function configureVolumes(module: ContainerModule, deployment, container, spec): void { const volumes: any[] = [] const volumeMounts: any[] = [] @@ -492,12 +493,12 @@ function configureVolumes(deployment, container, spec): void { volumes.push({ name: volumeName, hostPath: { - path: volume.hostPath, + path: resolve(module.path, volume.hostPath), }, }) volumeMounts.push({ name: volumeName, - mountPath: volume.containerPath || volume.hostPath, + mountPath: volume.containerPath, }) } else { throw new Error("Unsupported volume type: " + volumeType) diff --git a/garden-service/src/plugins/local/local-docker-swarm.ts b/garden-service/src/plugins/local/local-docker-swarm.ts index b71696cd8f..43642be139 100644 --- a/garden-service/src/plugins/local/local-docker-swarm.ts +++ b/garden-service/src/plugins/local/local-docker-swarm.ts @@ -20,6 +20,7 @@ import { DeployServiceParams } from "../../types/plugin/service/deployService" import { ExecInServiceParams } from "../../types/plugin/service/execInService" import { GetServiceStatusParams } from "../../types/plugin/service/getServiceStatus" import { EnvironmentStatus } from "../../types/plugin/provider/getEnvironmentStatus" +import { resolve } from "path" // should this be configurable and/or global across providers? const DEPLOY_TIMEOUT = 30 @@ -64,7 +65,7 @@ export const gardenPlugin = createGardenPlugin({ if (v.hostPath) { return { Type: "bind", - Source: v.hostPath, + Source: resolve(module.path, v.hostPath), Target: v.containerPath, } } else {