Fix race condition in refreshViews (#4094)

src/main/catalog/catalog-entity-registry.ts

@@ -20,7 +20,7 @@
  */
 
 import { action, computed, IComputedValue, IObservableArray, makeObservable, observable } from "mobx";
-import { CatalogCategoryRegistry, catalogCategoryRegistry, CatalogEntity, CatalogEntityKindData } from "../../common/catalog";
+import { CatalogCategoryRegistry, catalogCategoryRegistry, CatalogEntity, CatalogEntityConstructor, CatalogEntityKindData } from "../../common/catalog";
 import { iter } from "../../common/utils";
 
 export class CatalogEntityRegistry {
@@ -59,7 +59,7 @@ export class CatalogEntityRegistry {
     return this.items.filter((item) => item.apiVersion === apiVersion && item.kind === kind) as T[];
   }
 
-  getItemsByEntityClass<T extends CatalogEntity>({ apiVersion, kind }: CatalogEntityKindData): T[] {
+  getItemsByEntityClass<T extends CatalogEntity>({ apiVersion, kind }: CatalogEntityKindData & CatalogEntityConstructor<T>): T[] {
     return this.getItemsForApiKind(apiVersion, kind);
   }
 }

src/main/cluster-manager.ts

@@ -41,6 +41,8 @@ export class ClusterManager extends Singleton {
   private store = ClusterStore.getInstance();
   deleting = observable.set<ClusterId>();
 
+  @observable visibleCluster: ClusterId | undefined = undefined;
+
   constructor() {
     super();
     makeObservable(this);
@@ -61,8 +63,22 @@ export class ClusterManager extends Singleton {
       { fireImmediately: false },
     );
 
-    reaction(() => catalogEntityRegistry.getItemsForApiKind<KubernetesCluster>("entity.k8slens.dev/v1alpha1", "KubernetesCluster"), (entities) => {
-      this.syncClustersFromCatalog(entities);
+    reaction(
+      () => catalogEntityRegistry.getItemsByEntityClass(KubernetesCluster),
+      entities => this.syncClustersFromCatalog(entities),
+    );
+
+    reaction(() => [
+      catalogEntityRegistry.getItemsByEntityClass(KubernetesCluster),
+      this.visibleCluster,
+    ] as const, ([entities, visibleCluster]) => {
+      for (const entity of entities) {
+        if (entity.getId() === visibleCluster) {
+          entity.status.active = true;
+        } else {
+          entity.status.active = false;
+        }
+      }
     });
 
     observe(this.deleting, change => {

src/main/initializers/ipc.ts

@@ -20,13 +20,12 @@
  */
 
 import { BrowserWindow, dialog, IpcMainInvokeEvent } from "electron";
-import { KubernetesCluster } from "../../common/catalog-entities";
 import { clusterFrameMap } from "../../common/cluster-frames";
 import { clusterActivateHandler, clusterSetFrameIdHandler, clusterVisibilityHandler, clusterRefreshHandler, clusterDisconnectHandler, clusterKubectlApplyAllHandler, clusterKubectlDeleteAllHandler, clusterDeleteHandler, clusterSetDeletingHandler, clusterClearDeletingHandler } from "../../common/cluster-ipc";
 import type { ClusterId } from "../../common/cluster-types";
 import { ClusterStore } from "../../common/cluster-store";
 import { appEventBus } from "../../common/event-bus";
-import { dialogShowOpenDialogHandler, ipcMainHandle } from "../../common/ipc";
+import { dialogShowOpenDialogHandler, ipcMainHandle, ipcMainOn } from "../../common/ipc";
 import { catalogEntityRegistry } from "../catalog";
 import { pushCatalogToRenderer } from "../catalog-pusher";
 import { ClusterManager } from "../cluster-manager";
@@ -54,16 +53,8 @@ export function initIpcMainHandlers() {
     }
   });
 
-  ipcMainHandle(clusterVisibilityHandler, (event: IpcMainInvokeEvent, clusterId: ClusterId, visible: boolean) => {
-    const entity = catalogEntityRegistry.getById(clusterId);
-
-    for (const kubeEntity of catalogEntityRegistry.getItemsByEntityClass(KubernetesCluster)) {
-      kubeEntity.status.active = false;
-    }
-
-    if (entity) {
-      entity.status.active = visible;
-    }
+  ipcMainOn(clusterVisibilityHandler, (event, clusterId?: ClusterId) => {
+    ClusterManager.getInstance().visibleCluster = clusterId;
   });
 
   ipcMainHandle(clusterRefreshHandler, (event, clusterId: ClusterId) => {

src/renderer/components/cluster-manager/cluster-view.tsx

@@ -25,7 +25,7 @@ import { computed, makeObservable, reaction } from "mobx";
 import { disposeOnUnmount, observer } from "mobx-react";
 import type { RouteComponentProps } from "react-router";
 import { ClusterStatus } from "./cluster-status";
-import { hasLoadedView, initView, refreshViews } from "./lens-views";
+import { ClusterFrameHandler } from "./lens-views";
 import type { Cluster } from "../../../main/cluster";
 import { ClusterStore } from "../../../common/cluster-store";
 import { requestMain } from "../../../common/ipc";
@@ -57,7 +57,7 @@ export class ClusterView extends React.Component<Props> {
   @computed get isReady(): boolean {
     const { cluster, clusterId } = this;
 
-    return cluster?.ready && cluster?.available && hasLoadedView(clusterId);
+    return cluster?.ready && cluster?.available && ClusterFrameHandler.getInstance().hasLoadedView(clusterId);
   }
 
   componentDidMount() {
@@ -65,25 +65,23 @@ export class ClusterView extends React.Component<Props> {
   }
 
   componentWillUnmount() {
-    refreshViews();
+    ClusterFrameHandler.getInstance().clearVisibleCluster();
     catalogEntityRegistry.activeEntity = null;
   }
 
   bindEvents() {
     disposeOnUnmount(this, [
       reaction(() => this.clusterId, async (clusterId) => {
-        refreshViews(clusterId); // refresh visibility of active cluster
-        initView(clusterId); // init cluster-view (iframe), requires parent container #lens-views to be in DOM
+        ClusterFrameHandler.getInstance().setVisibleCluster(clusterId);
+        ClusterFrameHandler.getInstance().initView(clusterId);
         requestMain(clusterActivateHandler, clusterId, false); // activate and fetch cluster's state from main
-        catalogEntityRegistry.activeEntity = catalogEntityRegistry.getById(clusterId);
+        catalogEntityRegistry.activeEntity = clusterId;
       }, {
         fireImmediately: true,
       }),
 
-      reaction(() => [this.cluster?.ready, this.cluster?.disconnected], (values) => {
-        const disconnected = values[1];
-
-        if (hasLoadedView(this.clusterId) && disconnected) {
+      reaction(() => [this.cluster?.ready, this.cluster?.disconnected], ([, disconnected]) => {
+        if (ClusterFrameHandler.getInstance().hasLoadedView(this.clusterId) && disconnected) {
           navigate(catalogURL()); // redirect to catalog when active cluster get disconnected/not available
         }
       }),

src/renderer/components/cluster-manager/lens-views.ts

@@ -19,82 +19,117 @@
  * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
  */
 
-import { observable, when } from "mobx";
+import { action, IReactionDisposer, makeObservable, observable, reaction, when } from "mobx";
 import logger from "../../../main/logger";
-import { requestMain } from "../../../common/ipc";
 import { clusterVisibilityHandler } from "../../../common/cluster-ipc";
 import { ClusterStore } from "../../../common/cluster-store";
 import type { ClusterId } from "../../../common/cluster-types";
-import { getClusterFrameUrl } from "../../utils";
+import { getClusterFrameUrl, Singleton } from "../../utils";
+import { ipcRenderer } from "electron";
 
 export interface LensView {
-  isLoaded?: boolean
-  clusterId: ClusterId;
-  view: HTMLIFrameElement
+  isLoaded: boolean;
+  frame: HTMLIFrameElement;
 }
 
-export const lensViews = observable.map<ClusterId, LensView>();
+export class ClusterFrameHandler extends Singleton {
+  private views = observable.map<string, LensView>();
+  @observable private visibleCluster: string | null = null;
 
-export function hasLoadedView(clusterId: ClusterId): boolean {
-  return !!lensViews.get(clusterId)?.isLoaded;
-}
-
-export async function initView(clusterId: ClusterId) {
-  const cluster = ClusterStore.getInstance().getById(clusterId);
-
-  if (!cluster || lensViews.has(clusterId)) {
-    return;
+  constructor() {
+    super();
+    makeObservable(this);
+    reaction(() => this.visibleCluster, this.handleVisibleClusterChange);
   }
 
-  logger.info(`[LENS-VIEW]: init dashboard, clusterId=${clusterId}`);
-  const parentElem = document.getElementById("lens-views");
-  const iframe = document.createElement("iframe");
-
-  iframe.id = `cluster-frame-${cluster.id}`;
-  iframe.name = cluster.contextName;
-  iframe.setAttribute("src", getClusterFrameUrl(clusterId));
-  iframe.addEventListener("load", () => {
-    logger.info(`[LENS-VIEW]: loaded from ${iframe.src}`);
-    lensViews.get(clusterId).isLoaded = true;
-  }, { once: true });
-  lensViews.set(clusterId, { clusterId, view: iframe });
-  parentElem.appendChild(iframe);
-
-  logger.info(`[LENS-VIEW]: waiting cluster to be ready, clusterId=${clusterId}`);
-
-  try {
-    await when(() => cluster.ready, { timeout: 5_000 }); // we cannot wait forever because cleanup would be blocked for broken cluster connections
-    logger.info(`[LENS-VIEW]: cluster is ready, clusterId=${clusterId}`);
-  } finally {
-    await autoCleanOnRemove(clusterId, iframe);
+  public hasLoadedView(clusterId: string): boolean {
+    return Boolean(this.views.get(clusterId)?.isLoaded);
   }
-}
 
-export async function autoCleanOnRemove(clusterId: ClusterId, iframe: HTMLIFrameElement) {
-  await when(() => {
+  @action
+  public initView(clusterId: ClusterId) {
     const cluster = ClusterStore.getInstance().getById(clusterId);
 
-    return !cluster || (cluster.disconnected && lensViews.get(clusterId)?.isLoaded);
-  });
-  logger.info(`[LENS-VIEW]: remove dashboard, clusterId=${clusterId}`);
-  lensViews.delete(clusterId);
+    if (!cluster || this.views.has(clusterId)) {
+      return;
+    }
 
-  iframe.parentNode.removeChild(iframe);
-}
-
-export function refreshViews(visibleClusterId?: string) {
-  logger.info(`[LENS-VIEW]: refreshing iframe views, visible cluster id=${visibleClusterId}`);
-  const cluster = ClusterStore.getInstance().getById(visibleClusterId);
-
-  lensViews.forEach(({ clusterId, view, isLoaded }) => {
-    const isCurrent = clusterId === cluster?.id;
-    const isReady = cluster?.available && cluster?.ready;
-    const isVisible = isCurrent && isLoaded && isReady;
-
-    view.style.display = isVisible ? "flex" : "none";
-
-    requestMain(clusterVisibilityHandler, clusterId, isVisible).catch(() => {
-      logger.error(`[LENS-VIEW]: failed to set cluster visibility, clusterId=${clusterId}`);
-    });
-  });
+    logger.info(`[LENS-VIEW]: init dashboard, clusterId=${clusterId}`);
+    const parentElem = document.getElementById("lens-views");
+    const iframe = document.createElement("iframe");
+
+    iframe.id = `cluster-frame-${cluster.id}`;
+    iframe.name = cluster.contextName;
+    iframe.style.display = "none";
+    iframe.setAttribute("src", getClusterFrameUrl(clusterId));
+    iframe.addEventListener("load", () => {
+      logger.info(`[LENS-VIEW]: loaded from ${iframe.src}`);
+      this.views.get(clusterId).isLoaded = true;
+    }, { once: true });
+    this.views.set(clusterId, { frame: iframe, isLoaded: false });
+    parentElem.appendChild(iframe);
+
+    logger.info(`[LENS-VIEW]: waiting cluster to be ready, clusterId=${clusterId}`);
+
+    const dispose = when(
+      () => cluster.ready,
+      () => logger.info(`[LENS-VIEW]: cluster is ready, clusterId=${clusterId}`),
+    );
+
+    when(
+      // cluster.disconnect is set to `false` when the cluster starts to connect
+      () => !cluster.disconnected,
+      () => {
+        when(
+          () => {
+            const cluster = ClusterStore.getInstance().getById(clusterId);
+
+            return !cluster || (cluster.disconnected && this.views.get(clusterId)?.isLoaded);
+          },
+          () => {
+            logger.info(`[LENS-VIEW]: remove dashboard, clusterId=${clusterId}`);
+            this.views.delete(clusterId);
+            iframe.parentNode.removeChild(iframe);
+            dispose();
+          },
+        );
+      },
+    );
+  }
+
+  public setVisibleCluster(clusterId: ClusterId) {
+    this.visibleCluster = clusterId;
+  }
+
+  public clearVisibleCluster() {
+    this.visibleCluster = null;
+  }
+
+  private prevVisibleClusterChange?: IReactionDisposer;
+
+  private handleVisibleClusterChange = (clusterId: ClusterId | undefined) => {
+    logger.info(`[LENS-VIEW]: refreshing iframe views, visible cluster id=${clusterId}`);
+
+    ipcRenderer.send(clusterVisibilityHandler);
+
+    const cluster = ClusterStore.getInstance().getById(clusterId);
+
+    for (const { frame: view } of this.views.values()) {
+      view.style.display = "none";
+    }
+
+    if (cluster) {
+      const lensView = this.views.get(clusterId);
+
+      this.prevVisibleClusterChange?.();
+      this.prevVisibleClusterChange = when(
+        () => cluster.available && cluster.ready && lensView.isLoaded,
+        () => {
+          logger.info(`[LENS-VIEW]: cluster id=${clusterId} should now be visible`);
+          lensView.frame.style.display = "flex";
+          ipcRenderer.send(clusterVisibilityHandler, clusterId);
+        },
+      );
+    }
+  };
 }

src/renderer/lens-app.tsx

@@ -38,6 +38,7 @@ import { IpcRendererNavigationEvents } from "./navigation/events";
 import { catalogEntityRegistry } from "./api/catalog-entity-registry";
 import logger from "../common/logger";
 import { unmountComponentAtNode } from "react-dom";
+import { ClusterFrameHandler } from "./components/cluster-manager/lens-views";
 
 injectSystemCAs();
 
@@ -61,6 +62,12 @@ export class LensApp extends React.Component {
     };
   }
 
+  constructor(props: {}) {
+    super(props);
+
+    ClusterFrameHandler.createInstance();
+  }
+
   componentDidMount() {
     ipcRenderer.send(IpcRendererNavigationEvents.LOADED);
   }