From 2ce4c0c71c9ada51af7a0ff9303c6d3402e14828 Mon Sep 17 00:00:00 2001 From: Jim Ehrismann Date: Wed, 22 Dec 2021 18:11:56 -0500 Subject: [PATCH] fixed remaining issues around port-forward dialog, changed port-forward-item id to resource name, etc from local port Signed-off-by: Jim Ehrismann --- .../+network-port-forwards/port-forwards.tsx | 2 +- .../service-port-component.tsx | 34 +++++++++++-------- .../+workloads-pods/pod-container-port.tsx | 32 ++++++++++------- .../port-forward/port-forward-dialog.tsx | 32 ++++++++++++----- .../port-forward/port-forward-item.ts | 2 +- 5 files changed, 63 insertions(+), 39 deletions(-) diff --git a/src/renderer/components/+network-port-forwards/port-forwards.tsx b/src/renderer/components/+network-port-forwards/port-forwards.tsx index a76588619d..b6d4fa5fbf 100644 --- a/src/renderer/components/+network-port-forwards/port-forwards.tsx +++ b/src/renderer/components/+network-port-forwards/port-forwards.tsx @@ -70,7 +70,7 @@ export class PortForwards extends React.Component { showDetails = (item: PortForwardItem) => { navigation.push(portForwardsURL({ params: { - forwardport: String(item.getForwardPort()), + forwardport: item.getId(), }, })); }; diff --git a/src/renderer/components/+network-services/service-port-component.tsx b/src/renderer/components/+network-services/service-port-component.tsx index 1cc286f30d..7a1a3d8be3 100644 --- a/src/renderer/components/+network-services/service-port-component.tsx +++ b/src/renderer/components/+network-services/service-port-component.tsx @@ -28,7 +28,7 @@ import { observable, makeObservable, reaction, action } from "mobx"; import { cssNames } from "../../utils"; import { Notifications } from "../notifications"; import { Button } from "../button"; -import { aboutPortForwarding, addPortForward, getPortForward, getPortForwards, openPortForward, PortForwardDialog, portForwardStore, predictProtocol, removePortForward } from "../../port-forward"; +import { aboutPortForwarding, addPortForward, getPortForward, getPortForwards, openPortForward, PortForwardDialog, predictProtocol, removePortForward, startPortForward } from "../../port-forward"; import type { ForwardedPort } from "../../port-forward"; import { Spinner } from "../spinner"; import logger from "../../../common/logger"; @@ -43,6 +43,7 @@ export class ServicePortComponent extends React.Component { @observable waiting = false; @observable forwardPort = 0; @observable isPortForwarded = false; + @observable isActive = false; constructor(props: Props) { super(props); @@ -52,10 +53,11 @@ export class ServicePortComponent extends React.Component { componentDidMount() { disposeOnUnmount(this, [ - reaction(() => [portForwardStore.portForwards.slice(), this.props.service], () => this.checkExistingPortForwarding()), + reaction(() => this.props.service, () => this.checkExistingPortForwarding()), ]); } + @action async checkExistingPortForwarding() { const { service, port } = this.props; let portForward: ForwardedPort = { @@ -70,12 +72,14 @@ export class ServicePortComponent extends React.Component { portForward = await getPortForward(portForward); } catch (error) { this.isPortForwarded = false; + this.isActive = false; return; } this.forwardPort = portForward.forwardPort; - this.isPortForwarded = (portForward.status === "Active" && portForward.forwardPort) ? true : false; + this.isPortForwarded = true; + this.isActive = portForward.status === "Active"; } @action @@ -97,13 +101,16 @@ export class ServicePortComponent extends React.Component { // determine how many port-forwards already exist const { length } = getPortForwards(); - portForward = await addPortForward(portForward); + if (!this.isPortForwarded) { + portForward = await addPortForward(portForward); + } else if (!this.isActive) { + portForward = await startPortForward(portForward); + } this.forwardPort = portForward.forwardPort; if (portForward.status === "Active") { openPortForward(portForward); - this.isPortForwarded = true; // if this is the first port-forward show the about notification if (!length) { @@ -111,12 +118,11 @@ export class ServicePortComponent extends React.Component { } } else { Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`); - this.isPortForwarded = false; } } catch (error) { logger.error("[SERVICE-PORT-COMPONENT]:", error, portForward); - this.checkExistingPortForwarding(); } finally { + this.checkExistingPortForwarding(); this.waiting = false; } } @@ -134,15 +140,13 @@ export class ServicePortComponent extends React.Component { this.waiting = true; - console.log("stopPortForward()"); - try { await removePortForward(portForward); - this.isPortForwarded = false; } catch (error) { Notifications.error(`Error occurred stopping the port-forward from port ${portForward.forwardPort}.`); - this.checkExistingPortForwarding(); } finally { + this.checkExistingPortForwarding(); + this.forwardPort = 0; this.waiting = false; } } @@ -150,7 +154,7 @@ export class ServicePortComponent extends React.Component { render() { const { port, service } = this.props; - const portForwardAction = async () => { + const portForwardAction = action(async () => { if (this.isPortForwarded) { await this.stopPortForward(); } else { @@ -163,16 +167,16 @@ export class ServicePortComponent extends React.Component { protocol: predictProtocol(port.name), }; - PortForwardDialog.open(portForward, { openInBrowser: true }); + PortForwardDialog.open(portForward, { openInBrowser: true, onClose: () => this.checkExistingPortForwarding() }); } - }; + }); return (
this.portForward()}> {port.toString()} - + {this.waiting && ( )} diff --git a/src/renderer/components/+workloads-pods/pod-container-port.tsx b/src/renderer/components/+workloads-pods/pod-container-port.tsx index f0a933b6a6..1d23300523 100644 --- a/src/renderer/components/+workloads-pods/pod-container-port.tsx +++ b/src/renderer/components/+workloads-pods/pod-container-port.tsx @@ -28,7 +28,7 @@ import { action, observable, makeObservable, reaction } from "mobx"; import { cssNames } from "../../utils"; import { Notifications } from "../notifications"; import { Button } from "../button"; -import { aboutPortForwarding, addPortForward, getPortForward, getPortForwards, openPortForward, PortForwardDialog, portForwardStore, predictProtocol, removePortForward } from "../../port-forward";import type { ForwardedPort } from "../../port-forward"; +import { aboutPortForwarding, addPortForward, getPortForward, getPortForwards, openPortForward, PortForwardDialog, predictProtocol, removePortForward, startPortForward } from "../../port-forward";import type { ForwardedPort } from "../../port-forward"; import { Spinner } from "../spinner"; import logger from "../../../common/logger"; @@ -46,6 +46,7 @@ export class PodContainerPort extends React.Component { @observable waiting = false; @observable forwardPort = 0; @observable isPortForwarded = false; + @observable isActive = false; constructor(props: Props) { super(props); @@ -55,10 +56,11 @@ export class PodContainerPort extends React.Component { componentDidMount() { disposeOnUnmount(this, [ - reaction(() => [portForwardStore.portForwards.slice(), this.props.pod], () => this.checkExistingPortForwarding()), + reaction(() => this.props.pod, () => this.checkExistingPortForwarding()), ]); } + @action async checkExistingPortForwarding() { const { pod, port } = this.props; let portForward: ForwardedPort = { @@ -73,12 +75,14 @@ export class PodContainerPort extends React.Component { portForward = await getPortForward(portForward); } catch (error) { this.isPortForwarded = false; + this.isActive = false; return; } this.forwardPort = portForward.forwardPort; - this.isPortForwarded = (portForward.status === "Active" && portForward.forwardPort) ? true : false; + this.isPortForwarded = true; + this.isActive = portForward.status === "Active"; } @action @@ -100,11 +104,14 @@ export class PodContainerPort extends React.Component { // determine how many port-forwards already exist const { length } = getPortForwards(); - portForward = await addPortForward(portForward); + if (!this.isPortForwarded) { + portForward = await addPortForward(portForward); + } else if (!this.isActive) { + portForward = await startPortForward(portForward); + } if (portForward.status === "Active") { openPortForward(portForward); - this.isPortForwarded = true; // if this is the first port-forward show the about notification if (!length) { @@ -112,12 +119,11 @@ export class PodContainerPort extends React.Component { } } else { Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`); - this.isPortForwarded = false; } } catch (error) { logger.error("[POD-CONTAINER-PORT]:", error, portForward); - this.checkExistingPortForwarding(); } finally { + this.checkExistingPortForwarding(); this.waiting = false; } } @@ -137,11 +143,11 @@ export class PodContainerPort extends React.Component { try { await removePortForward(portForward); - this.isPortForwarded = false; } catch (error) { Notifications.error(`Error occurred stopping the port-forward from port ${portForward.forwardPort}.`); - this.checkExistingPortForwarding(); } finally { + this.checkExistingPortForwarding(); + this.forwardPort = 0; this.waiting = false; } } @@ -151,7 +157,7 @@ export class PodContainerPort extends React.Component { const { name, containerPort, protocol } = port; const text = `${name ? `${name}: ` : ""}${containerPort}/${protocol}`; - const portForwardAction = async () => { + const portForwardAction = action(async () => { if (this.isPortForwarded) { await this.stopPortForward(); } else { @@ -164,16 +170,16 @@ export class PodContainerPort extends React.Component { protocol: predictProtocol(port.name), }; - PortForwardDialog.open(portForward, { openInBrowser: true }); + PortForwardDialog.open(portForward, { openInBrowser: true, onClose: () => this.checkExistingPortForwarding() }); } - }; + }); return (
this.portForward()}> {text} - + {this.waiting && ( )} diff --git a/src/renderer/port-forward/port-forward-dialog.tsx b/src/renderer/port-forward/port-forward-dialog.tsx index 412da77dc5..39a31539ba 100644 --- a/src/renderer/port-forward/port-forward-dialog.tsx +++ b/src/renderer/port-forward/port-forward-dialog.tsx @@ -34,12 +34,14 @@ import type { ForwardedPort } from "./port-forward-item"; import { aboutPortForwarding, openPortForward } from "."; import { Checkbox } from "../components/checkbox"; import logger from "../../common/logger"; +import { noop } from "lodash"; interface Props extends Partial { } interface PortForwardDialogOpenOptions { - openInBrowser: boolean + openInBrowser: boolean; + onClose: () => void; } const dialogState = observable.object({ @@ -47,6 +49,7 @@ const dialogState = observable.object({ data: null as ForwardedPort, useHttps: false, openInBrowser: false, + onClose: noop, }); @observer @@ -59,11 +62,12 @@ export class PortForwardDialog extends Component { makeObservable(this); } - static open(portForward: ForwardedPort, options: PortForwardDialogOpenOptions = { openInBrowser: false }) { + static open(portForward: ForwardedPort, options: PortForwardDialogOpenOptions = { openInBrowser: false, onClose: noop }) { dialogState.isOpen = true; dialogState.data = portForward; dialogState.useHttps = portForward.protocol === "https"; dialogState.openInBrowser = options.openInBrowser; + dialogState.onClose = options.onClose; } static close() { @@ -85,7 +89,8 @@ export class PortForwardDialog extends Component { this.desiredPort = this.currentPort; }; - onClose = () => { + onClose = async () => { + await (async () => dialogState.onClose())(); }; changePort = (value: string) => { @@ -102,15 +107,25 @@ export class PortForwardDialog extends Component { portForward.protocol = dialogState.useHttps ? "https" : "http"; - if (currentPort || portForward.status === "Disabled") { + if (currentPort) { + const wasRunning = portForward.status === "Active"; + portForward = await modifyPortForward(portForward, desiredPort); + + if (wasRunning && portForward.status === "Disabled") { + Notifications.error(`Error occurred starting port-forward, the local port ${portForward.forwardPort} may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`); + } } else { portForward.forwardPort = desiredPort; portForward = await addPortForward(portForward); - // if this is the first port-forward show the about notification - if (!length) { - aboutPortForwarding(); + if (portForward.status === "Disabled") { + Notifications.error(`Error occurred starting port-forward, the local port ${portForward.forwardPort} may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`); + } else { + // if this is the first port-forward show the about notification + if (!length) { + aboutPortForwarding(); + } } } @@ -118,8 +133,7 @@ export class PortForwardDialog extends Component { openPortForward(portForward); } } catch (error) { - Notifications.error(`Error occurred starting port-forward, the local port may not be available or the ${portForward.kind} ${portForward.name} may not be reachable`); - logger.error("[PORT-FORWARD-DIALOG]:", error, portForward); + logger.error(`[PORT-FORWARD-DIALOG]: ${error}`, portForward); } finally { close(); } diff --git a/src/renderer/port-forward/port-forward-item.ts b/src/renderer/port-forward/port-forward-item.ts index 7fbc311be1..90a9c7c218 100644 --- a/src/renderer/port-forward/port-forward-item.ts +++ b/src/renderer/port-forward/port-forward-item.ts @@ -64,7 +64,7 @@ export class PortForwardItem implements ItemObject { } getId() { - return `${this.namespace}/${this.kind}/${this.name}:${this.port}`; + return `${this.namespace}-${this.kind}-${this.name}:${this.port}`; } getKind() {