From 69ca8f960ec633708e5dfa1a47c4be13ca31b904 Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 02:20:34 -0400 Subject: [PATCH 1/3] fix(clusters): Restart Cluster reports a failed cluster load instead of staying disabled useRestartClusterClick awaited getClusterInfo, and built the per-instance clients, outside any try. When either failed, the click handler rejected unhandled: the Restart button stayed disabled until remount, the "Restarting" toast spun until its 60s duration ran out, and the rejection reached the console as an unhandled one. Mirror useRollingConfigUpdate's handling from #1861: load the restart targets inside a try, end the restart toast with "Could not load the cluster's instances, so nothing was restarted.", and clear the pending state in a finally. Both hooks built their targets the same way (fetch the cluster, check Fabric Connect, keep RUNNING instances, build a client each, reverse), so that now lives in one getClusterRestartTargets helper. Also stop reporting an empty cluster as "Cluster fully restarted!", the same zero-instance fix #1861 made to the config update. Closes #1853 Co-Authored-By: Claude Opus 5.5 --- src/hooks/getClusterRestartTargets.test.ts | 70 +++++++ src/hooks/getClusterRestartTargets.ts | 35 ++++ src/hooks/useRestartClusterClick.test.tsx | 138 +++++++++++++ src/hooks/useRestartClusterClick.tsx | 230 +++++++++++---------- src/hooks/useRollingConfigUpdate.tsx | 25 +-- 5 files changed, 366 insertions(+), 132 deletions(-) create mode 100644 src/hooks/getClusterRestartTargets.test.ts create mode 100644 src/hooks/getClusterRestartTargets.ts create mode 100644 src/hooks/useRestartClusterClick.test.tsx diff --git a/src/hooks/getClusterRestartTargets.test.ts b/src/hooks/getClusterRestartTargets.test.ts new file mode 100644 index 000000000..1e380def9 --- /dev/null +++ b/src/hooks/getClusterRestartTargets.test.ts @@ -0,0 +1,70 @@ +import type { Cluster, Instance } from '@/integrations/api/api.patch'; +import { afterEach, describe, expect, it, vi } from 'vitest'; + +const { fabricConnect } = vi.hoisted(() => ({ fabricConnect: { current: false } })); +vi.mock('@/config/getInstanceClient', () => ({ + getInstanceClient: vi.fn((config: unknown) => ({ config })), +})); +vi.mock('@/features/auth/store/authStore', () => ({ + authStore: { checkForFabricConnect: vi.fn(() => fabricConnect.current) }, +})); +vi.mock('@/features/cluster/queries/getClusterInfoQuery', () => ({ getClusterInfo: vi.fn() })); + +import { getInstanceClient } from '@/config/getInstanceClient'; +import { authStore } from '@/features/auth/store/authStore'; +import { getClusterInfo } from '@/features/cluster/queries/getClusterInfoQuery'; +import { getClusterRestartTargets } from './getClusterRestartTargets'; + +function instance(id: string, status: Instance['status'] = 'RUNNING'): Instance { + return { + id, + status, + instanceFqdn: `${id}.example.com`, + operationsApiPort: 9925, + operationsApiSecure: true, + } as Instance; +} + +afterEach(() => { + fabricConnect.current = false; + vi.clearAllMocks(); +}); + +describe('getClusterRestartTargets', () => { + it('builds a client for each RUNNING instance, last instance first, and keeps every instance', async () => { + const instances = [instance('ins-a'), instance('ins-b', 'STOPPED'), instance('ins-c')]; + vi.mocked(getClusterInfo).mockResolvedValue({ id: 'clu-1', instances } as Cluster); + + const { allInstances, targets } = await getClusterRestartTargets('clu-1'); + + expect(getClusterInfo).toHaveBeenCalledWith('clu-1'); + expect(allInstances).toBe(instances); + expect(targets.map(({ instance }) => instance.id)).toEqual(['ins-c', 'ins-a']); + expect(targets[0].instanceClient).toEqual({ + config: { id: 'ins-c', forceFabricConnect: false, operationsUrl: 'https://ins-c.example.com:9925/' }, + }); + }); + + it("forwards the cluster's Fabric Connect mode to every client", async () => { + fabricConnect.current = true; + vi.mocked(getClusterInfo).mockResolvedValue({ id: 'clu-1', instances: [instance('ins-a')] } as Cluster); + + await getClusterRestartTargets('clu-1'); + + expect(authStore.checkForFabricConnect).toHaveBeenCalledWith('clu-1'); + expect(getInstanceClient).toHaveBeenCalledWith(expect.objectContaining({ forceFabricConnect: true })); + }); + + it('treats a cluster without an instance list as empty', async () => { + vi.mocked(getClusterInfo).mockResolvedValue({ id: 'clu-1' } as Cluster); + + expect(await getClusterRestartTargets('clu-1')).toEqual({ allInstances: [], targets: [] }); + }); + + it('rejects when the cluster cannot be fetched, so callers can report it', async () => { + vi.mocked(getClusterInfo).mockRejectedValue(new Error('503')); + + await expect(getClusterRestartTargets('clu-1')).rejects.toThrow('503'); + expect(getInstanceClient).not.toHaveBeenCalled(); + }); +}); diff --git a/src/hooks/getClusterRestartTargets.ts b/src/hooks/getClusterRestartTargets.ts new file mode 100644 index 000000000..f5f430941 --- /dev/null +++ b/src/hooks/getClusterRestartTargets.ts @@ -0,0 +1,35 @@ +import { getInstanceClient } from '@/config/getInstanceClient'; +import { authStore } from '@/features/auth/store/authStore'; +import { getClusterInfo } from '@/features/cluster/queries/getClusterInfoQuery'; +import { Instance } from '@/integrations/api/api.patch'; +import { getOperationsUrlForInstance } from '@/lib/urls/getOperationsUrlForInstance'; +import type { AxiosInstance } from 'axios'; + +export interface ClusterRestartTarget { + instance: Instance; + instanceClient: AxiosInstance; +} + +export interface ClusterRestartTargets { + allInstances: Instance[]; + /** A client for each RUNNING instance, in the order a rolling restart visits them. */ + targets: ClusterRestartTarget[]; +} + +export async function getClusterRestartTargets(clusterId: string): Promise { + const cluster = await getClusterInfo(clusterId); + const allInstances = cluster?.instances ?? []; + const clusterUsesFabricConnect = !!cluster && authStore.checkForFabricConnect(cluster.id); + const targets = allInstances + .filter(instance => instance.status === 'RUNNING') + .map(instance => ({ + instance, + instanceClient: getInstanceClient({ + id: instance.id, + forceFabricConnect: clusterUsesFabricConnect, + operationsUrl: getOperationsUrlForInstance(instance), + }), + })) + .reverse(); + return { allInstances, targets }; +} diff --git a/src/hooks/useRestartClusterClick.test.tsx b/src/hooks/useRestartClusterClick.test.tsx new file mode 100644 index 000000000..edffce2a0 --- /dev/null +++ b/src/hooks/useRestartClusterClick.test.tsx @@ -0,0 +1,138 @@ +/** + * @vitest-environment jsdom + */ +import type { Cluster, Instance } from '@/integrations/api/api.patch'; +import { getRestartState } from '@/lib/restart/restartTracker'; +import { act, renderHook } from '@testing-library/react'; +import type { AxiosInstance } from 'axios'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@tanstack/react-router', () => ({ + useParams: () => ({ clusterId: 'clu-1' }), +})); +vi.mock('@/config/getInstanceClient', () => ({ + getInstanceClient: vi.fn(({ id }: { id: string }) => ({ target: id })), +})); +vi.mock('@/features/auth/store/authStore', () => ({ + authStore: { checkForFabricConnect: () => false }, +})); +vi.mock('@/features/cluster/queries/getClusterInfoQuery', () => ({ getClusterInfo: vi.fn() })); +vi.mock('@/integrations/api/instance/status/getInstanceUserInfo', () => ({ getInstanceUserInfo: vi.fn() })); +vi.mock('@/integrations/api/instance/status/restartInstance', () => ({ restartInstance: vi.fn() })); +vi.mock('@/lib/sleep', () => ({ sleep: vi.fn(() => Promise.resolve()) })); +vi.mock('sonner', () => ({ + toast: { loading: vi.fn(() => 'toast-1'), error: vi.fn(), success: vi.fn(), dismiss: vi.fn() }, +})); + +import { getInstanceClient } from '@/config/getInstanceClient'; +import { getClusterInfo } from '@/features/cluster/queries/getClusterInfoQuery'; +import { getInstanceUserInfo } from '@/integrations/api/instance/status/getInstanceUserInfo'; +import { restartInstance } from '@/integrations/api/instance/status/restartInstance'; +import { toast } from 'sonner'; +import { useRestartClusterClick } from './useRestartClusterClick'; + +function instance(id: string, overrides: Partial = {}): Instance { + return { + id, + status: 'RUNNING', + instanceFqdn: `${id}.example.com`, + operationsApiPort: 9925, + operationsApiSecure: true, + ...overrides, + } as Instance; +} + +function targetOf(client: AxiosInstance): string { + return (client as unknown as { target: string }).target; +} + +let restarted: string[] = []; + +async function clickRestart() { + const { result } = renderHook(() => useRestartClusterClick()); + await act(async () => { + await (result.current.onRestartClick() as unknown as Promise); + }); + return result; +} + +beforeEach(() => { + restarted = []; + vi.mocked(getInstanceUserInfo).mockResolvedValue({} as never); + vi.mocked(restartInstance).mockImplementation(async ({ instanceClient }) => { + restarted.push(targetOf(instanceClient)); + return { message: 'ok' }; + }); +}); + +afterEach(() => vi.clearAllMocks()); + +describe('useRestartClusterClick', () => { + it('restarts only the RUNNING instances, last first, and reports the one it could not restart', async () => { + vi.mocked(getClusterInfo).mockResolvedValue( + { + id: 'clu-1', + instances: [instance('ins-a'), instance('ins-b', { status: 'STOPPED' }), instance('ins-c')], + } as Cluster, + ); + + const result = await clickRestart(); + + expect(restarted).toEqual(['ins-c', 'ins-a']); + expect(result.current.isRestartPending).toBe(false); + expect(toast.error).toHaveBeenCalledWith( + 'Error', + expect.objectContaining({ description: expect.stringContaining('Only 2 "RUNNING" instances were restarted') }), + ); + }); + + it('reports success once every instance restarted', async () => { + vi.mocked(getClusterInfo).mockResolvedValue({ id: 'clu-1', instances: [instance('ins-a')] } as Cluster); + + await clickRestart(); + + expect(restarted).toEqual(['ins-a']); + expect(toast.success).toHaveBeenCalledWith('Success', expect.objectContaining({ id: 'toast-1' })); + expect(getRestartState('clu-1')).toBeUndefined(); + }); + + it('reports a cluster with no instances as a failure, not a success', async () => { + vi.mocked(getClusterInfo).mockResolvedValue({ id: 'clu-1', instances: [] } as unknown as Cluster); + + await clickRestart(); + + expect(toast.success).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith( + 'Error', + expect.objectContaining({ description: expect.stringContaining('No instances were found within the cluster') }), + ); + }); + + describe('when the cluster cannot be loaded', () => { + it.each([ + ['the cluster fetch rejects', () => vi.mocked(getClusterInfo).mockRejectedValue(new Error('503'))], + ['an instance client cannot be built', () => { + vi.mocked(getClusterInfo).mockResolvedValue({ id: 'clu-1', instances: [instance('ins-a')] } as Cluster); + vi.mocked(getInstanceClient).mockImplementationOnce(() => { + throw new TypeError('Invalid URL'); + }); + }], + ])('when %s, it reports it on the restart toast, restarts nothing, and stops pending', async (_label, arrange) => { + arrange(); + + const result = await clickRestart(); + + expect(restartInstance).not.toHaveBeenCalled(); + expect(result.current.isRestartPending).toBe(false); + expect(getRestartState('clu-1')).toBeUndefined(); + expect(toast.success).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith( + 'Error', + expect.objectContaining({ + id: 'toast-1', + description: "Could not load the cluster's instances, so nothing was restarted.", + }), + ); + }); + }); +}); diff --git a/src/hooks/useRestartClusterClick.tsx b/src/hooks/useRestartClusterClick.tsx index 923374f84..8311e9d12 100644 --- a/src/hooks/useRestartClusterClick.tsx +++ b/src/hooks/useRestartClusterClick.tsx @@ -1,14 +1,11 @@ import { ProgressBar } from '@/components/ProgressBar'; -import { getInstanceClient } from '@/config/getInstanceClient'; -import { authStore } from '@/features/auth/store/authStore'; -import { getClusterInfo } from '@/features/cluster/queries/getClusterInfoQuery'; +import { ClusterRestartTargets, getClusterRestartTargets } from '@/hooks/getClusterRestartTargets'; import { getInstanceUserInfo } from '@/integrations/api/instance/status/getInstanceUserInfo'; import { restartInstance } from '@/integrations/api/instance/status/restartInstance'; import { excludeFalsy } from '@/lib/arrays/excludeFalsy'; import { pluralize } from '@/lib/pluralize'; import { markRestarting } from '@/lib/restart/restartTracker'; import { sleep } from '@/lib/sleep'; -import { getOperationsUrlForInstance } from '@/lib/urls/getOperationsUrlForInstance'; import { useParams } from '@tanstack/react-router'; import { useCallback, useState } from 'react'; import { toast } from 'sonner'; @@ -57,124 +54,131 @@ export function useRestartClusterClick( ), }); - const cluster = await getClusterInfo(clusterId); - const clusterUsesFabricConnect = authStore.checkForFabricConnect(cluster.id); - const allInstances = cluster?.instances ?? []; - const runningInstances = allInstances.filter(instance => instance.status === 'RUNNING').reverse(); - const instanceClients = runningInstances.map(instance => - getInstanceClient({ - id: instance.id, - forceFabricConnect: clusterUsesFabricConnect, - operationsUrl: getOperationsUrlForInstance(instance), - }) - ); - let instancesRestarted = 0; + try { + let plan: ClusterRestartTargets; + try { + plan = await getClusterRestartTargets(clusterId); + } catch { + toast.error('Error', { + id: toastId, + description: "Could not load the cluster's instances, so nothing was restarted.", + duration: 10_000, + action: { + label: 'Dismiss', + onClick: () => toast.dismiss(), + }, + }); + return; + } + const { allInstances, targets } = plan; + let instancesRestarted = 0; - // One instance at a time, so the cluster stays reachable through the others unless it has only - // one. The TTLs only matter if this loop dies without reaching `finally`. - const releaseCluster = markRestarting([clusterId], { - reach: runningInstances.length === 1 ? 'down' : 'rolling', - ttlMs: (runningInstances.length + 1) * INSTANCE_RESTART_TTL_MS, - }); + // One instance at a time, so the cluster stays reachable through the others unless it has only + // one. The TTLs only matter if this loop dies without reaching `finally`. + const releaseCluster = markRestarting([clusterId], { + reach: targets.length === 1 ? 'down' : 'rolling', + ttlMs: (targets.length + 1) * INSTANCE_RESTART_TTL_MS, + }); - try { - for (let i = 0; i < instanceClients.length; i++) { - const instanceClient = instanceClients[i]; - if (!canceled) { - toast.loading(`Restarting Instance ${i + 1} of ${instanceClients.length}`, { - ...toastConfig, - id: toastId, - description: ( - - ), - }); - let releaseInstance: (() => void) | undefined; - try { - // Make sure the instance is responding. Ungated, so an instance central manager already - // reports as restarting fails here instead of stalling the loop. - await getInstanceUserInfo({ - instanceClient, - skipRestartGate: true, + try { + for (let i = 0; i < targets.length; i++) { + const { instance, instanceClient } = targets[i]; + if (!canceled) { + toast.loading(`Restarting Instance ${i + 1} of ${targets.length}`, { + ...toastConfig, + id: toastId, + description: ( + + ), }); - releaseInstance = markRestarting([runningInstances[i].id], { - reach: 'down', - ttlMs: INSTANCE_RESTART_TTL_MS, - }); - // Then restart it. - await restartInstance({ - operation: 'restart', - replicated: false, - instanceClient, - }); - instancesRestarted += 1; - } catch { - if (i + 1 !== instanceClients.length) { - // If it fails to restart, or wasn't available, warn for a bit then move on. - toast.loading(`Failed Restarting Instance ${i + 1} of ${instanceClients.length}`, { - ...toastConfig, - id: toastId, - description: 'We will carry on momentarily.', + let releaseInstance: (() => void) | undefined; + try { + // Make sure the instance is responding. Ungated, so an instance central manager already + // reports as restarting fails here instead of stalling the loop. + await getInstanceUserInfo({ + instanceClient, + skipRestartGate: true, + }); + releaseInstance = markRestarting([instance.id], { + reach: 'down', + ttlMs: INSTANCE_RESTART_TTL_MS, }); - await sleep(3000); + // Then restart it. + await restartInstance({ + operation: 'restart', + replicated: false, + instanceClient, + }); + instancesRestarted += 1; + } catch { + if (i + 1 !== targets.length) { + // If it fails to restart, or wasn't available, warn for a bit then move on. + toast.loading(`Failed Restarting Instance ${i + 1} of ${targets.length}`, { + ...toastConfig, + id: toastId, + description: 'We will carry on momentarily.', + }); + await sleep(3000); + } + } finally { + releaseInstance?.(); } - } finally { - releaseInstance?.(); } } + } finally { + // Releasing refetches the cluster's queries (`onRestartSettled`). + releaseCluster(); } - } finally { - // Releasing refetches the cluster's queries (`onRestartSettled`). - releaseCluster(); - } - - setIsRestartPending(false); - if (canceled) { - toast.error('Cancelled', { - id: toastId, - description: `The restart was partially cancelled.`, - duration: 10_000, - action: { - label: 'Dismiss', - onClick: () => toast.dismiss(), - }, - }); - } else if (allInstances.length === instancesRestarted) { - onRestartedSuccessfully?.(); - toast.success('Success', { - id: toastId, - description: `Cluster fully restarted!`, - duration: 10_000, - action: { - label: 'Dismiss', - onClick: () => toast.dismiss(), - }, - }); - } else { - const allTheInstances = pluralize(allInstances.length, 'instance', 'instances'); - const someRunningInstancesWere = pluralize( - instancesRestarted, - '"RUNNING" instance was', - '"RUNNING" instances were', - ); - toast.error('Error', { - id: toastId, - description: `Failed to fully restart cluster.\n` - + ([ - allInstances.length === 0 && 'No instances were found within the cluster to restart.', - instancesRestarted === 0 && `No instances were in a "RUNNING" state of ${allTheInstances}.`, - allInstances.length !== instancesRestarted - && `Only ${someRunningInstancesWere} restarted of ${allTheInstances}.`, - ].filter(excludeFalsy).shift() || ''), - duration: 10_000, - action: { - label: 'Dismiss', - onClick: () => toast.dismiss(), - }, - }); + if (canceled) { + toast.error('Cancelled', { + id: toastId, + description: `The restart was partially cancelled.`, + duration: 10_000, + action: { + label: 'Dismiss', + onClick: () => toast.dismiss(), + }, + }); + } else if (allInstances.length > 0 && allInstances.length === instancesRestarted) { + onRestartedSuccessfully?.(); + toast.success('Success', { + id: toastId, + description: `Cluster fully restarted!`, + duration: 10_000, + action: { + label: 'Dismiss', + onClick: () => toast.dismiss(), + }, + }); + } else { + const allTheInstances = pluralize(allInstances.length, 'instance', 'instances'); + const someRunningInstancesWere = pluralize( + instancesRestarted, + '"RUNNING" instance was', + '"RUNNING" instances were', + ); + toast.error('Error', { + id: toastId, + description: `Failed to fully restart cluster.\n` + + ([ + allInstances.length === 0 && 'No instances were found within the cluster to restart.', + instancesRestarted === 0 && `No instances were in a "RUNNING" state of ${allTheInstances}.`, + allInstances.length !== instancesRestarted + && `Only ${someRunningInstancesWere} restarted of ${allTheInstances}.`, + ].filter(excludeFalsy).shift() || ''), + duration: 10_000, + action: { + label: 'Dismiss', + onClick: () => toast.dismiss(), + }, + }); + } + } finally { + setIsRestartPending(false); } }, [clusterId, onRestartedSuccessfully]); diff --git a/src/hooks/useRollingConfigUpdate.tsx b/src/hooks/useRollingConfigUpdate.tsx index b3423d961..19b34090b 100644 --- a/src/hooks/useRollingConfigUpdate.tsx +++ b/src/hooks/useRollingConfigUpdate.tsx @@ -1,9 +1,7 @@ import { ProgressBar } from '@/components/ProgressBar'; -import { getInstanceClient } from '@/config/getInstanceClient'; import { InstanceClientIdConfig, InstanceTypeConfig } from '@/config/instanceClientConfig'; import { useInstanceClientIdParams } from '@/config/useInstanceClient'; -import { authStore } from '@/features/auth/store/authStore'; -import { getClusterInfo } from '@/features/cluster/queries/getClusterInfoQuery'; +import { getClusterRestartTargets } from '@/hooks/getClusterRestartTargets'; import { Instance } from '@/integrations/api/api.patch'; import { getInstanceUserInfo } from '@/integrations/api/instance/status/getInstanceUserInfo'; import { restartInstance } from '@/integrations/api/instance/status/restartInstance'; @@ -12,7 +10,6 @@ import { acknowledgedReplicationNodes } from '@/integrations/api/replication'; import { excludeFalsy } from '@/lib/arrays/excludeFalsy'; import { pluralize } from '@/lib/pluralize'; import { sleep } from '@/lib/sleep'; -import { getOperationsUrlForInstance } from '@/lib/urls/getOperationsUrlForInstance'; import { useQueryClient } from '@tanstack/react-query'; import type { AxiosInstance } from 'axios'; import { useCallback, useState } from 'react'; @@ -51,23 +48,13 @@ async function planRollingConfigUpdate( fanOutAfterCanary: false, }; } - const cluster = await getClusterInfo(operationsParams.entityId); - const allInstances = cluster?.instances || []; - const clusterUsesFabricConnect = !!cluster && authStore.checkForFabricConnect(cluster.id); - const targets = allInstances - .filter(instance => instance.status === 'RUNNING') - .map(instance => ({ - instanceClient: getInstanceClient({ - id: instance.id, - forceFabricConnect: clusterUsesFabricConnect, - operationsUrl: getOperationsUrlForInstance(instance), - }), - replicationFqdn: instance.replicationFqdn, - })) - .reverse(); + const { allInstances, targets } = await getClusterRestartTargets(operationsParams.entityId); return { allInstances, - targets, + targets: targets.map(({ instance, instanceClient }) => ({ + instanceClient, + replicationFqdn: instance.replicationFqdn, + })), fanOutAfterCanary: shouldFanOutSetConfiguration(allInstances), }; } From b9dfc8be7eafa0180182e0aa61357deabf540a0e Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Sun, 11 Oct 2026 02:29:06 -0400 Subject: [PATCH 2/3] fix(clusters): honor Cancel clicked while an instance is being checked The restart, config-update and apply-licenses loops only looked at their Cancel flag at the top of each iteration. A Cancel clicked while an instance's availability check was in flight was therefore ignored for that instance: the restart, the config write, or the license install went out anyway. Recheck the flag once the check returns, before the first request that changes the instance. Also stop telling the user no instance was RUNNING when every RUNNING instance failed to restart. Both cluster hooks keyed that message on nothing having restarted; key it on there being no RUNNING target, so a cluster whose restarts all failed reads "Only 0 RUNNING instances were restarted of N". Found by the pre-push cross-model review of the previous commit; both predate it. Co-Authored-By: Claude Opus 5.5 --- src/hooks/useApplyLicensesClick.test.tsx | 78 +++++++++++++++++++++++ src/hooks/useApplyLicensesClick.tsx | 3 + src/hooks/useRestartClusterClick.test.tsx | 33 ++++++++++ src/hooks/useRestartClusterClick.tsx | 5 +- src/hooks/useRollingConfigUpdate.test.tsx | 30 +++++++++ src/hooks/useRollingConfigUpdate.tsx | 5 +- 6 files changed, 152 insertions(+), 2 deletions(-) create mode 100644 src/hooks/useApplyLicensesClick.test.tsx diff --git a/src/hooks/useApplyLicensesClick.test.tsx b/src/hooks/useApplyLicensesClick.test.tsx new file mode 100644 index 000000000..87f4f5a62 --- /dev/null +++ b/src/hooks/useApplyLicensesClick.test.tsx @@ -0,0 +1,78 @@ +/** + * @vitest-environment jsdom + */ +import type { SchemaLicense } from '@/integrations/api/api.gen'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, renderHook } from '@testing-library/react'; +import { PropsWithChildren } from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +vi.mock('@/config/useInstanceClient', () => { + const instanceClient = { target: 'ins-a' }; + const params = { entityType: 'instance', entityId: 'ins-a', instanceClient }; + return { + useInstanceClient: () => instanceClient, + useInstanceClientIdParams: () => params, + }; +}); +vi.mock('@/integrations/api/instance/status/getInstanceUserInfo', () => ({ getInstanceUserInfo: vi.fn() })); +vi.mock('@/integrations/api/instance/auth/installUsageLicense', () => ({ installUsageLicense: vi.fn() })); +vi.mock('@/lib/sleep', () => ({ sleep: vi.fn(() => Promise.resolve()) })); +vi.mock('sonner', () => ({ + toast: { loading: vi.fn(() => 'toast-1'), error: vi.fn(), success: vi.fn(), dismiss: vi.fn() }, +})); + +import { installUsageLicense } from '@/integrations/api/instance/auth/installUsageLicense'; +import { getInstanceUserInfo } from '@/integrations/api/instance/status/getInstanceUserInfo'; +import { toast } from 'sonner'; +import { useApplyLicensesClick } from './useApplyLicensesClick'; + +const LICENSES = [{ license: 'lic-1' }, { license: 'lic-2' }] as SchemaLicense[]; + +function wrapper() { + const queryClient = new QueryClient(); + return ({ children }: PropsWithChildren) => {children} + ; +} + +async function applyLicenses() { + const { result } = renderHook(() => useApplyLicensesClick({ licenses: LICENSES }), { wrapper: wrapper() }); + await act(async () => { + await (result.current.onApplyLicensesClick() as unknown as Promise); + }); + return result; +} + +beforeEach(() => { + vi.mocked(getInstanceUserInfo).mockResolvedValue({} as never); + vi.mocked(installUsageLicense).mockResolvedValue({ message: 'ok' } as never); +}); + +afterEach(() => vi.clearAllMocks()); + +describe('useApplyLicensesClick', () => { + it('installs every license and reports success', async () => { + const result = await applyLicenses(); + + expect(vi.mocked(installUsageLicense).mock.calls.map(([body]) => body.license)).toEqual(['lic-1', 'lic-2']); + expect(result.current.isApplyLicensesPending).toBe(false); + expect(toast.success).toHaveBeenCalled(); + }); + + it('does not install a license when Cancel is clicked while the instance is being checked', async () => { + vi.mocked(getInstanceUserInfo).mockImplementation(async () => { + cancel(); + return {} as never; + }); + + await applyLicenses(); + + expect(installUsageLicense).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith('Cancelled', expect.anything()); + }); +}); + +function cancel() { + const [, options] = vi.mocked(toast.loading).mock.calls.at(-1)!; + (options as unknown as { action: { onClick: () => void } }).action.onClick(); +} diff --git a/src/hooks/useApplyLicensesClick.tsx b/src/hooks/useApplyLicensesClick.tsx index a6303e539..90750d256 100644 --- a/src/hooks/useApplyLicensesClick.tsx +++ b/src/hooks/useApplyLicensesClick.tsx @@ -74,6 +74,9 @@ export function useApplyLicensesClick({ licenses }: ApplyLicensesClickParams): A await getInstanceUserInfo({ instanceClient, }); + if (canceled) { + continue; + } // Then install the usage license to it. await installUsageLicense({ license: license.license, diff --git a/src/hooks/useRestartClusterClick.test.tsx b/src/hooks/useRestartClusterClick.test.tsx index edffce2a0..5d89b76a3 100644 --- a/src/hooks/useRestartClusterClick.test.tsx +++ b/src/hooks/useRestartClusterClick.test.tsx @@ -96,6 +96,34 @@ describe('useRestartClusterClick', () => { expect(getRestartState('clu-1')).toBeUndefined(); }); + it('does not restart an instance when Cancel is clicked while it is being checked', async () => { + vi.mocked(getClusterInfo).mockResolvedValue( + { id: 'clu-1', instances: [instance('ins-a'), instance('ins-b')] } as Cluster, + ); + vi.mocked(getInstanceUserInfo).mockImplementation(async () => { + cancel(); + return {} as never; + }); + + await clickRestart(); + + expect(restartInstance).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith('Cancelled', expect.anything()); + }); + + it('says the RUNNING instances failed to restart, not that none were RUNNING', async () => { + vi.mocked(getClusterInfo).mockResolvedValue( + { id: 'clu-1', instances: [instance('ins-a'), instance('ins-b')] } as Cluster, + ); + vi.mocked(restartInstance).mockRejectedValue(new Error('did not come back')); + + await clickRestart(); + + const [, options] = vi.mocked(toast.error).mock.calls.at(-1)!; + expect(options?.description).toContain('Only 0 "RUNNING" instances were restarted of 2 instances.'); + expect(options?.description).not.toContain('No instances were in a "RUNNING" state'); + }); + it('reports a cluster with no instances as a failure, not a success', async () => { vi.mocked(getClusterInfo).mockResolvedValue({ id: 'clu-1', instances: [] } as unknown as Cluster); @@ -136,3 +164,8 @@ describe('useRestartClusterClick', () => { }); }); }); + +function cancel() { + const [, options] = vi.mocked(toast.loading).mock.calls.at(-1)!; + (options as unknown as { action: { onClick: () => void } }).action.onClick(); +} diff --git a/src/hooks/useRestartClusterClick.tsx b/src/hooks/useRestartClusterClick.tsx index 8311e9d12..e028d1af8 100644 --- a/src/hooks/useRestartClusterClick.tsx +++ b/src/hooks/useRestartClusterClick.tsx @@ -102,6 +102,9 @@ export function useRestartClusterClick( instanceClient, skipRestartGate: true, }); + if (canceled) { + continue; + } releaseInstance = markRestarting([instance.id], { reach: 'down', ttlMs: INSTANCE_RESTART_TTL_MS, @@ -166,7 +169,7 @@ export function useRestartClusterClick( description: `Failed to fully restart cluster.\n` + ([ allInstances.length === 0 && 'No instances were found within the cluster to restart.', - instancesRestarted === 0 && `No instances were in a "RUNNING" state of ${allTheInstances}.`, + targets.length === 0 && `No instances were in a "RUNNING" state of ${allTheInstances}.`, allInstances.length !== instancesRestarted && `Only ${someRunningInstancesWere} restarted of ${allTheInstances}.`, ].filter(excludeFalsy).shift() || ''), diff --git a/src/hooks/useRollingConfigUpdate.test.tsx b/src/hooks/useRollingConfigUpdate.test.tsx index 1a70ba91b..f2560055b 100644 --- a/src/hooks/useRollingConfigUpdate.test.tsx +++ b/src/hooks/useRollingConfigUpdate.test.tsx @@ -434,6 +434,36 @@ describe('useRollingConfigUpdate outcome reporting', () => { expect(options?.description).toContain('Only 1 "RUNNING" instance was restarted of 2 instances.'); expect(options?.description).not.toContain('may already have the new configuration saved'); }); + + it('says the RUNNING instances failed to restart, not that none were RUNNING', async () => { + useCluster([instance('ins-a', '5.1.29'), instance('ins-b', '5.1.29')]); + vi.mocked(restartInstance).mockRejectedValue(new Error('did not come back')); + + await save(); + + const [, options] = vi.mocked(toast.error).mock.calls.at(-1)!; + expect(options?.description).toContain('Only 0 "RUNNING" instances were restarted of 2 instances.'); + expect(options?.description).not.toContain('No instances were in a "RUNNING" state'); + }); +}); + +describe('useRollingConfigUpdate cancellation', () => { + it('does not write or restart an instance when Cancel is clicked while it is being checked', async () => { + useCluster([instance('ins-a', '5.1.29'), instance('ins-b', '5.1.29')]); + vi.mocked(getInstanceUserInfo).mockImplementation(async ({ instanceClient }) => { + events.push(`check ${targetOf(instanceClient)}`); + cancel(); + return {} as never; + }); + + await save(); + + expect(events).toEqual(['check ins-b']); + expect(toast.error).toHaveBeenCalledWith( + 'Cancelled', + expect.objectContaining({ description: 'The config update was partially cancelled.' }), + ); + }); }); describe('useRollingConfigUpdate when the cluster cannot be loaded', () => { diff --git a/src/hooks/useRollingConfigUpdate.tsx b/src/hooks/useRollingConfigUpdate.tsx index 19b34090b..8acab2351 100644 --- a/src/hooks/useRollingConfigUpdate.tsx +++ b/src/hooks/useRollingConfigUpdate.tsx @@ -131,6 +131,9 @@ export function useRollingConfigUpdate( await getInstanceUserInfo({ instanceClient, }); + if (canceled) { + continue; + } if (!replicationFqdn || !acknowledged.has(replicationFqdn.toLowerCase())) { await setConfiguration({ @@ -213,7 +216,7 @@ export function useRollingConfigUpdate( description: `Failed to fully update cluster.\n` + ([ allInstances.length === 0 && 'No instances were found within the cluster to restart.', - instancesRestarted === 0 && `No instances were in a "RUNNING" state of ${allTheInstances}.`, + targets.length === 0 && `No instances were in a "RUNNING" state of ${allTheInstances}.`, allInstances.length !== instancesRestarted && `Only ${someRunningInstancesWere} restarted of ${allTheInstances}.`, ].filter(excludeFalsy).shift() || '') From 3afc3a91b8132c6d9982e4bfb8b4b2b31a6f0e8c Mon Sep 17 00:00:00 2001 From: Dawson Toth Date: Mon, 12 Oct 2026 12:08:39 -0400 Subject: [PATCH 3/3] refactor(restart): end the restart and license loops when Cancel lands during the check Cancellation never unsets, so continuing only re-checked the flag on every remaining target. Refs #1853 Co-Authored-By: Claude Opus 5.5 --- src/hooks/useApplyLicensesClick.tsx | 2 +- src/hooks/useRestartClusterClick.tsx | 2 +- src/hooks/useRollingConfigUpdate.tsx | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/hooks/useApplyLicensesClick.tsx b/src/hooks/useApplyLicensesClick.tsx index 90750d256..493838764 100644 --- a/src/hooks/useApplyLicensesClick.tsx +++ b/src/hooks/useApplyLicensesClick.tsx @@ -75,7 +75,7 @@ export function useApplyLicensesClick({ licenses }: ApplyLicensesClickParams): A instanceClient, }); if (canceled) { - continue; + break; } // Then install the usage license to it. await installUsageLicense({ diff --git a/src/hooks/useRestartClusterClick.tsx b/src/hooks/useRestartClusterClick.tsx index e028d1af8..ca5eefc76 100644 --- a/src/hooks/useRestartClusterClick.tsx +++ b/src/hooks/useRestartClusterClick.tsx @@ -103,7 +103,7 @@ export function useRestartClusterClick( skipRestartGate: true, }); if (canceled) { - continue; + break; } releaseInstance = markRestarting([instance.id], { reach: 'down', diff --git a/src/hooks/useRollingConfigUpdate.tsx b/src/hooks/useRollingConfigUpdate.tsx index 8acab2351..12e8fe22a 100644 --- a/src/hooks/useRollingConfigUpdate.tsx +++ b/src/hooks/useRollingConfigUpdate.tsx @@ -132,7 +132,7 @@ export function useRollingConfigUpdate( instanceClient, }); if (canceled) { - continue; + break; } if (!replicationFqdn || !acknowledged.has(replicationFqdn.toLowerCase())) {