fix: resolve bugs in benchmark lifecycle, snapshot filtering, and dark mode

- Fix PVC bind loop leak on unmount via cancellation ref
- Fix DeleteOptions body structure for proper foreground propagation
- Filter snapshots to tns-csi driver only (was showing all drivers)
- Fix stale closures in Escape key handlers with useCallback
- Add loading state to cleanup delete button, remove window.confirm/alert
- Use CSS custom properties for protocol chart colors (dark mode support)
- Fix all 35 ESLint warnings (import sort, indent, boolean attrs)

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
DevContainer User
2026-03-04 12:47:33 +00:00
parent 6f35c6c81b
commit c1c5e8a37d
19 changed files with 113 additions and 76 deletions
+15 -1
View File
@@ -7,6 +7,19 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
## [Unreleased]
## [0.2.6] - 2026-03-04
### Fixed
- **PVC bind loop leak** — benchmark page's PVC wait loop now checks a cancellation ref on unmount, preventing leaked async work after navigation
- **DeleteOptions body** — `deleteJob` now sends correct Kubernetes `DeleteOptions` with `apiVersion` and `kind` fields so foreground propagation is honored
- **Snapshot filtering** — `TnsCsiDataContext` now filters volume snapshots and snapshot classes to only tns-csi driver ones using `filterTnsCsiVolumeSnapshots` and `isTnsCsiVolumeSnapshotClass` (previously showed all drivers' snapshots)
- **Stale closures in Escape handlers** — `closeSc` / `closeVolume` in StorageClassesPage and VolumesPage wrapped in `useCallback` with proper deps, removing `eslint-disable` suppressions
- **Cleanup button double-click** — benchmark cleanup "Delete Job + PVC" button now has a `cleaningUp` loading state with disabled styling, replacing unsafe `window.confirm`/`window.alert` calls
- **Dark mode protocol colors** — replaced hardcoded hex colors in OverviewPage protocol chart with `var(--mui-palette-*)` tokens
- **Import style consistency** — unified `React.useMemo` to destructured `useMemo` in OverviewPage
- **Lint warnings** — fixed all 35 ESLint warnings (import sorting, indentation, boolean attributes, line length, chained call newlines)
## [0.2.4] - 2026-02-26
### Added
@@ -89,7 +102,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- TypeScript strict mode with zero `any` types
- ESLint + Prettier code quality tooling
[Unreleased]: https://github.com/privilegedescalation/headlamp-tns-csi-plugin/compare/v0.2.4...HEAD
[Unreleased]: https://github.com/privilegedescalation/headlamp-tns-csi-plugin/compare/v0.2.6...HEAD
[0.2.6]: https://github.com/privilegedescalation/headlamp-tns-csi-plugin/compare/v0.2.5...v0.2.6
[0.2.4]: https://github.com/privilegedescalation/headlamp-tns-csi-plugin/compare/v0.2.3...v0.2.4
[0.2.3]: https://github.com/privilegedescalation/headlamp-tns-csi-plugin/compare/v0.2.2...v0.2.3
[0.2.2]: https://github.com/privilegedescalation/headlamp-tns-csi-plugin/compare/v0.2.1...v0.2.2
+10 -3
View File
@@ -13,12 +13,14 @@ import {
filterTnsCsiPersistentVolumes,
filterTnsCsiPVCs,
filterTnsCsiStorageClasses,
filterTnsCsiVolumeSnapshots,
isKubeList,
isTnsCsiVolumeSnapshotClass,
TNS_CSI_PROVISIONER,
TnsCsiPersistentVolume,
TnsCsiPersistentVolumeClaim,
TnsCsiPod,
TnsCsiStorageClass,
TNS_CSI_PROVISIONER,
VolumeSnapshot,
VolumeSnapshotClass,
} from './k8s';
@@ -151,14 +153,19 @@ export function TnsCsiDataProvider({ children }: { children: React.ReactNode })
'/apis/snapshot.storage.k8s.io/v1/volumesnapshotclasses'
);
if (!cancelled && isKubeList(vscList)) {
setVolumeSnapshotClasses(vscList.items as VolumeSnapshotClass[]);
const allSnapshotClasses = vscList.items as VolumeSnapshotClass[];
const tnsCsiSnapshotClasses = allSnapshotClasses.filter(isTnsCsiVolumeSnapshotClass);
setVolumeSnapshotClasses(tnsCsiSnapshotClasses);
setSnapshotCrdAvailable(true);
const tnsCsiClassNames = new Set(tnsCsiSnapshotClasses.map(c => c.metadata.name));
const vsList = await ApiProxy.request(
'/apis/snapshot.storage.k8s.io/v1/volumesnapshots'
);
if (!cancelled && isKubeList(vsList)) {
setVolumeSnapshots(vsList.items as VolumeSnapshot[]);
const allSnapshots = vsList.items as VolumeSnapshot[];
setVolumeSnapshots(filterTnsCsiVolumeSnapshots(allSnapshots, tnsCsiClassNames));
}
}
} catch {
+7 -2
View File
@@ -77,7 +77,8 @@ export const KBENCH_STORAGE_CLASS_ANNOTATION = 'tns-csi.headlamp/storage-class';
// ---------------------------------------------------------------------------
function shortId(): string {
return Math.random().toString(36).slice(2, 8);
return Math.random().toString(36)
.slice(2, 8);
}
export function generateJobName(): string {
@@ -269,7 +270,11 @@ export async function fetchKbenchLogs(jobName: string, namespace: string): Promi
export async function deleteJob(jobName: string, namespace: string): Promise<void> {
await ApiProxy.request(`/apis/batch/v1/namespaces/${namespace}/jobs/${jobName}`, {
method: 'DELETE',
body: JSON.stringify({ propagationPolicy: 'Foreground' }),
body: JSON.stringify({
apiVersion: 'v1',
kind: 'DeleteOptions',
propagationPolicy: 'Foreground',
}),
headers: { 'Content-Type': 'application/json' },
});
}
+4 -4
View File
@@ -1,5 +1,5 @@
import { fireEvent, render, screen, waitFor, act } from '@testing-library/react';
import { describe, expect, it, vi, beforeEach } from 'vitest';
import { act, fireEvent, render, screen, waitFor } from '@testing-library/react';
import { beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock('@kinvolk/headlamp-plugin/lib', () => ({
ApiProxy: {
@@ -39,9 +39,9 @@ vi.mock('../api/kbench', async importOriginal => {
};
});
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import { ApiProxy } from '@kinvolk/headlamp-plugin/lib';
import { createPvc, createJob, listKbenchJobs } from '../api/kbench';
import { createJob, createPvc, listKbenchJobs } from '../api/kbench';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import { defaultContext, makeSampleStorageClass } from '../test-helpers';
import BenchmarkPage from './BenchmarkPage';
+31 -19
View File
@@ -15,7 +15,7 @@ import {
StatusLabel,
} from '@kinvolk/headlamp-plugin/lib/CommonComponents';
import React, { useCallback, useEffect, useRef, useState } from 'react';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import { formatAge } from '../api/k8s';
import type { BenchmarkState, KbenchJobSummary, KbenchResult } from '../api/kbench';
import {
createJob,
@@ -32,7 +32,7 @@ import {
listKbenchJobs,
parseKbenchLog,
} from '../api/kbench';
import { formatAge } from '../api/k8s';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
// ---------------------------------------------------------------------------
// Result display components
@@ -184,8 +184,8 @@ function KbenchResultDisplay({ result }: { result: KbenchResult }) {
]}
/>
</SectionBox>
<ResultTable title="IOPS (Read/Write)" rows={iopsRows} higherIsBetter={true} />
<ResultTable title="Bandwidth" rows={bwRows} higherIsBetter={true} />
<ResultTable title="IOPS (Read/Write)" rows={iopsRows} higherIsBetter />
<ResultTable title="Bandwidth" rows={bwRows} higherIsBetter />
<ResultTable title="Latency" rows={latRows} higherIsBetter={false} />
</>
);
@@ -601,6 +601,8 @@ export default function BenchmarkPage() {
const [currentResult, setCurrentResult] = useState<KbenchResult | null>(null);
const [lastNamespace, setLastNamespace] = useState('default');
const pollRef = useRef<ReturnType<typeof setInterval> | null>(null);
const cancelledRef = useRef(false);
const [cleaningUp, setCleaningUp] = useState(false);
const scNames = storageClasses.map(sc => sc.metadata.name);
@@ -618,6 +620,7 @@ export default function BenchmarkPage() {
mode: string;
}) {
stopPolling();
cancelledRef.current = false;
setCurrentResult(null);
setLastNamespace(opts.namespace);
@@ -650,7 +653,7 @@ export default function BenchmarkPage() {
setBenchState({ status: 'waiting-pvc', pvcName });
const pvcDeadline = Date.now() + MAX_PVC_WAIT_MS;
let pvcBound = false;
while (Date.now() < pvcDeadline) {
while (Date.now() < pvcDeadline && !cancelledRef.current) {
try {
const pvc = (await ApiProxy.request(
`/api/v1/namespaces/${opts.namespace}/persistentvolumeclaims/${pvcName}`
@@ -664,6 +667,7 @@ export default function BenchmarkPage() {
}
await new Promise(r => setTimeout(r, 5000));
}
if (cancelledRef.current) return;
if (!pvcBound) {
setBenchState({
status: 'failed',
@@ -746,8 +750,14 @@ export default function BenchmarkPage() {
}, POLL_INTERVAL_MS);
}
// Clean up polling on unmount
useEffect(() => () => stopPolling(), []);
// Clean up polling and cancel async loops on unmount
useEffect(
() => () => {
cancelledRef.current = true;
stopPolling();
},
[]
);
const isRunning =
benchState.status !== 'idle' &&
@@ -808,24 +818,25 @@ export default function BenchmarkPage() {
<button
onClick={async () => {
const state = benchState;
if (state.status !== 'complete') return;
if (
!window.confirm(
`Delete job "${state.jobName}" and PVC "${state.pvcName}"?`
)
)
return;
if (state.status !== 'complete' || cleaningUp) return;
setCleaningUp(true);
try {
await deleteJob(state.jobName, lastNamespace);
await deletePvc(state.pvcName, lastNamespace);
setBenchState({ status: 'idle' });
setCurrentResult(null);
} catch (err: unknown) {
alert(
`Cleanup error: ${err instanceof Error ? err.message : String(err)}`
);
setBenchState({
status: 'failed',
error: `Cleanup error: ${err instanceof Error ? err.message : String(err)}`,
jobName: state.jobName,
pvcName: state.pvcName,
});
} finally {
setCleaningUp(false);
}
}}
disabled={cleaningUp}
aria-label="Delete benchmark job and PVC"
style={{
padding: '6px 14px',
@@ -833,11 +844,12 @@ export default function BenchmarkPage() {
color: 'var(--mui-palette-error-main, #d32f2f)',
background: 'transparent',
borderRadius: '4px',
cursor: 'pointer',
cursor: cleaningUp ? 'not-allowed' : 'pointer',
fontSize: '13px',
opacity: cleaningUp ? 0.6 : 1,
}}
>
Delete Job + PVC
{cleaningUp ? 'Deleting...' : 'Delete Job + PVC'}
</button>
),
},
+1 -1
View File
@@ -8,7 +8,7 @@
import { NameValueTable, SectionBox } from '@kinvolk/headlamp-plugin/lib/CommonComponents';
import React from 'react';
import { formatAge, isPodReady, getPodRestarts, TnsCsiPod } from '../api/k8s';
import { formatAge, getPodRestarts, isPodReady, TnsCsiPod } from '../api/k8s';
interface DriverPodDetailSectionProps {
resource: {
+1 -1
View File
@@ -6,8 +6,8 @@ vi.mock(
async () => await import('./__mocks__/commonComponents')
);
import { makeSampleMetrics, makeSamplePod, sampleCSIDriver } from '../test-helpers';
import DriverStatusCard from './DriverStatusCard';
import { makeSamplePod, sampleCSIDriver, makeSampleMetrics } from '../test-helpers';
describe('DriverStatusCard', () => {
it('shows "Not detected" when no CSI driver is present', () => {
+3 -3
View File
@@ -1,5 +1,5 @@
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { describe, expect, it, vi, beforeEach } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock(
'@kinvolk/headlamp-plugin/lib/CommonComponents',
@@ -15,9 +15,9 @@ vi.mock('../api/metrics', async importOriginal => {
};
});
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import { fetchControllerMetrics } from '../api/metrics';
import { defaultContext, makeSamplePod, makeSampleMetrics } from '../test-helpers';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import { defaultContext, makeSampleMetrics, makeSamplePod } from '../test-helpers';
import MetricsPage from './MetricsPage';
function mockContext(overrides?: Parameters<typeof defaultContext>[0]) {
+1 -1
View File
@@ -11,9 +11,9 @@ import {
StatusLabel,
} from '@kinvolk/headlamp-plugin/lib/CommonComponents';
import React, { useCallback, useEffect, useState } from 'react';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import type { TnsCsiMetrics } from '../api/metrics';
import { fetchControllerMetrics, formatBytes, groupByLabel, sumSamples } from '../api/metrics';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
function formatAuditTime(iso: string): string {
const date = new Date(iso);
+3 -3
View File
@@ -1,5 +1,5 @@
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { describe, expect, it, vi, beforeEach } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock(
'@kinvolk/headlamp-plugin/lib/CommonComponents',
@@ -15,15 +15,15 @@ vi.mock('../api/metrics', async importOriginal => {
};
});
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import { fetchControllerMetrics } from '../api/metrics';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import {
defaultContext,
makeSampleMetrics,
makeSamplePod,
makeSamplePV,
makeSamplePVC,
makeSampleStorageClass,
makeSampleMetrics,
sampleCSIDriver,
} from '../test-helpers';
import OverviewPage from './OverviewPage';
+19 -18
View File
@@ -14,11 +14,11 @@ import {
SimpleTable,
StatusLabel,
} from '@kinvolk/headlamp-plugin/lib/CommonComponents';
import React, { useCallback, useEffect, useState } from 'react';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import React, { useCallback, useEffect, useMemo, useState } from 'react';
import { formatAge, formatProtocol, phaseToStatus } from '../api/k8s';
import type { TnsCsiMetrics } from '../api/metrics';
import { fetchControllerMetrics } from '../api/metrics';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import DriverStatusCard from './DriverStatusCard';
// ---------------------------------------------------------------------------
@@ -26,10 +26,10 @@ import DriverStatusCard from './DriverStatusCard';
// ---------------------------------------------------------------------------
const PROTOCOL_COLORS: Record<string, string> = {
NFS: '#1976d2',
'NVMe-oF': '#9c27b0',
iSCSI: '#f57c00',
Other: '#9e9e9e',
NFS: 'var(--mui-palette-primary-main, #1976d2)',
'NVMe-oF': 'var(--mui-palette-secondary-main, #9c27b0)',
iSCSI: 'var(--mui-palette-warning-main, #f57c00)',
Other: 'var(--mui-palette-action-disabled, #9e9e9e)',
};
function protocolChartData(storageClasses: Array<{ parameters?: { protocol?: string } }>) {
@@ -85,7 +85,7 @@ export default function OverviewPage() {
void fetchMetrics();
}, [fetchMetrics]);
const capacityByPool: Map<string, number> = React.useMemo(() => {
const capacityByPool: Map<string, number> = useMemo(() => {
const map = new Map<string, number>();
if (!metrics) return map;
const handleToPool = new Map<string, string>();
@@ -256,19 +256,19 @@ export default function OverviewPage() {
},
...(pvcStatusCounts.Pending > 0
? [
{
name: 'PVCs (Pending)',
value: <StatusLabel status="warning">{pvcStatusCounts.Pending}</StatusLabel>,
},
]
{
name: 'PVCs (Pending)',
value: <StatusLabel status="warning">{pvcStatusCounts.Pending}</StatusLabel>,
},
]
: []),
...(pvcStatusCounts.Lost > 0
? [
{
name: 'PVCs (Lost)',
value: <StatusLabel status="error">{pvcStatusCounts.Lost}</StatusLabel>,
},
]
{
name: 'PVCs (Lost)',
value: <StatusLabel status="error">{pvcStatusCounts.Lost}</StatusLabel>,
},
]
: []),
]}
/>
@@ -318,7 +318,8 @@ export default function OverviewPage() {
</SectionBox>
)}
{/* Provisioned capacity by pool (from Prometheus metrics — shown when TrueNAS API not configured) */}
{/* Provisioned capacity by pool (from Prometheus metrics —
shown when TrueNAS API not configured) */}
{poolStats.length === 0 && !poolStatsError && capacityByPool.size > 0 && (
<SectionBox title="Provisioned Capacity by Pool">
<NameValueTable
+1 -1
View File
@@ -7,8 +7,8 @@
import { NameValueTable, SectionBox } from '@kinvolk/headlamp-plugin/lib/CommonComponents';
import React from 'react';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import { findBoundPv, formatProtocol } from '../api/k8s';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
interface PVCDetailSectionProps {
resource: {
+1 -1
View File
@@ -12,9 +12,9 @@ import {
StatusLabel,
} from '@kinvolk/headlamp-plugin/lib/CommonComponents';
import React from 'react';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import type { VolumeSnapshot } from '../api/k8s';
import { formatAge } from '../api/k8s';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
export default function SnapshotsPage() {
const { volumeSnapshots, volumeSnapshotClasses, snapshotCrdAvailable, loading, error } =
+2 -2
View File
@@ -1,5 +1,5 @@
import { fireEvent, render, screen } from '@testing-library/react';
import { describe, expect, it, vi, beforeEach } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock(
'@kinvolk/headlamp-plugin/lib/CommonComponents',
@@ -16,7 +16,7 @@ vi.mock('react-router-dom', () => ({
vi.mock('../api/TnsCsiDataContext');
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import { defaultContext, makeSampleStorageClass, makeSamplePV } from '../test-helpers';
import { defaultContext, makeSamplePV, makeSampleStorageClass } from '../test-helpers';
import StorageClassesPage from './StorageClassesPage';
function mockContext(overrides?: Parameters<typeof defaultContext>[0]) {
+5 -6
View File
@@ -13,11 +13,11 @@ import {
SimpleTable,
StatusLabel,
} from '@kinvolk/headlamp-plugin/lib/CommonComponents';
import React, { useEffect, useState } from 'react';
import React, { useCallback, useEffect, useState } from 'react';
import { useHistory, useLocation } from 'react-router-dom';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import type { TnsCsiStorageClass } from '../api/k8s';
import { formatProtocol } from '../api/k8s';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
// ---------------------------------------------------------------------------
// Detail drawer
@@ -196,10 +196,10 @@ export default function StorageClassesPage() {
history.push(`${location.pathname}#${name}`);
};
const closeSc = () => {
const closeSc = useCallback(() => {
setSelectedName(null);
history.push(location.pathname);
};
}, [history, location.pathname]);
useEffect(() => {
if (!selectedName) return;
@@ -208,8 +208,7 @@ export default function StorageClassesPage() {
};
window.addEventListener('keydown', handleKey);
return () => window.removeEventListener('keydown', handleKey);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [selectedName]);
}, [selectedName, closeSc]);
if (loading) return <Loader title="Loading storage classes..." />;
+1 -1
View File
@@ -1,5 +1,5 @@
import { fireEvent, render, screen } from '@testing-library/react';
import { describe, expect, it, vi, beforeEach } from 'vitest';
import { beforeEach, describe, expect, it, vi } from 'vitest';
vi.mock(
'@kinvolk/headlamp-plugin/lib/CommonComponents',
+5 -6
View File
@@ -11,11 +11,11 @@ import {
SimpleTable,
StatusLabel,
} from '@kinvolk/headlamp-plugin/lib/CommonComponents';
import React, { useEffect, useState } from 'react';
import React, { useCallback, useEffect, useState } from 'react';
import { useHistory, useLocation } from 'react-router-dom';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
import type { TnsCsiPersistentVolume } from '../api/k8s';
import { formatAccessModes, formatAge, formatProtocol, phaseToStatus } from '../api/k8s';
import { useTnsCsiContext } from '../api/TnsCsiDataContext';
// ---------------------------------------------------------------------------
// Detail panel
@@ -180,10 +180,10 @@ export default function VolumesPage() {
history.push(`${location.pathname}#${name}`);
};
const closeVolume = () => {
const closeVolume = useCallback(() => {
setSelectedName(null);
history.push(location.pathname);
};
}, [history, location.pathname]);
useEffect(() => {
if (!selectedName) return;
@@ -192,8 +192,7 @@ export default function VolumesPage() {
};
window.addEventListener('keydown', handleKey);
return () => window.removeEventListener('keydown', handleKey);
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [selectedName]);
}, [selectedName, closeVolume]);
if (loading) return <Loader title="Loading volumes..." />;
+2 -2
View File
@@ -15,20 +15,20 @@ import {
} from '@kinvolk/headlamp-plugin/lib';
import React from 'react';
import { TnsCsiDataProvider } from './api/TnsCsiDataContext';
import TnsCsiSettings from './components/TnsCsiSettings';
import BenchmarkPage from './components/BenchmarkPage';
import DriverPodDetailSection from './components/DriverPodDetailSection';
import StorageClassBenchmarkButton from './components/integrations/StorageClassBenchmarkButton';
import {
buildPVColumns,
buildStorageClassColumns,
} from './components/integrations/StorageClassColumns';
import StorageClassBenchmarkButton from './components/integrations/StorageClassBenchmarkButton';
import MetricsPage from './components/MetricsPage';
import OverviewPage from './components/OverviewPage';
import PVCDetailSection from './components/PVCDetailSection';
import PVDetailSection from './components/PVDetailSection';
import SnapshotsPage from './components/SnapshotsPage';
import StorageClassesPage from './components/StorageClassesPage';
import TnsCsiSettings from './components/TnsCsiSettings';
import VolumesPage from './components/VolumesPage';
// ---------------------------------------------------------------------------
+1 -1
View File
@@ -4,7 +4,6 @@
*/
import { vi } from 'vitest';
import type { TnsCsiContextValue } from './api/TnsCsiDataContext';
import type {
CSIDriver,
TnsCsiPersistentVolume,
@@ -15,6 +14,7 @@ import type {
VolumeSnapshotClass,
} from './api/k8s';
import type { TnsCsiMetrics } from './api/metrics';
import type { TnsCsiContextValue } from './api/TnsCsiDataContext';
// ---------------------------------------------------------------------------
// Default context value (everything empty / zeroed)