From 103b3dd4050aec6d2f9e0923b1459af638c9dc38 Mon Sep 17 00:00:00 2001 From: Jim Ehrismann Date: Tue, 21 Dec 2021 17:41:17 -0500 Subject: [PATCH] further refactoring and bug fixing Signed-off-by: Jim Ehrismann --- .../port-forward-menu.tsx | 14 +++- .../port-forward/port-forward-dialog.tsx | 14 ++-- .../port-forward/port-forward-item.ts | 9 +-- .../port-forward/port-forward.store.ts | 79 ++++++------------- 4 files changed, 45 insertions(+), 71 deletions(-) diff --git a/src/renderer/components/+network-port-forwards/port-forward-menu.tsx b/src/renderer/components/+network-port-forwards/port-forward-menu.tsx index 1a0e7bcc4e..ce2fa65416 100644 --- a/src/renderer/components/+network-port-forwards/port-forward-menu.tsx +++ b/src/renderer/components/+network-port-forwards/port-forward-menu.tsx @@ -45,6 +45,18 @@ export class PortForwardMenu extends React.Component { } } + private startPortForwarding = async () => { + const { portForward } = this.props; + + const pf = await startPortForward(portForward); + + if (pf.status === "Disabled") { + const { name, kind, forwardPort } = portForward; + + Notifications.error(`Error occurred starting port-forward, the local port ${forwardPort} may not be available or the ${kind} ${name} may not be reachable`); + } + }; + renderStartStopMenuItem() { const { portForward, toolbar } = this.props; @@ -58,7 +70,7 @@ export class PortForwardMenu extends React.Component { } return ( - startPortForward(portForward)}> + Start diff --git a/src/renderer/port-forward/port-forward-dialog.tsx b/src/renderer/port-forward/port-forward-dialog.tsx index b8fa901175..412da77dc5 100644 --- a/src/renderer/port-forward/port-forward-dialog.tsx +++ b/src/renderer/port-forward/port-forward-dialog.tsx @@ -101,9 +101,8 @@ export class PortForwardDialog extends Component { const { length } = getPortForwards(); portForward.protocol = dialogState.useHttps ? "https" : "http"; - portForward.status = "Active"; - if (currentPort) { + if (currentPort || portForward.status === "Disabled") { portForward = await modifyPortForward(portForward, desiredPort); } else { portForward.forwardPort = desiredPort; @@ -115,14 +114,11 @@ export class PortForwardDialog extends Component { } } - if (portForward.status === "Active") { - if (dialogState.openInBrowser) { - openPortForward(portForward); - } - } 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`); + if (portForward.status === "Active" && dialogState.openInBrowser) { + 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); } finally { close(); @@ -187,7 +183,7 @@ export class PortForwardDialog extends Component { {this.renderContents()} diff --git a/src/renderer/port-forward/port-forward-item.ts b/src/renderer/port-forward/port-forward-item.ts index ab69dc2494..7fbc311be1 100644 --- a/src/renderer/port-forward/port-forward-item.ts +++ b/src/renderer/port-forward/port-forward-item.ts @@ -63,12 +63,8 @@ export class PortForwardItem implements ItemObject { return this.namespace; } - get id() { - return this.forwardPort; - } - getId() { - return String(this.forwardPort); + return `${this.namespace}/${this.kind}/${this.name}:${this.port}`; } getKind() { @@ -94,10 +90,11 @@ export class PortForwardItem implements ItemObject { getSearchFields() { return [ this.name, - this.id, + this.namespace, this.kind, this.port, this.forwardPort, + this.status, ]; } } diff --git a/src/renderer/port-forward/port-forward.store.ts b/src/renderer/port-forward/port-forward.store.ts index 4e62fdb6a7..cf88ed9a4e 100644 --- a/src/renderer/port-forward/port-forward.store.ts +++ b/src/renderer/port-forward/port-forward.store.ts @@ -23,7 +23,7 @@ import { action, makeObservable, observable, reaction } from "mobx"; import { ItemStore } from "../../common/item.store"; import { autoBind, createStorage, disposer } from "../utils"; -import { ForwardedPort, ForwardedPortStatus, PortForwardItem } from "./port-forward-item"; +import { ForwardedPort, PortForwardItem } from "./port-forward-item"; import { apiBase } from "../api"; import { waitUntilFree } from "tcp-port-used"; import logger from "../../common/logger"; @@ -68,8 +68,6 @@ export class PortForwardStore extends ItemStore { } loadAll() { - console.log("portForwardStore.loadAll()"); - return this.loadItems(() => { const portForwards = getPortForwards(); @@ -153,17 +151,13 @@ export const startPortForward = action( async (portForward: ForwardedPort): Prom pf.status = "Active"; } catch (error) { - logger.warn("[PORT-FORWARD-STORE] Error starting port-forward:", error, pf); + logger.warn(`[PORT-FORWARD-STORE] Error starting port-forward: ${error}`, pf); pf.status = "Disabled"; } setPortForward(pf); - portForward.forwardPort = pf.forwardPort; - portForward.protocol = pf.protocol; - portForward.status = pf.status; - - return portForward; + return pf as ForwardedPort; }); export const addPortForward = action(async (portForward: ForwardedPort): Promise => { @@ -193,7 +187,7 @@ export async function getActivePortForward(portForward: ForwardedPort): Promise< try { response = await apiBase.get(`/pods/port-forward/${portForward.namespace}/${portForward.kind}/${portForward.name}`, { query: { port, forwardPort, protocol }}); } catch (error) { - logger.warn("[PORT-FORWARD-STORE] Error getting active port-forward:", error, portForward); + logger.warn(`[PORT-FORWARD-STORE] Error getting active port-forward: ${error}`, portForward); } portForward.status = response?.port ? "Active" : "Disabled"; @@ -223,32 +217,30 @@ export async function getPortForward(portForward: ForwardedPort): Promise { - +export const modifyPortForward = action(async (portForward: ForwardedPort, desiredPort: number): Promise => { const pf = findPortForward(portForward); - + if (!pf) { throw new Error("port-forward not found"); } - try { - await getActivePortForward(portForward); + if (pf.status === "Active") { + try { + await stopPortForward(pf); + } catch { + // ignore, assume it is stopped and proceed to restart it + } + + pf.forwardPort = desiredPort; + setPortForward(pf); - } catch (error) { - // port is not active - return "Disabled"; + return await startPortForward(pf); } - return pf.status; -} - -export const modifyPortForward = action(async (portForward: ForwardedPort, desiredPort: number): Promise => { - - await removePortForward(portForward); - portForward.forwardPort = desiredPort; - - return await addPortForward(portForward); + pf.forwardPort = desiredPort; + setPortForward(pf); + return pf as ForwardedPort; }); @@ -256,9 +248,7 @@ export const stopPortForward = action(async (portForward: ForwardedPort) => { const pf = findPortForward(portForward); if (!pf) { - const error = new Error("port-forward not found"); - - logger.warn("[PORT-FORWARD-STORE] Error getting port-forward:", error, portForward); + logger.warn("[PORT-FORWARD-STORE] Error getting port-forward: port-forward not found", portForward); return; } @@ -269,7 +259,7 @@ export const stopPortForward = action(async (portForward: ForwardedPort) => { await apiBase.del(`/pods/port-forward/${portForward.namespace}/${portForward.kind}/${portForward.name}`, { query: { port, forwardPort }}); await waitUntilFree(+forwardPort, 200, 1000); } catch (error) { - logger.warn("[PORT-FORWARD-STORE] Error removing active port-forward:", error, portForward); + logger.warn(`[PORT-FORWARD-STORE] Error stopping active port-forward: ${error}`, portForward); throw (error); } @@ -283,7 +273,7 @@ export const removePortForward = action(async (portForward: ForwardedPort) => { if (!pf) { const error = new Error("port-forward not found"); - logger.warn("[PORT-FORWARD-STORE] Error getting port-forward:", error, portForward); + logger.warn(`[PORT-FORWARD-STORE] Error getting port-forward: ${error}`, portForward); return; } @@ -292,16 +282,14 @@ export const removePortForward = action(async (portForward: ForwardedPort) => { await stopPortForward(portForward); } catch (error) { if (pf.status === "Active") { - logger.warn("[PORT-FORWARD-STORE] Error removing port-forward:", error, portForward); + logger.warn(`[PORT-FORWARD-STORE] Error removing port-forward: ${error}`, portForward); } } const index = portForwardStore.portForwards.findIndex(portForwardsEqual(portForward)); if (index >= 0 ) { - console.log("removePortForward splice", index, portForwardStore.portForwards[index]); portForwardStore.portForwards.splice(index, 1); - console.log("portForwards after splice", portForwardStore.portForwards); } }); @@ -311,7 +299,7 @@ export async function getActivePortForwards(clusterId?: string): Promise => { - try { - // get the active port-forwards to update the status - const activePortForwards = await getActivePortForwards(clusterId); - - portForwardStore.portForwards.map((item, index) => { - if (activePortForwards.find(portForwardsEqual(item)) && portForwardStore.portForwards[index].status !== "Active") { - portForwardStore.portForwards[index].status = "Active"; - } else if (portForwardStore.portForwards[index].status !== "Disabled") { - portForwardStore.portForwards[index].status = "Disabled"; - } - }); - } catch (error) { - logger.warn("[PORT-FORWARD-STORE] Error getting all active port-forwards:", error); - } - - return portForwardStore.portForwards; -}); - export const portForwardStore = new PortForwardStore();