From f0c3001ddcd4f66dffdb68176e3ad37b86aa6dbe Mon Sep 17 00:00:00 2001 From: Jon Edvald Date: Mon, 3 Aug 2020 19:06:07 +0200 Subject: [PATCH] fix(k8s): fix hot reload path handling on Windows --- garden-service/package-lock.json | 21 ++++++++- garden-service/package.json | 1 + garden-service/src/build-dir.ts | 5 +-- .../src/plugins/kubernetes/hot-reload.ts | 24 +++++----- garden-service/src/util/fs.ts | 13 +++++- .../unit/src/plugins/kubernetes/hot-reload.ts | 44 ++++++++++++++----- 6 files changed, 80 insertions(+), 28 deletions(-) diff --git a/garden-service/package-lock.json b/garden-service/package-lock.json index a1411402ea..58a839c269 100644 --- a/garden-service/package-lock.json +++ b/garden-service/package-lock.json @@ -11540,8 +11540,7 @@ "remove-trailing-separator": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/remove-trailing-separator/-/remove-trailing-separator-1.1.0.tgz", - "integrity": "sha1-wkvOKig62tW8P1jg1IJJuSN52O8=", - "dev": true + "integrity": "sha1-wkvOKig62tW8P1jg1IJJuSN52O8=" }, "remove-trailing-slash": { "version": "0.1.0", @@ -13587,6 +13586,24 @@ "resolved": "https://registry.npmjs.org/universalify/-/universalify-1.0.0.tgz", "integrity": "sha512-rb6X1W158d7pRQBg5gkR8uPaSfiids68LTJQYOtEUhoJUWBdaQHsuT/EUduxXYxcrt4r5PJ4fuHW1MHT6p0qug==" }, + "unixify": { + "version": "1.0.0", + "resolved": "https://registry.npmjs.org/unixify/-/unixify-1.0.0.tgz", + "integrity": "sha1-OmQcjC/7zk2mg6XHDwOkYpQMIJA=", + "requires": { + "normalize-path": "^2.1.1" + }, + "dependencies": { + "normalize-path": { + "version": "2.1.1", + "resolved": "https://registry.npmjs.org/normalize-path/-/normalize-path-2.1.1.tgz", + "integrity": "sha1-GrKLVW4Zg2Oowab35vogE3/mrtk=", + "requires": { + "remove-trailing-separator": "^1.0.1" + } + } + } + }, "unpipe": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/unpipe/-/unpipe-1.0.0.tgz", diff --git a/garden-service/package.json b/garden-service/package.json index 6b6e48c362..ad70b2a3d4 100644 --- a/garden-service/package.json +++ b/garden-service/package.json @@ -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", diff --git a/garden-service/src/build-dir.ts b/garden-service/src/build-dir.ts index 69b928bf9e..1c56fa27ed 100644 --- a/garden-service/src/build-dir.ts +++ b/garden-service/src/build-dir.ts @@ -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" @@ -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, diff --git a/garden-service/src/plugins/kubernetes/hot-reload.ts b/garden-service/src/plugins/kubernetes/hot-reload.ts index ea58231589..b395e79b09 100644 --- a/garden-service/src/plugins/kubernetes/hot-reload.ts +++ b/garden-service/src/plugins/kubernetes/hot-reload.ts @@ -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" @@ -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 - export type HotReloadableKind = "Deployment" | "DaemonSet" | "StatefulSet" +export const RSYNC_PORT_NAME = "garden-rsync" export const hotReloadableKinds: HotReloadableKind[] = ["Deployment", "DaemonSet", "StatefulSet"] interface ConfigureHotReloadParams { @@ -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)) + } } diff --git a/garden-service/src/util/fs.ts b/garden-service/src/util/fs.ts index c12c29bb96..a5f73eed35 100644 --- a/garden-service/src/util/fs.ts +++ b/garden-service/src/util/fs.ts @@ -6,6 +6,7 @@ * 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" @@ -13,8 +14,9 @@ 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" @@ -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` */ diff --git a/garden-service/test/unit/src/plugins/kubernetes/hot-reload.ts b/garden-service/test/unit/src/plugins/kubernetes/hot-reload.ts index 4d31543665..2fd19dba4a 100644 --- a/garden-service/test/unit/src/plugins/kubernetes/hot-reload.ts +++ b/garden-service/test/unit/src/plugins/kubernetes/hot-reload.ts @@ -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, @@ -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 () => { @@ -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: (RSYNC_PORT_NAME) }, + }, volumeMounts: [ { name: "garden-sync", @@ -105,14 +119,6 @@ describe("configureHotReload", () => { ], }, ], - readinessProbe: { - initialDelaySeconds: 2, - periodSeconds: 1, - timeoutSeconds: 3, - successThreshold: 1, - failureThreshold: 5, - tcpSocket: { port: (RSYNC_PORT_NAME) }, - }, volumes: [ { name: "garden-sync", @@ -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"]) + }) })