Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 7 additions & 2 deletions assets/src/components/cd/utils/ClusterSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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(
Expand Down
129 changes: 78 additions & 51 deletions assets/src/components/kubernetes/Cluster.tsx
Original file line number Diff line number Diff line change
@@ -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,
Expand Down Expand Up @@ -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>
Expand Down Expand Up @@ -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)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Stale cluster context during switching

When the selected cluster is absent from the previous bounded cluster page and its new query is still loading, this fallback exposes the previous cluster through ClusterContext even though the route already identifies the new cluster. Context consumers then issue resource requests and actions against the old cluster while displaying the new cluster's URL.

Knowledge Base Used:


const clusterMissing = isKubernetesClusterMissing({
clusterId,
loading,
hasData: !!data,
currentClusterId: data?.cluster?.id,
clusterIds: mapExistingNodes(data?.clusters).map(({ id }) => id),
})

const namespaceQueryOptions = getNamespacesOptions({
client: AxiosInstance(clusterId!),
Expand Down Expand Up @@ -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 (
Expand All @@ -228,7 +255,7 @@ export default function Cluster({
</div>
)

if (loading && !data) return <LoadingIndicator />
if (!queryData) return <LoadingIndicator />

if (!clusterId && defaultClusterId)
return (
Expand All @@ -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}>
Expand Down
15 changes: 8 additions & 7 deletions assets/src/components/kubernetes/Navigation.tsx
Original file line number Diff line number Diff line change
@@ -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'

Expand All @@ -10,6 +10,7 @@ import {
getKubernetesAbsPath,
NETWORK_REL_PATH,
RBAC_REL_PATH,
replaceKubernetesClusterId,
STORAGE_REL_PATH,
WORKLOADS_REL_PATH,
} from '../../routes/kubernetesRoutesConsts'
Expand All @@ -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' },
Expand All @@ -51,10 +52,6 @@ export default function Navigation() {
[]
)

useLayoutEffect(() => {
if (clusterId) sessionStorage.setItem(LAST_SELECTED_CLUSTER_KEY, clusterId)
}, [pathname, clusterId])

return (
<ResponsiveLayoutPage>
<ResponsiveLayoutSidenavContainer>
Expand All @@ -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
Expand Down
122 changes: 122 additions & 0 deletions assets/src/components/kubernetes/clusterSelection.test.ts
Original file line number Diff line number Diff line change
@@ -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)
})
})
Loading
Loading