Skip to content

Commit

Permalink
fix(k8s): fix hot reload path handling on Windows
Browse files Browse the repository at this point in the history
  • Loading branch information
edvald authored and eysi09 committed Aug 5, 2020
1 parent cfe399c commit f0c3001
Show file tree
Hide file tree
Showing 6 changed files with 80 additions and 28 deletions.
21 changes: 19 additions & 2 deletions garden-service/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions garden-service/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
"typeorm": "^0.2.25",
"typescript-memoize": "^1.0.0-alpha.3",
"uniqid": "^5.2.0",
"unixify": "^1.0.0",
"unzipper": "^0.10.11",
"username": "^5.1.0",
"uuid": "^8.1.0",
Expand Down
5 changes: 2 additions & 3 deletions garden-service/src/build-dir.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,11 @@

import { map as bluebirdMap } from "bluebird"
import semver from "semver"
import normalize = require("normalize-path")
import { isAbsolute, join, parse, resolve, sep, relative } from "path"
import { emptyDir, ensureDir } from "fs-extra"
import { ConfigurationError, RuntimeError } from "./exceptions"
import { FileCopySpec, Module, getModuleKey } from "./types/module"
import { normalizeLocalRsyncPath } from "./util/fs"
import { normalizeLocalRsyncPath, normalizeRelativePath } from "./util/fs"
import { LogEntry } from "./logger/log-entry"
import { ModuleConfig } from "./config/module"
import { ConfigGraph } from "./config-graph"
Expand Down Expand Up @@ -100,7 +99,7 @@ export class BuildDir {
}

// Normalize to relative POSIX-style paths
const files = module.version.files.map((f) => normalize(isAbsolute(f) ? relative(module.path, f) : f))
const files = module.version.files.map((f) => normalizeRelativePath(module.path, f))

await this.sync({
module,
Expand Down
24 changes: 14 additions & 10 deletions garden-service/src/plugins/kubernetes/hot-reload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import normalizePath = require("normalize-path")
import { V1Deployment, V1DaemonSet, V1StatefulSet, V1Container } from "@kubernetes/client-node"
import { ContainerModule, ContainerHotReloadSpec } from "../container/config"
import { RuntimeError, ConfigurationError } from "../../exceptions"
import { resolve as resolvePath, dirname, posix, relative, resolve } from "path"
import { resolve as resolvePath, dirname, posix } from "path"
import { deline, gardenAnnotationKey } from "../../util/string"
import { set, sortBy, flatten } from "lodash"
import { Service } from "../../types/service"
Expand All @@ -31,14 +31,11 @@ import { labelSelectorToString } from "./util"
import { KubeApi } from "./api"
import { syncWithOptions } from "../../util/sync"
import { Module } from "../../types/module"
import { sleep } from "../../util/util"

export const RSYNC_PORT_NAME = "garden-rsync"

export type HotReloadableResource = KubernetesResource<V1Deployment | V1DaemonSet | V1StatefulSet>

export type HotReloadableKind = "Deployment" | "DaemonSet" | "StatefulSet"

export const RSYNC_PORT_NAME = "garden-rsync"
export const hotReloadableKinds: HotReloadableKind[] = ["Deployment", "DaemonSet", "StatefulSet"]

interface ConfigureHotReloadParams {
Expand Down Expand Up @@ -387,9 +384,16 @@ export async function syncToService({ ctx, service, hotReloadSpec, namespace, wo
* `subdir/myfile` in the output, and if `source` = `.` or `*`, it would be transformed to `mydir/subdir/myfile`.
*/
export function filesForSync(module: Module, source: string): string[] {
const normalizedSource = resolve(module.path, source.replace("**/", "").replace("*", ""))
const moduleFiles = module.version.files
const files = normalizedSource === "" ? moduleFiles : moduleFiles.filter((path) => path.startsWith(normalizedSource))
const normalizedFiles = files.map((f) => relative(normalizedSource, f))
return normalizedFiles
const normalizedSource = source.replace("**/", "").replace("*", "")

// Normalize to relative POSIX-style paths
const moduleFiles = module.version.files.map((f) => normalizeRelativePath(module.path, f))

if (normalizedSource === "" || normalizedSource === ".") {
return moduleFiles
} else {
return moduleFiles
.filter((path) => path.startsWith(normalizedSource))
.map((path) => posix.relative(normalizedSource, path))
}
}
13 changes: 12 additions & 1 deletion garden-service/src/util/fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,17 @@
* file, You can obtain one at http://mozilla.org/MPL/2.0/.
*/

import unixify = require("unixify")
import klaw = require("klaw")
import glob from "glob"
import _spawn from "cross-spawn"
import { pathExists, readFile, writeFile, lstat } from "fs-extra"
import minimatch = require("minimatch")
import { some } from "lodash"
import { join, basename, win32, posix } from "path"
import { FilesystemError } from "../exceptions"
import { platform } from "os"

import { FilesystemError } from "../exceptions"
import { VcsHandler } from "../vcs/vcs"
import { LogEntry } from "../logger/log-entry"
import { ModuleConfig } from "../config/module"
Expand Down Expand Up @@ -156,6 +158,15 @@ export function normalizeLocalRsyncPath(path: string) {
return platform() === "win32" ? toCygwinPath(path) : path
}

/**
* Normalize given path to POSIX-style path relative to `root`
*/
export function normalizeRelativePath(root: string, path: string) {
root = unixify(root)
path = unixify(path)
return posix.isAbsolute(path) ? posix.relative(root, path) : path
}

/**
* Return a list of all files in directory at `path`
*/
Expand Down
44 changes: 32 additions & 12 deletions garden-service/test/unit/src/plugins/kubernetes/hot-reload.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@
import { platform } from "os"
import { expect } from "chai"
import td from "testdouble"
import { HotReloadableResource, rsyncSourcePath, filesForSync, RSYNC_PORT_NAME } from "../../../../../src/plugins/kubernetes/hot-reload"
import {
HotReloadableResource,
rsyncSourcePath,
filesForSync,
RSYNC_PORT_NAME,
} from "../../../../../src/plugins/kubernetes/hot-reload"

import {
removeTrailingSlashes,
Expand All @@ -18,6 +23,7 @@ import {
} from "../../../../../src/plugins/kubernetes/hot-reload"
import { setPlatform, makeTestGarden, TestGarden, getDataDir } from "../../../../helpers"
import { ConfigGraph } from "../../../../../src/config-graph"
import { cloneDeep } from "lodash"

describe("configureHotReload", () => {
it("should correctly augment a resource manifest with containers and volume for hot reloading", async () => {
Expand Down Expand Up @@ -90,6 +96,14 @@ describe("configureHotReload", () => {
value: "0.0.0.0/0",
},
],
readinessProbe: {
initialDelaySeconds: 2,
periodSeconds: 1,
timeoutSeconds: 3,
successThreshold: 1,
failureThreshold: 5,
tcpSocket: { port: <object>(<unknown>RSYNC_PORT_NAME) },
},
volumeMounts: [
{
name: "garden-sync",
Expand All @@ -105,14 +119,6 @@ describe("configureHotReload", () => {
],
},
],
readinessProbe: {
initialDelaySeconds: 2,
periodSeconds: 1,
timeoutSeconds: 3,
successThreshold: 1,
failureThreshold: 5,
tcpSocket: { port: <object>(<unknown>RSYNC_PORT_NAME) },
},
volumes: [
{
name: "garden-sync",
Expand Down Expand Up @@ -309,20 +315,34 @@ describe("filesForSync", () => {
})

it("should respect module include and exclude", async () => {
const moduleA = await graph.getModule("module-a")
const moduleA = graph.getModule("module-a")
const files = filesForSync(moduleA, "*")
expect(files).to.eql(["somedir/yes.txt", "yes.txt"])
})

it("should treat '.' sources the same as '*'", async () => {
const moduleA = await graph.getModule("module-a")
const moduleA = graph.getModule("module-a")
const files = filesForSync(moduleA, ".")
expect(files).to.eql(["somedir/yes.txt", "yes.txt"])
})

it("should filter files on source prefix, and return the relative path from the source path", async () => {
const moduleA = await graph.getModule("module-a")
const moduleA = graph.getModule("module-a")
const files = filesForSync(moduleA, "somedir")
expect(files).to.eql(["yes.txt"])
})

it("should correctly handle Windows paths", async () => {
const moduleA = cloneDeep(graph.getModule("module-a"))

moduleA.path = "C:\\Some Directory\\code\\module-a"
moduleA.version.files = [
"C:\\Some Directory\\code\\module-a\\somedir\\yes.txt",
"C:\\Some Directory\\code\\module-a\\yes.txt",
]

expect(filesForSync(moduleA, "somedir")).to.eql(["yes.txt"])
expect(filesForSync(moduleA, "*")).to.eql(["somedir/yes.txt", "yes.txt"])
expect(filesForSync(moduleA, ".")).to.eql(["somedir/yes.txt", "yes.txt"])
})
})

0 comments on commit f0c3001

Please sign in to comment.