Skip to content

Commit

Permalink
fix(k8s): hostPath is now relative to module source path
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
edvald committed Nov 1, 2019
1 parent 1fc8310 commit 8b9bbfe
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 6 deletions.
14 changes: 14 additions & 0 deletions docs/reference/module-types/container.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
14 changes: 14 additions & 0 deletions docs/reference/module-types/maven-container.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
12 changes: 11 additions & 1 deletion garden-service/src/plugins/container/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
9 changes: 5 additions & 4 deletions garden-service/src/plugins/kubernetes/container/deployment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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[] = []

Expand All @@ -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)
Expand Down
3 changes: 2 additions & 1 deletion garden-service/src/plugins/local/local-docker-swarm.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 8b9bbfe

Please sign in to comment.