Release hotfixes

More resilient workbench boot, and fix cluster switcher bug

Author: michaeljguarino

assets/src/components/cd/utils/ClusterSelector.tsx

@@ -21,6 +21,7 @@ import { ComponentPropsWithRef, useCallback, useMemo, useState } from 'react'
 import { useTheme } from 'styled-components'
 
 import { useProjectId } from '../../contexts/ProjectsContext'
+import { withCurrentCluster } from '../../kubernetes/clusterSelection'
 import { useFetchPaginatedData } from '../../utils/table/useFetchPaginatedData'
 import { ClusterUpgradeChip } from '../clusters/ClusterUpgradeButton'
 
@@ -66,8 +67,12 @@ export default function ClusterSelector({
   )
 
   const clusters = useMemo(
-    () => data?.clusters?.edges?.flatMap((e) => (e?.node ? e.node : [])) || [],
-    [data?.clusters?.edges]
+    () =>
+      withCurrentCluster(
+        data?.clusters?.edges?.flatMap((e) => (e?.node ? e.node : [])) || [],
+        data?.cluster
+      ),
+    [data?.cluster, data?.clusters?.edges]
   )
 
   const findCluster = useCallback(

assets/src/components/kubernetes/Cluster.tsx

@@ -1,6 +1,12 @@
 import { EmptyState } from '@pluralsh/design-system'
 import { isEmpty } from 'lodash'
-import { createContext, useContext, useEffect, useMemo } from 'react'
+import {
+  createContext,
+  useContext,
+  useEffect,
+  useLayoutEffect,
+  useMemo,
+} from 'react'
 import {
   Navigate,
   Outlet,
@@ -29,11 +35,14 @@ import { useProjectId } from '../contexts/ProjectsContext'
 import { GqlError } from '../utils/Alert'
 import LoadingIndicator from '../utils/LoadingIndicator'
 import { useSimpleToast } from '../utils/SimpleToastContext'
+import {
+  getDefaultKubernetesClusterId,
+  isKubernetesClusterMissing,
+  LAST_SELECTED_CLUSTER_KEY,
+} from './clusterSelection'
 import { DataSelectProvider } from './common/DataSelect'
 import { getNamespaceListLoadError } from './common/namespaceList'
 
-import { LAST_SELECTED_CLUSTER_KEY } from './Navigation'
-
 type ClusterContextT = {
   clusters: KubernetesClusterFragment[]
   refetch?: Nullable<() => void>
@@ -111,30 +120,39 @@ export default function Cluster({
   const { search } = useLocation()
   const navigate = useNavigate()
 
-  const { data, error, refetch, loading } = useKubernetesClustersQuery({
-    pollInterval: 60_000,
-    fetchPolicy: 'cache-and-network',
-    variables: {
-      currentClusterId: clusterId,
-      hasCurrentClusterId: !!clusterId,
-      projectId,
-    },
-  })
+  const { data, previousData, error, refetch, loading } =
+    useKubernetesClustersQuery({
+      pollInterval: 60_000,
+      fetchPolicy: 'cache-and-network',
+      variables: {
+        currentClusterId: clusterId,
+        hasCurrentClusterId: !!clusterId,
+        projectId,
+      },
+    })
 
+  // Variable changes miss the cache, so `data` is empty while the new cluster
+  // loads. Keep the previous result on screen so the dashboard doesn't unmount.
+  const queryData = data ?? previousData
   const clusters = useMemo(
-    () => mapExistingNodes(data?.clusters),
-    [data?.clusters]
+    () => mapExistingNodes(queryData?.clusters),
+    [queryData?.clusters]
   )
-  const currentCluster = data?.cluster
-
-  const hasCurrentClusterId =
-    currentCluster?.id === clusterId ||
-    clusters.some(({ id }) => id === clusterId)
-
-  const cluster =
-    currentCluster?.id === clusterId
-      ? currentCluster
-      : clusters.find(({ id }) => id === clusterId)
+  const currentCluster = [data?.cluster, queryData?.cluster].find(
+    (candidate) => candidate?.id === clusterId
+  )
+  const cluster = currentCluster ?? clusters.find(({ id }) => id === clusterId)
+  // Don't unmount the dashboard while the new cluster(id:) result is in flight.
+  const clusterForContext =
+    cluster ?? (loading ? queryData?.cluster : undefined)
+
+  const clusterMissing = isKubernetesClusterMissing({
+    clusterId,
+    loading,
+    hasData: !!data,
+    currentClusterId: data?.cluster?.id,
+    clusterIds: mapExistingNodes(data?.clusters).map(({ id }) => id),
+  })
 
   const namespaceQueryOptions = getNamespacesOptions({
     client: AxiosInstance(clusterId!),
@@ -184,39 +202,48 @@ export default function Cluster({
   }, [clusterId, namespaceListError, popToast])
 
   const context = useMemo(
-    () => ({ clusters, refetch, cluster, namespaces }) as ClusterContextT,
-    [clusters, refetch, cluster, namespaces]
+    () =>
+      ({
+        clusters,
+        refetch,
+        cluster: clusterForContext,
+        namespaces,
+      }) as ClusterContextT,
+    [clusters, refetch, clusterForContext, namespaces]
   )
 
-  const defaultClusterId = useMemo(() => {
-    if (isEmpty(clusters)) return undefined
-
-    const lastSelectedClusterId = sessionStorage.getItem(
-      LAST_SELECTED_CLUSTER_KEY
-    )
-    const lastSelectedClusterExists = clusters.some(
-      ({ id }) => id === lastSelectedClusterId
-    )
-    const mgmtCluster = clusters.find(({ self }) => !!self)
+  const defaultClusterId = useMemo(
+    () =>
+      getDefaultKubernetesClusterId(
+        clusters,
+        sessionStorage.getItem(LAST_SELECTED_CLUSTER_KEY)
+      ),
+    [clusters]
+  )
 
-    return lastSelectedClusterExists
-      ? lastSelectedClusterId
-      : mgmtCluster
-        ? mgmtCluster?.id
-        : clusters[0].id
-  }, [clusters])
+  useLayoutEffect(() => {
+    if (cluster && clusterId && cluster.id === clusterId) {
+      sessionStorage.setItem(LAST_SELECTED_CLUSTER_KEY, cluster.id)
+    }
+  }, [cluster, clusterId])
 
   useEffect(() => {
-    if (clusterId && defaultClusterId && !hasCurrentClusterId) {
-      navigate(`${defaultClusterId}${search}`, {
-        replace: true,
-      })
-    }
-  }, [defaultClusterId, navigate, search, clusterId, hasCurrentClusterId])
+    if (!clusterMissing || !defaultClusterId) return
+
+    navigate(`${getDefaultClusterPath(defaultClusterId)}${search}`, {
+      replace: true,
+    })
+  }, [
+    clusterMissing,
+    defaultClusterId,
+    getDefaultClusterPath,
+    navigate,
+    search,
+  ])
 
   useEffect(() => {
     refetchNamespaces()
-  }, [refetchNamespaces, cluster])
+  }, [refetchNamespaces, clusterId])
 
   if (error)
     return (
@@ -228,7 +255,7 @@ export default function Cluster({
       </div>
     )
 
-  if (loading && !data) return <LoadingIndicator />
+  if (!queryData) return <LoadingIndicator />
 
   if (!clusterId && defaultClusterId)
     return (
@@ -238,7 +265,7 @@ export default function Cluster({
       />
     )
 
-  if (!cluster) return <EmptyState message="No clusters found." />
+  if (!cluster && !loading) return <EmptyState message="No clusters found." />
 
   return (
     <ClusterContext value={context}>

assets/src/components/kubernetes/Navigation.tsx

@@ -1,4 +1,4 @@
-import { ReactNode, useLayoutEffect, useMemo, useState } from 'react'
+import { ReactNode, useMemo, useState } from 'react'
 import { Outlet, useLocation, useNavigate, useParams } from 'react-router-dom'
 import { useTheme } from 'styled-components'
 
@@ -10,6 +10,7 @@ import {
   getKubernetesAbsPath,
   NETWORK_REL_PATH,
   RBAC_REL_PATH,
+  replaceKubernetesClusterId,
   STORAGE_REL_PATH,
   WORKLOADS_REL_PATH,
 } from '../../routes/kubernetesRoutesConsts'
@@ -24,7 +25,7 @@ import { DataSelectInputs } from './common/DataSelect'
 
 export const NAMESPACE_PARAM = 'namespace'
 export const FILTER_PARAM = 'filter'
-export const LAST_SELECTED_CLUSTER_KEY = 'plural-last-selected-cluster'
+export { LAST_SELECTED_CLUSTER_KEY } from './clusterSelection'
 
 const directory: Directory = [
   { path: WORKLOADS_REL_PATH, label: 'Workloads' },
@@ -51,10 +52,6 @@ export default function Navigation() {
     []
   )
 
-  useLayoutEffect(() => {
-    if (clusterId) sessionStorage.setItem(LAST_SELECTED_CLUSTER_KEY, clusterId)
-  }, [pathname, clusterId])
-
   return (
     <ResponsiveLayoutPage>
       <ResponsiveLayoutSidenavContainer>
@@ -72,7 +69,11 @@ export default function Navigation() {
             allowDeselect={false}
             hideTitleContent
             onClusterChange={(cluster) => {
-              if (cluster?.id) navigate(pathname.replace(clusterId, cluster.id))
+              if (!cluster?.id || cluster.id === clusterId) return
+
+              navigate(
+                replaceKubernetesClusterId(pathname, clusterId, cluster.id)
+              )
             }}
           />
           <SideNavEntries

assets/src/components/kubernetes/clusterSelection.test.ts

@@ -0,0 +1,122 @@
+import { describe, expect, it } from 'vitest'
+import { replaceKubernetesClusterId } from '../../routes/kubernetesRoutesConsts'
+import {
+  getDefaultKubernetesClusterId,
+  isKubernetesClusterMissing,
+  withCurrentCluster,
+} from './clusterSelection'
+
+describe('replaceKubernetesClusterId', () => {
+  it('swaps only the kubernetes cluster path segment', () => {
+    expect(
+      replaceKubernetesClusterId(
+        '/kubernetes/cluster-a/workloads/deployments',
+        'cluster-a',
+        'cluster-b'
+      )
+    ).toBe('/kubernetes/cluster-b/workloads/deployments')
+  })
+
+  it('does not use a raw substring replace', () => {
+    expect(
+      replaceKubernetesClusterId(
+        '/kubernetes/cluster-a/workloads/cluster-a',
+        'cluster-a',
+        'cluster-b'
+      )
+    ).toBe('/kubernetes/cluster-b/workloads/cluster-a')
+  })
+
+  it('leaves unrelated paths unchanged', () => {
+    expect(
+      replaceKubernetesClusterId(
+        '/cd/clusters/cluster-a',
+        'cluster-a',
+        'cluster-b'
+      )
+    ).toBe('/cd/clusters/cluster-a')
+  })
+})
+
+describe('withCurrentCluster', () => {
+  it('prepends the current cluster when it is missing from the page', () => {
+    expect(
+      withCurrentCluster([{ id: 'a' }, { id: 'b' }], { id: 'current' })
+    ).toEqual([{ id: 'current' }, { id: 'a' }, { id: 'b' }])
+  })
+
+  it('does not duplicate the current cluster', () => {
+    expect(withCurrentCluster([{ id: 'a' }, { id: 'b' }], { id: 'a' })).toEqual(
+      [{ id: 'a' }, { id: 'b' }]
+    )
+  })
+})
+
+describe('getDefaultKubernetesClusterId', () => {
+  const clusters = [
+    { id: 'worker', self: false },
+    { id: 'mgmt', self: true },
+  ]
+
+  it('prefers the last selected cluster when it still exists', () => {
+    expect(getDefaultKubernetesClusterId(clusters, 'worker')).toBe('worker')
+  })
+
+  it('falls back to the management cluster', () => {
+    expect(getDefaultKubernetesClusterId(clusters, 'gone')).toBe('mgmt')
+  })
+
+  it('returns undefined when there are no clusters', () => {
+    expect(getDefaultKubernetesClusterId([], 'worker')).toBeUndefined()
+  })
+})
+
+describe('isKubernetesClusterMissing', () => {
+  it('does not treat a cluster as missing while the query is in flight', () => {
+    expect(
+      isKubernetesClusterMissing({
+        clusterId: 'b',
+        loading: true,
+        hasData: true,
+        currentClusterId: 'a',
+        clusterIds: ['a'],
+      })
+    ).toBe(false)
+  })
+
+  it('does not redirect when the requested cluster is in the page even if cluster(id:) is stale', () => {
+    expect(
+      isKubernetesClusterMissing({
+        clusterId: 'b',
+        loading: false,
+        hasData: true,
+        currentClusterId: 'a',
+        clusterIds: ['a', 'b'],
+      })
+    ).toBe(false)
+  })
+
+  it('does not redirect when the requested cluster is only on cluster(id:)', () => {
+    expect(
+      isKubernetesClusterMissing({
+        clusterId: 'b',
+        loading: false,
+        hasData: true,
+        currentClusterId: 'b',
+        clusterIds: ['a'],
+      })
+    ).toBe(false)
+  })
+
+  it('flags a settled query whose cluster is neither listed nor returned by id', () => {
+    expect(
+      isKubernetesClusterMissing({
+        clusterId: 'missing',
+        loading: false,
+        hasData: true,
+        currentClusterId: undefined,
+        clusterIds: ['a', 'mgmt'],
+      })
+    ).toBe(true)
+  })
+})