Port forward URL doesn't work after re-deploy

[eysi09]

Bug

Current Behavior

After a re-deploy in watch mode, the port-forward link that Garden prints stops working (→ Forward: http://localhost:60749 → frontend:8080 (http)) and the page returns an empty response.

Expected behavior

The link continues to work.

Reproducible example

1. garden deploy -w in demo-project.
2. Open the Forward: http://localhost:<port>... link for, say, the frontend service
3. Change some code to trigger a re-deploy of the service
4. Try the link again and notice that it doesn't work

[edvald]

Huh. I somehow believed the underlying port-forward connection would close when the connected pod is killed. Guess we need more elaborate connection management to account for that then.

[mjgallag]

Ran into this issue today and did a little research, here's how Google's Skaffold worked around it.

[olivercodes]

If nobody is working on it, I'd like to help (and make my first contribution to garden). While I work, is the community slack the best place to ask questions?

Current investigation lead me to this, thinking of trying to call killPortForward within

garden/garden-service/src/plugins/kubernetes/port-forward.ts

Lines 58 to 64 in 4fa85dc

	/**
	* Creates or re-uses an existing tunnel to a Kubernetes resources.
	*
	* We maintain a simple in-process cache of randomly allocated local ports that have been port-forwarded to a
	* given port on a given Kubernetes resource.
	*/
	export async function getPortForward({

My first thought though is that it's probably bad UX, since if we force a new one every time, the localhost port will be different every time the user saves and deploys again (and instead of just refreshing, they have to type in the new port if this is say, a frontend application they are working on).

--

And how does this work?

garden/garden-service/src/plugins/kubernetes/port-forward.ts

Lines 85 to 94 in 4fa85dc

	if (registered && !registered.proc.killed) {
	log.debug(`Reusing local port ${registered.localPort} for ${targetResource}`)
	return registered
	}
	const k8sCtx = <KubernetesPluginContext>ctx
	// Forward random free local port to the remote rsync container.
	localPort = await getPort()
	const portMapping = `${localPort}:${port}`

...we check to see if the registered portFwd is running, and log out that we'll reuse it. But then in line 93, we don't seem to use it (unless this get-port package is somehow aware of the registered port).

[edvald]

Hey @olivercodes! Sorry about the late response, been taking it easy over the holidays :P

Probably best to start by explaining the overall architecture here.

The Garden CLI itself creates its own basic TCP proxy (see https://github.com/garden-io/garden/blob/master/garden-service/src/proxy.ts), and binds the user-facing local ports (the ones we print the CLI) as soon as we get an indication of which ports are exposed on the deployed services. Then, when the user connects to one of those ports, we call out to the getPortForward handler on the provider (currently just implemented on the kubernetes provider) to start an actual tunnel to the service port. This means that A) we don't create unnecessary tunnels on startup, while at the same B) we keep a persistent user-facing port even if the k8s port forward needs to be replaced.

What's broken here is the reconnection when the remote service is replaced. It's actually broken in two places, both in the k8s provider (where the port-forward process stays up even though we can no longer connect through it), and the failure handling in the Garden proxy is not really implemented (

garden/garden-service/src/proxy.ts

Line 55 in b244c8e

// TODO: handle dead port forwards

).

I just posted #1467 which deals with a separate thing (and tbh has more to do with the introduction of the sqlite datastore) but is in the general realm of port forwarding. I'll take a quick look at this now to troubleshoot, and ping back with results :)

[olivercodes]

@edvald - no worries, I had some free time on the holidays and went poking around.

It seems like your pr may fix this issue, happy to help test if that's more valuable.