fix(perf): reuse port-forwards when hot-reloading

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

@@ -7,6 +7,7 @@
  */
 
 import * as Bluebird from "bluebird"
+import { ChildProcess } from "child_process"
 import * as execa from "execa"
 import { V1Deployment, V1DaemonSet, V1StatefulSet, V1ObjectMeta } from "@kubernetes/client-node"
 import { HotReloadServiceParams } from "../../types/plugin/params"
@@ -17,7 +18,7 @@ import { kubectl } from "./kubectl"
 import getPort = require("get-port")
 import { RuntimeError, ConfigurationError } from "../../exceptions"
 import { resolve as resolvePath, normalize } from "path"
-import { Omit } from "../../util/util"
+import { Omit, registerCleanupFunction } from "../../util/util"
 import { deline } from "../../util/string"
 import { set } from "lodash"
 import { Service } from "../../types/service"
@@ -174,57 +175,6 @@ export async function hotReloadContainer(
   return {}
 }
 
-/**
- * Ensure a tunnel is set up for connecting to the target service's sync container, and perform a sync.
- */
-export async function syncToService(
-  ctx: PluginContext,
-  service: Service,
-  hotReloadSpec: ContainerHotReloadSpec,
-  targetKind: HotReloadableKind,
-  targetName: string,
-  log: LogEntry,
-) {
-  const namespace = await getAppNamespace(ctx, ctx.provider)
-
-  // Forward random free local port to the remote rsync container.
-  const rsyncLocalPort = await getPort()
-
-  const targetDeployment = `${targetKind.toLowerCase()}/${targetName}`
-  const portMapping = `${rsyncLocalPort}:${RSYNC_PORT}`
-
-  log.debug(
-    `Forwarding local port ${rsyncLocalPort} to ${targetDeployment} sync container port ${RSYNC_PORT}`,
-  )
-
-  // TODO: use the API directly instead of kubectl (need to reverse engineer kubectl a bit to get how that works)
-  const proc = kubectl(ctx.provider.config.context, namespace)
-    .spawn(["port-forward", targetDeployment, portMapping])
-
-  return new Promise((resolve, reject) => {
-    proc.on("error", (error) => {
-      reject(new RuntimeError(`Unexpected error while synchronising to service ${service.name}: ${error.message}`, {
-        error,
-        serviceName: service.name,
-      }))
-    })
-
-    proc.stdout.on("data", (line) => {
-      // This is unfortunately the best indication that we have that the connection is up...
-      if (line.toString().includes("Forwarding from ")) {
-        Bluebird.map(hotReloadSpec.sync, ({ source, target }) => {
-          const src = rsyncSourcePath(service.sourceModule.path, source)
-          const destination = `rsync://localhost:${rsyncLocalPort}/volume/${rsyncTargetPath(target)}`
-          return execa("rsync", ["-vrptgo", src, destination])
-        })
-          .then(resolve)
-          .catch(reject)
-          .finally(() => !proc.killed && proc.kill())
-      }
-    })
-  })
-}
-
 /**
  * Creates the initial copy command for the sync init container.
  *
@@ -272,3 +222,101 @@ export function rsyncTargetPath(path: string) {
   return path.replace(/^\/*/, "")
     .replace(/\/*$/, "/")
 }
+
+/**
+ * Below is the logic that manages syncing into a service's running container.
+ *
+ * Before performing a sync, we set up a port-forward from a randomly allocated local port to the rsync sidecar
+ * container attached to the target service's container.
+ *
+ * Since hot-reloading is a time-sensitive operation for the end-user, and because setting up this port-forward
+ * can take several tens of milliseconds, we maintain a simple in-process cache of previously allocated ports
+ * (registeredPortForwards below). Therefore, subsequent hot reloads after the initial one (during the execution
+ * of the enclosing Garden command) finish more quickly.
+ */
+
+type PortForward = {
+  rsyncLocalPort: number,
+  proc: ChildProcess,
+}
+
+const registeredPortForwards: { [targetDeployment: string]: PortForward } = {}
+
+registerCleanupFunction("kill-hot-reload-port-forward-procs", () => {
+  for (const { proc } of Object.values(registeredPortForwards)) {
+    !proc.killed && proc.kill()
+  }
+})
+
+/**
+ * Ensure a tunnel is set up for connecting to the target service's sync container, and perform a sync.
+ */
+export async function syncToService(
+  ctx: PluginContext,
+  service: Service,
+  hotReloadSpec: ContainerHotReloadSpec,
+  targetKind: HotReloadableKind,
+  targetName: string,
+  log: LogEntry,
+) {
+
+  let rsyncLocalPort
+  const targetDeployment = `${targetKind.toLowerCase()}/${targetName}`
+
+  try {
+    rsyncLocalPort = await getLocalRsyncPort(ctx, log, targetDeployment)
+  } catch (error) {
+    throw new RuntimeError(`Unexpected error while synchronising to service ${service.name}: ${error.message}`, {
+      error,
+      serviceName: service.name,
+    })
+  }
+
+  return Bluebird.map(hotReloadSpec.sync, ({ source, target }) => {
+    const src = rsyncSourcePath(service.sourceModule.path, source)
+    const destination = `rsync://localhost:${rsyncLocalPort}/volume/${rsyncTargetPath(target)}`
+    return execa("rsync", ["-vrptgo", src, destination])
+  })
+
+}
+
+async function getLocalRsyncPort(ctx: PluginContext, log: LogEntry, targetDeployment: string): Promise<number> {
+
+  let rsyncLocalPort
+
+  const registered = registeredPortForwards[targetDeployment]
+
+  if (registered && !registered.proc.killed) {
+    rsyncLocalPort = registered.rsyncLocalPort
+    log.debug(`Reusing local port ${rsyncLocalPort} for ${targetDeployment} sync container`)
+    return rsyncLocalPort
+  }
+
+  const namespace = await getAppNamespace(ctx, ctx.provider)
+
+  // Forward random free local port to the remote rsync container.
+  rsyncLocalPort = await getPort()
+  const portMapping = `${rsyncLocalPort}:${RSYNC_PORT}`
+
+  log.debug(`Forwarding local port ${rsyncLocalPort} to ${targetDeployment} sync container port ${RSYNC_PORT}`)
+
+  // TODO: use the API directly instead of kubectl (need to reverse engineer kubectl a bit to get how that works)
+  const proc = kubectl(ctx.provider.config.context, namespace)
+    .spawn(["port-forward", targetDeployment, portMapping])
+
+  return new Promise((resolve) => {
+    proc.on("error", (error) => {
+      !proc.killed && proc.kill()
+      throw error
+    })
+
+    proc.stdout.on("data", (line) => {
+      // This is unfortunately the best indication that we have that the connection is up...
+      if (line.toString().includes("Forwarding from ")) {
+        const portForward = { proc, rsyncLocalPort }
+        registeredPortForwards[targetDeployment] = portForward
+        resolve(rsyncLocalPort)
+      }
+    })
+  })
+}