fix: use abs target paths in HR copy commands

This fixes various issues with hot reload sync configuration and
simplifies the internal logic.

One issue resolved by this change is that Dockerfiles with WORKDIRs
nested under one of the sync targets no longer cause problems with hot
reloading.

garden-service/src/plugins/kubernetes/hot-reload.ts

@@ -17,7 +17,7 @@ import { getAppNamespace } from "./namespace"
 import { kubectl } from "./kubectl"
 import getPort = require("get-port")
 import { RuntimeError, ConfigurationError } from "../../exceptions"
-import { resolve as resolvePath, normalize } from "path"
+import { resolve as resolvePath, normalize, dirname } from "path"
 import { Omit, registerCleanupFunction } from "../../util/util"
 import { deline } from "../../util/string"
 import { set } from "lodash"
@@ -26,7 +26,6 @@ import { PluginContext } from "../../plugin-context"
 import { LogEntry } from "../../logger/log-entry"
 import { getResourceContainer } from "./helm/common"
 import { waitForContainerService } from "./container/status"
-import { FileCopySpec } from "../../types/module"
 
 export const RSYNC_PORT = 873
 export const RSYNC_PORT_NAME = "garden-rsync"
@@ -65,14 +64,13 @@ export function configureHotReload({
 
   // We're copying the target folder, not just its contents
   const syncConfig = hotReloadSpec.sync
-  const syncTarget = syncConfig.map(pair => removeTrailingSlashes(pair.target))
-
-  const copyCommands = makeCopyCommands(syncConfig)
+  const targets = syncConfig.map(pair => removeTrailingSlashes(pair.target))
+  const copyCommand = makeCopyCommand(targets)
 
   const initContainer = {
     name: "garden-sync-init",
     image: mainContainer.image,
-    command: ["/bin/sh", "-c", copyCommands],
+    command: ["/bin/sh", "-c", copyCommand],
     env: mainContainer.env || [],
     imagePullPolicy: "IfNotPresent",
     volumeMounts: [{
@@ -81,7 +79,7 @@ export function configureHotReload({
     }],
   }
 
-  const syncMounts = syncTarget.map(t => {
+  const syncMounts = targets.map(t => {
     return {
       name: syncVolumeName,
       mountPath: t,
@@ -178,31 +176,24 @@ export async function hotReloadContainer(
 /**
  * Creates the initial copy command for the sync init container.
  *
- * This handles copying the contents from source into a volume for
- * the rsync container to update.
+ * This handles copying the target paths from the service's container into a volume that is then shared with the
+ * rsync sidecar container.
  *
- * This needs to deal with nested pathing as well as root.
- * This will first create the path, and then copy the contents from the
- * docker image into the shared volume as a base for the rsync command
- * to update.
+ * Changes to a source path in a given sync spec are then applied to the corresponding target path (from the same
+ * spec) inside the rsync sidecar container, which propagates the changes into the running service's container
+ * (which mounts mounts the volume at the appropriate subpaths).
  *
- * @param syncConfig
+ * @param syncTargets
  */
-export function makeCopyCommands(syncConfig: FileCopySpec[]) {
-  const commands = syncConfig.map(({ source, target }) => {
-    const adjustedSource = `${removeTrailingSlashes(source)}/.`
-    const absTarget = normalize(`/.garden/hot_reload/${target}/`)
-    return `mkdir -p ${absTarget} && cp -r ${adjustedSource} ${absTarget}`
+export function makeCopyCommand(syncTargets: string[]) {
+  const commands = syncTargets.map((target) => {
+    const syncCopySource = normalize(`${target}/`)
+    const syncVolumeTarget = normalize(`/.garden/hot_reload/${target}/`)
+    return `mkdir -p ${dirname(syncVolumeTarget)} && cp -r ${syncCopySource} ${syncVolumeTarget}`
   })
   return commands.join(" && ")
 }
 
-/**
- * Ensure that there's no trailing slash
- *
- * converts /src/foo/ into /src/foo
- * @param path
- */
 export function removeTrailingSlashes(path: string) {
   return path.replace(/\/*$/, "")
 }
@@ -215,7 +206,7 @@ export function rsyncSourcePath(modulePath: string, sourcePath: string) {
 /**
  * Removes leading slash, and ensures there's exactly one trailing slash.
  *
- * converts /src/foo into src/foo/
+ * Converts /src/foo into src/foo/
  * @param target
  */
 export function rsyncTargetPath(path: string) {

garden-service/test/src/plugins/kubernetes/hot-reload.ts

@@ -3,7 +3,7 @@ import { HotReloadableResource } from "../../../../src/plugins/kubernetes/hot-re
 
 import {
   removeTrailingSlashes,
-  makeCopyCommands,
+  makeCopyCommand,
   configureHotReload,
 } from "../../../../src/plugins/kubernetes/hot-reload"
 
@@ -105,7 +105,7 @@ describe("configureHotReload", () => {
                 command: [
                   "/bin/sh",
                   "-c",
-                  "mkdir -p /.garden/hot_reload/app/ && cp -r */. /.garden/hot_reload/app/",
+                  "mkdir -p /.garden/hot_reload && cp -r /app/ /.garden/hot_reload/app/",
                 ],
                 env: [],
                 imagePullPolicy: "IfNotPresent",
@@ -140,42 +140,23 @@ describe("removeTrailingSlashes", () => {
   }
 })
 
-describe("makeCopyCommands", () => {
-  // Test cases for each type
-  const configs: any[] = [
-    // Source is missing slash
-    [
-      [{ source: "src", target: "/app/src" }],
-      "mkdir -p /.garden/hot_reload/app/src/ && cp -r src/. /.garden/hot_reload/app/src/",
-    ],
-    // Source ends on slash
-    [
-      [{ source: "src/", target: "/app/src" }],
-      "mkdir -p /.garden/hot_reload/app/src/ && cp -r src/. /.garden/hot_reload/app/src/",
-    ],
-    // Base root of the module
-    [
-      [{ source: ".", target: "/app" }],
-      "mkdir -p /.garden/hot_reload/app/ && cp -r ./. /.garden/hot_reload/app/",
-    ],
-    // Subdirectory not ending on a slash
-    [
-      [{ source: "src/foo", target: "/app/foo" }],
-      "mkdir -p /.garden/hot_reload/app/foo/ && cp -r src/foo/. /.garden/hot_reload/app/foo/",
-    ],
-    // Multiple pairs
-    [
-      [
-        { source: "src1", target: "/app/src1" },
-        { source: "src2", target: "/app/src2" },
-      ],
-      "mkdir -p /.garden/hot_reload/app/src1/ && cp -r src1/. /.garden/hot_reload/app/src1/ && " +
-      "mkdir -p /.garden/hot_reload/app/src2/ && cp -r src2/. /.garden/hot_reload/app/src2/",
-    ],
-  ]
-  for (const config of configs) {
-    it("correctly generates copy commands for the hot reload init container", () => {
-      expect(makeCopyCommands(config[0])).to.eql(config[1])
-    })
-  }
+describe("makeCopyCommand", () => {
+
+  it("ensures a trailing slash in the copy source and target", () => {
+    const cmd = "mkdir -p /.garden/hot_reload && cp -r /app/ /.garden/hot_reload/app/"
+    expect(makeCopyCommand(["/app/"])).to.eql(cmd)
+    expect(makeCopyCommand(["/app"])).to.eql(cmd)
+  })
+
+  it("correctly handles target paths with more than one path component", () => {
+    const cmd = "mkdir -p /.garden/hot_reload/app/src && cp -r /app/src/foo/ /.garden/hot_reload/app/src/foo/"
+    expect(makeCopyCommand(["/app/src/foo"])).to.eql(cmd)
+  })
+
+  it("correctly handles multiple target paths", () => {
+    const cmd = "mkdir -p /.garden/hot_reload/app && cp -r /app/src1/ /.garden/hot_reload/app/src1/ && " +
+      "mkdir -p /.garden/hot_reload/app && cp -r /app/src2/ /.garden/hot_reload/app/src2/"
+    expect(makeCopyCommand(["/app/src1", "/app/src2/"])).to.eql(cmd)
+  })
+
 })