diff --git a/DESIGN.md b/DESIGN.md index b20775d52..e7e13ac77 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -44,6 +44,14 @@ The wide hero artwork spans both columns beneath the form card in three independ - **Only query errors are muted, and only connectivity-class ones.** Mutations still toast; a 403 or a Harper 500 during a restart still toasts. - **Detection is per tab.** A page learns of a container op started elsewhere only from a cluster-record fetch, and the in-cluster studio pages don't poll the cluster, so they don't notice one started in another tab. +## Cluster config updates — when an instance may skip its own `set_configuration` + +[`useRollingConfigUpdate`](src/hooks/useRollingConfigUpdate.tsx) saves the config editor's change by restarting a cluster's RUNNING instances one at a time, and every instance it restarts must already have the new config on disk: `set_configuration` only writes the file, and nothing applies it until a restart. + +- **Pre-5.2, mixed-version, or unknown-version clusters write each instance before restarting it.** `replicated: true` on `set_configuration` exists from Harper 5.2.0 ([`shouldFanOutSetConfiguration`](src/integrations/api/instance/status/setConfiguration.ts)); 5.0 and 5.1 apply the change locally and report no peers. +- **On 5.2+ the first instance is still written and restarted alone.** Only once it is back up on the new config is the write fanned out with `replicated: true` from that instance — even when it was the last instance to restart, because the fan-out also reaches live members Studio does not restart (not RUNNING, or failed earlier), which the per-instance path leaves on the old config. Fanning out first would leave an unproven config pending on every node, which a Cancel could no longer contain. +- **A later instance skips its write only when the fan-out acknowledged that exact node** — its CM `replicationFqdn` (lower-cased) appears among the successful `replicated[]` entries. Fabric names each Harper node after that FQDN (host-manager sets `NODE_HOSTNAME` and `REPLICATION_HOSTNAME` to it). A count of acknowledgements is not proof: the origin's replication peers are not CM's restart targets, so a stale peer can stand in for a missing one. A target with no `replicationFqdn`, a rejected fan-out (the instance client rejects any response with a failed peer), or a response with no usable `replicated[]` all keep the per-instance write. + ## Cluster upgrade picker — which versions an existing cluster is offered [`buildUpgradeVersionOptions`](src/features/clusters/upsert/lib/buildUpgradeVersionOptions.ts) is the one place that decides, and its test file is the spec. Two rules, and they differ on purpose: 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/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..493838764 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) { + break; + } // 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 new file mode 100644 index 000000000..5d89b76a3 --- /dev/null +++ b/src/hooks/useRestartClusterClick.test.tsx @@ -0,0 +1,171 @@ +/** + * @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('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); + + 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.", + }), + ); + }); + }); +}); + +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 923374f84..ca5eefc76 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,134 @@ 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, + }); + if (canceled) { + break; + } + 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.', + targets.length === 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.test.tsx b/src/hooks/useRollingConfigUpdate.test.tsx new file mode 100644 index 000000000..f2560055b --- /dev/null +++ b/src/hooks/useRollingConfigUpdate.test.tsx @@ -0,0 +1,487 @@ +/** + * @vitest-environment jsdom + */ +import type { Cluster, Instance } from '@/integrations/api/api.patch'; +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, renderHook } from '@testing-library/react'; +import type { AxiosInstance } from 'axios'; +import { PropsWithChildren } from 'react'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +interface FakeClient { + target: string; +} + +const { operationsParams } = vi.hoisted(() => ({ + operationsParams: { + current: { + entityType: 'cluster' as 'cluster' | 'instance', + entityId: 'clu-1', + instanceClient: { target: 'cluster-load-balancer' } as unknown, + }, + }, +})); +vi.mock('@/config/useInstanceClient', () => ({ + useInstanceClientIdParams: () => operationsParams.current, +})); +vi.mock('@/config/getInstanceClient', () => ({ + getInstanceClient: ({ 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('@/integrations/api/instance/status/setConfiguration', async (importOriginal) => ({ + ...await importOriginal(), + setConfiguration: 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 { getClusterInfo } from '@/features/cluster/queries/getClusterInfoQuery'; +import { getInstanceUserInfo } from '@/integrations/api/instance/status/getInstanceUserInfo'; +import { restartInstance } from '@/integrations/api/instance/status/restartInstance'; +import { setConfiguration } from '@/integrations/api/instance/status/setConfiguration'; +import { toast } from 'sonner'; +import { useRollingConfigUpdate } from './useRollingConfigUpdate'; + +const CHANGES = { logging_level: 'debug' }; + +function instance(id: string, version: string | undefined, overrides: Partial = {}): Instance { + return { + id, + version, + status: 'RUNNING', + instanceFqdn: `${id}.example.com`, + replicationFqdn: `${id}-repl.example.com`, + operationsApiPort: 9925, + operationsApiSecure: true, + ...overrides, + } as Instance; +} + +function useCluster(instances: Instance[]) { + vi.mocked(getClusterInfo).mockResolvedValue({ id: 'clu-1', instances } as Cluster); +} + +let events: string[] = []; + +function targetOf(client: AxiosInstance): string { + return (client as unknown as FakeClient).target; +} + +const CONFIGURED = 'Configuration successfully set. You must restart Harper for new config settings to take effect.'; + +/** The shape harper-pro's `replicateOperation` returns: each peer's own answer with its node name attached. */ +function acknowledging(...nodes: string[]) { + return { + message: CONFIGURED, + replicated: nodes.map((node, requestId) => ({ message: CONFIGURED, requestId, node })), + }; +} + +function wrapper() { + const queryClient = new QueryClient(); + return ({ children }: PropsWithChildren) => {children} + ; +} + +async function save() { + const { result } = renderHook(() => useRollingConfigUpdate(), { wrapper: wrapper() }); + await act(async () => { + await (result.current.onConfigUpdate(CHANGES) as unknown as Promise); + }); + return result; +} + +beforeEach(() => { + events = []; + operationsParams.current = { + entityType: 'cluster', + entityId: 'clu-1', + instanceClient: { target: 'cluster-load-balancer' }, + }; + vi.mocked(getInstanceUserInfo).mockImplementation(async ({ instanceClient }) => { + events.push(`check ${targetOf(instanceClient)}`); + return {} as never; + }); + vi.mocked(setConfiguration).mockImplementation(async ({ instanceClient, ...changes }) => { + events.push(`${changes.replicated ? 'replicate' : 'write'} ${targetOf(instanceClient)}`); + return { message: 'ok' }; + }); + vi.mocked(restartInstance).mockImplementation(async ({ instanceClient }) => { + events.push(`restart ${targetOf(instanceClient)}`); + return { message: 'ok' }; + }); +}); + +afterEach(() => vi.clearAllMocks()); + +describe('useRollingConfigUpdate on a cluster that replicates set_configuration (5.2+)', () => { + it('writes and restarts one instance, fans the write out from it, then only restarts the acknowledged rest', async () => { + useCluster([instance('ins-a', '5.2.13'), instance('ins-b', '5.3.0'), instance('ins-c', '5.2.0')]); + vi.mocked(setConfiguration).mockImplementation(async ({ instanceClient, ...changes }) => { + events.push(`${changes.replicated ? 'replicate' : 'write'} ${targetOf(instanceClient)}`); + // Node names are compared case-insensitively. + return changes.replicated ? acknowledging('INS-B-REPL.example.com', 'ins-a-repl.example.com') : { message: 'ok' }; + }); + + await save(); + + expect(events).toEqual([ + 'check ins-c', + 'write ins-c', + 'restart ins-c', + 'replicate ins-c', + 'check ins-b', + 'restart ins-b', + 'check ins-a', + 'restart ins-a', + ]); + expect(setConfiguration).toHaveBeenCalledWith(expect.objectContaining({ ...CHANGES, replicated: true })); + expect(toast.success).toHaveBeenCalled(); + }); + + it('never starts the next restart before the previous one has finished', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0'), instance('ins-c', '5.2.0')]); + vi.mocked(setConfiguration).mockImplementation(async ({ instanceClient, ...changes }) => { + events.push(`${changes.replicated ? 'replicate' : 'write'} ${targetOf(instanceClient)}`); + return changes.replicated ? acknowledging('ins-a-repl.example.com', 'ins-b-repl.example.com') : { message: 'ok' }; + }); + let inFlight = 0; + let maxInFlight = 0; + const pending: Array<() => void> = []; + vi.mocked(restartInstance).mockImplementation(({ instanceClient }) => { + events.push(`restart ${targetOf(instanceClient)}`); + inFlight += 1; + maxInFlight = Math.max(maxInFlight, inFlight); + return new Promise(resolve => + pending.push(() => { + inFlight -= 1; + resolve({ message: 'ok' }); + }) + ); + }); + const { result } = renderHook(() => useRollingConfigUpdate(), { wrapper: wrapper() }); + + let done: Promise | undefined; + act(() => { + done = result.current.onConfigUpdate(CHANGES) as unknown as Promise; + }); + for (const expected of ['ins-c', 'ins-b', 'ins-a']) { + await vi.waitFor(() => expect(pending).toHaveLength(1)); + expect(events.at(-1)).toBe(`restart ${expected}`); + await act(async () => pending.shift()!()); + } + await act(async () => done); + + expect(maxInFlight).toBe(1); + expect(toast.success).toHaveBeenCalled(); + }); + + it('keeps the local write for a target replication did not acknowledge, whatever the count', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0'), instance('ins-c', '5.2.0')]); + vi.mocked(setConfiguration).mockImplementation(async ({ instanceClient, ...changes }) => { + events.push(`${changes.replicated ? 'replicate' : 'write'} ${targetOf(instanceClient)}`); + // Two acknowledgements for two remaining targets — but one is a peer Studio is not restarting. + return changes.replicated ? acknowledging('ins-b-repl.example.com', 'stale-peer.example.com') : { message: 'ok' }; + }); + + await save(); + + expect(events).toEqual([ + 'check ins-c', + 'write ins-c', + 'restart ins-c', + 'replicate ins-c', + 'check ins-b', + 'restart ins-b', + 'check ins-a', + 'write ins-a', + 'restart ins-a', + ]); + }); + + it('keeps the local write for a target whose replication name is unknown', async () => { + useCluster([ + instance('ins-a', '5.2.0', { replicationFqdn: undefined }), + instance('ins-b', '5.2.0'), + ]); + vi.mocked(setConfiguration).mockImplementation(async ({ instanceClient, ...changes }) => { + events.push(`${changes.replicated ? 'replicate' : 'write'} ${targetOf(instanceClient)}`); + return changes.replicated ? acknowledging('ins-a-repl.example.com') : { message: 'ok' }; + }); + + await save(); + + expect(events).toEqual([ + 'check ins-b', + 'write ins-b', + 'restart ins-b', + 'replicate ins-b', + 'check ins-a', + 'write ins-a', + 'restart ins-a', + ]); + }); + + it('falls back to writing every remaining instance when the fan-out is rejected, still counting the canary', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0'), instance('ins-c', '5.2.0')]); + vi.mocked(setConfiguration).mockImplementation(async ({ instanceClient, ...changes }) => { + events.push(`${changes.replicated ? 'replicate' : 'write'} ${targetOf(instanceClient)}`); + if (changes.replicated) { + // What `rejectReplicationFailures` turns a partial fan-out into. + throw 'The operation partially succeeded, but 1 node failed:\nins-a-repl.example.com: timeout'; + } + return { message: 'ok' }; + }); + + await save(); + + expect(events).toEqual([ + 'check ins-c', + 'write ins-c', + 'restart ins-c', + 'replicate ins-c', + 'check ins-b', + 'write ins-b', + 'restart ins-b', + 'check ins-a', + 'write ins-a', + 'restart ins-a', + ]); + expect(toast.success).toHaveBeenCalled(); + expect(toast.error).not.toHaveBeenCalled(); + expect(toast.loading).not.toHaveBeenCalledWith(expect.stringContaining('Failed'), expect.anything()); + }); + + it('falls back to writing every remaining instance when the response names no peers', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0')]); + + await save(); + + expect(events).toEqual([ + 'check ins-b', + 'write ins-b', + 'restart ins-b', + 'replicate ins-b', + 'check ins-a', + 'write ins-a', + 'restart ins-a', + ]); + }); + + it('waits to fan out until an instance has actually come back on the new configuration', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0'), instance('ins-c', '5.2.0')]); + vi.mocked(restartInstance).mockImplementation(async ({ instanceClient }) => { + events.push(`restart ${targetOf(instanceClient)}`); + if (targetOf(instanceClient) === 'ins-c') { + throw new Error('did not come back'); + } + return { message: 'ok' }; + }); + vi.mocked(setConfiguration).mockImplementation(async ({ instanceClient, ...changes }) => { + events.push(`${changes.replicated ? 'replicate' : 'write'} ${targetOf(instanceClient)}`); + return changes.replicated ? acknowledging('ins-a-repl.example.com', 'ins-c-repl.example.com') : { message: 'ok' }; + }); + + await save(); + + expect(events).toEqual([ + 'check ins-c', + 'write ins-c', + 'restart ins-c', + 'check ins-b', + 'write ins-b', + 'restart ins-b', + 'replicate ins-b', + 'check ins-a', + 'restart ins-a', + ]); + expect(toast.error).toHaveBeenCalledWith( + 'Error', + expect.objectContaining({ description: expect.stringContaining('may already have the new configuration saved') }), + ); + }); + + it('fans out after the last restart too, so live members Studio does not restart still get the write', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0', { status: 'STOPPED' })]); + + await save(); + + expect(events).toEqual(['check ins-a', 'write ins-a', 'restart ins-a', 'replicate ins-a']); + expect(toast.error).toHaveBeenCalledWith( + 'Error', + expect.objectContaining({ description: expect.stringContaining('may already have the new configuration saved') }), + ); + }); + + it('does not fan out on a cluster with no other live member', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0', { status: 'TERMINATED' })]); + + await save(); + + expect(events).toEqual(['check ins-a', 'write ins-a', 'restart ins-a']); + }); + + it('says configuration may be pending when cancelled after a fan-out', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0'), instance('ins-c', '5.2.0')]); + vi.mocked(setConfiguration).mockImplementation(async ({ instanceClient, ...changes }) => { + events.push(`${changes.replicated ? 'replicate' : 'write'} ${targetOf(instanceClient)}`); + if (changes.replicated) { + cancel(); + return acknowledging('ins-a-repl.example.com', 'ins-b-repl.example.com'); + } + return { message: 'ok' }; + }); + + await save(); + + expect(events).toEqual(['check ins-c', 'write ins-c', 'restart ins-c', 'replicate ins-c']); + expect(toast.error).toHaveBeenCalledWith( + 'Cancelled', + expect.objectContaining({ description: expect.stringContaining('may already have the new configuration saved') }), + ); + }); + + it('does not fan out once cancelled, and keeps the plain cancellation copy', async () => { + useCluster([instance('ins-a', '5.2.0'), instance('ins-b', '5.2.0')]); + vi.mocked(restartInstance).mockImplementation(async ({ instanceClient }) => { + events.push(`restart ${targetOf(instanceClient)}`); + cancel(); + return { message: 'ok' }; + }); + + await save(); + + expect(events).toEqual(['check ins-b', 'write ins-b', 'restart ins-b']); + expect(toast.error).toHaveBeenCalledWith( + 'Cancelled', + expect.objectContaining({ description: 'The config update was partially cancelled.' }), + ); + }); +}); + +describe('useRollingConfigUpdate on clusters that must not replicate', () => { + it.each([ + ['a mixed-version cluster', ['5.2.13', '5.1.20', '5.2.13']], + ['an instance with no reported version', ['5.2.13', undefined, '5.2.13']], + ['a 5.2.0 prerelease', ['5.2.0-beta.1', '5.2.0', '5.2.0']], + ['a pre-5.2 cluster', ['5.1.29', '5.1.29', '5.1.29']], + ])('writes every instance locally on %s', async (_label, versions) => { + useCluster(versions.map((version, index) => instance(`ins-${index}`, version))); + + await save(); + + expect(vi.mocked(setConfiguration).mock.calls.some(([body]) => 'replicated' in body)).toBe(false); + expect(events).toEqual([ + 'check ins-2', + 'write ins-2', + 'restart ins-2', + 'check ins-1', + 'write ins-1', + 'restart ins-1', + 'check ins-0', + 'write ins-0', + 'restart ins-0', + ]); + }); + + it('writes the single instance a per-instance view targets', async () => { + operationsParams.current = { + entityType: 'instance', + entityId: 'ins-a', + instanceClient: { target: 'ins-a-direct' }, + }; + + await save(); + + expect(getClusterInfo).not.toHaveBeenCalled(); + expect(events).toEqual(['check ins-a-direct', 'write ins-a-direct', 'restart ins-a-direct']); + }); +}); + +describe('useRollingConfigUpdate outcome reporting', () => { + it('reports a cluster with no instances as a failure, not a success', async () => { + useCluster([]); + + await save(); + + expect(toast.success).not.toHaveBeenCalled(); + expect(toast.error).toHaveBeenCalledWith( + 'Error', + expect.objectContaining({ description: expect.stringContaining('No instances were found within the cluster') }), + ); + }); + + it('does not mention pending configuration when nothing was fanned out', async () => { + useCluster([instance('ins-a', '5.1.29'), instance('ins-b', '5.1.29')]); + vi.mocked(restartInstance).mockImplementation(async ({ instanceClient }) => { + events.push(`restart ${targetOf(instanceClient)}`); + if (targetOf(instanceClient) === 'ins-b') { + throw new Error('did not come back'); + } + return { message: 'ok' }; + }); + + await save(); + + const [, options] = vi.mocked(toast.error).mock.calls.at(-1)!; + 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', () => { + it('reports it, changes nothing, and stops pending', async () => { + vi.mocked(getClusterInfo).mockRejectedValue(new Error('503')); + + const result = await save(); + + expect(events).toEqual([]); + expect(result.current.isPending).toBe(false); + expect(toast.error).toHaveBeenCalledWith( + 'Error', + expect.objectContaining({ description: expect.stringContaining('no configuration was changed') }), + ); + }); +}); + +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/useRollingConfigUpdate.tsx b/src/hooks/useRollingConfigUpdate.tsx index 3d1ccee48..12e8fe22a 100644 --- a/src/hooks/useRollingConfigUpdate.tsx +++ b/src/hooks/useRollingConfigUpdate.tsx @@ -1,17 +1,17 @@ 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'; -import { setConfiguration } from '@/integrations/api/instance/status/setConfiguration'; +import { setConfiguration, shouldFanOutSetConfiguration } from '@/integrations/api/instance/status/setConfiguration'; +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'; import { toast } from 'sonner'; @@ -24,6 +24,41 @@ interface RollingConfigUpdateResponse { isPending: boolean; } +const MAY_BE_PENDING = + 'Instances that were not restarted may already have the new configuration saved, and will apply it the next time they restart.'; + +interface RestartTarget { + instanceClient: AxiosInstance; + replicationFqdn?: string; +} + +interface RollingConfigUpdatePlan { + allInstances: Instance[]; + targets: RestartTarget[]; + fanOutAfterCanary: boolean; +} + +async function planRollingConfigUpdate( + operationsParams: InstanceClientIdConfig & InstanceTypeConfig, +): Promise { + if (operationsParams.entityType !== 'cluster') { + return { + allInstances: [{ id: operationsParams.entityId, status: 'RUNNING' } as Instance], + targets: [{ instanceClient: operationsParams.instanceClient }], + fanOutAfterCanary: false, + }; + } + const { allInstances, targets } = await getClusterRestartTargets(operationsParams.entityId); + return { + allInstances, + targets: targets.map(({ instance, instanceClient }) => ({ + instanceClient, + replicationFqdn: instance.replicationFqdn, + })), + fanOutAfterCanary: shouldFanOutSetConfiguration(allInstances), + }; +} + export function useRollingConfigUpdate( { onRestartedSuccessfully }: RollingConfigUpdateParams = {}, ): RollingConfigUpdateResponse { @@ -54,119 +89,148 @@ export function useRollingConfigUpdate( ), }); - const cluster = operationsParams.entityType === 'cluster' - ? await getClusterInfo(operationsParams.entityId) - : null; - const allInstances = operationsParams.entityType === 'cluster' - ? cluster?.instances || [] - : [{ id: operationsParams.entityId, status: 'RUNNING' } as Instance]; - const clusterUsesFabricConnect = !!cluster && authStore.checkForFabricConnect(cluster.id); - const instanceClients = operationsParams.entityType === 'cluster' - ? allInstances - .filter(instance => instance.status === 'RUNNING') - .map(instance => - getInstanceClient({ - id: instance.id, - forceFabricConnect: clusterUsesFabricConnect, - operationsUrl: getOperationsUrlForInstance(instance), - }) - ) - .reverse() - : [operationsParams.instanceClient]; - let instancesRestarted = 0; + try { + let plan: RollingConfigUpdatePlan; + try { + plan = await planRollingConfigUpdate(operationsParams); + } catch { + toast.error('Error', { + id: toastId, + description: "Could not load the cluster's instances, so no configuration was changed.", + duration: 10_000, + action: { + label: 'Dismiss', + onClick: () => toast.dismiss(), + }, + }); + return; + } + const { allInstances, targets, fanOutAfterCanary } = plan; + let fanOutAttempted = false; + let acknowledged = new Set(); + let instancesRestarted = 0; - if (instanceClients.length) { - for (let i = 0; i < instanceClients.length; i++) { - const instanceClient = instanceClients[i]; - if (!canceled) { - toast.loading(`Updating Instance ${i + 1} of ${instanceClients.length}`, { - ...toastConfig, - id: toastId, - description: ( - - ), + for (let i = 0; i < targets.length; i++) { + const { instanceClient, replicationFqdn } = targets[i]; + if (canceled) { + break; + } + toast.loading(`Updating Instance ${i + 1} of ${targets.length}`, { + ...toastConfig, + id: toastId, + description: ( + + ), + }); + let restarted = false; + try { + // Make sure the instance is responding. + await getInstanceUserInfo({ + instanceClient, }); - try { - // Make sure the instance is responding. - await getInstanceUserInfo({ - instanceClient, - }); + if (canceled) { + break; + } + if (!replicationFqdn || !acknowledged.has(replicationFqdn.toLowerCase())) { await setConfiguration({ ...params, instanceClient, }); - // Then restart it. - await restartInstance({ - operation: 'restart_service', - replicated: false, - instanceClient, + } + // Then restart it. + await restartInstance({ + operation: 'restart_service', + replicated: false, + instanceClient, + }); + instancesRestarted += 1; + restarted = true; + } 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.', }); - 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.', - }); - await sleep(3000); - } + await sleep(3000); } } - } - } - setIsPending(false); - void queryClient.invalidateQueries({ queryKey: ['get_configuration', operationsParams.entityId] }); + // The configuration spreads only once an instance has come back up running it, and a + // later target skips its own write only if replication acknowledged that exact node. + if (restarted && fanOutAfterCanary && !fanOutAttempted && !canceled) { + fanOutAttempted = true; + toast.loading('Saving configuration to the cluster', { + ...toastConfig, + id: toastId, + description: ( + + ), + }); + acknowledged = await setConfiguration({ + ...params, + replicated: true, + instanceClient, + }).then(acknowledgedReplicationNodes, () => new Set()); + } + } - if (canceled) { - toast.error('Cancelled', { - id: toastId, - description: `The config update was partially cancelled.`, - duration: 10_000, - action: { - label: 'Dismiss', - onClick: () => toast.dismiss(), - }, - }); - } else if (allInstances.length === instancesRestarted) { - onRestartedSuccessfully?.(); - toast.success('Success', { - id: toastId, - description: `Your configuration has been updated successfully!`, - 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 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}.`, - 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 config update was partially cancelled.${fanOutAttempted ? ` ${MAY_BE_PENDING}` : ''}`, + duration: 10_000, + action: { + label: 'Dismiss', + onClick: () => toast.dismiss(), + }, + }); + } else if (allInstances.length > 0 && allInstances.length === instancesRestarted) { + onRestartedSuccessfully?.(); + toast.success('Success', { + id: toastId, + description: `Your configuration has been updated successfully!`, + 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 update cluster.\n` + + ([ + allInstances.length === 0 && 'No instances were found within the cluster to restart.', + targets.length === 0 && `No instances were in a "RUNNING" state of ${allTheInstances}.`, + allInstances.length !== instancesRestarted + && `Only ${someRunningInstancesWere} restarted of ${allTheInstances}.`, + ].filter(excludeFalsy).shift() || '') + + (fanOutAttempted ? '\n' + MAY_BE_PENDING : ''), + duration: 10_000, + action: { + label: 'Dismiss', + onClick: () => toast.dismiss(), + }, + }); + } + } finally { + setIsPending(false); + void queryClient.invalidateQueries({ queryKey: ['get_configuration', operationsParams.entityId] }); } }, [operationsParams, onRestartedSuccessfully, queryClient]); diff --git a/src/integrations/api/instance/status/setConfiguration.test.ts b/src/integrations/api/instance/status/setConfiguration.test.ts new file mode 100644 index 000000000..7c7c13dd9 --- /dev/null +++ b/src/integrations/api/instance/status/setConfiguration.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it } from 'vitest'; +import { shouldFanOutSetConfiguration } from './setConfiguration'; + +describe('shouldFanOutSetConfiguration', () => { + it('is true when every live instance runs 5.2.0 or later', () => { + expect(shouldFanOutSetConfiguration([ + { status: 'RUNNING', version: '5.2.0' }, + { status: 'RUNNING', version: '5.3.1' }, + { status: 'STOPPED', version: '5.2.13' }, + ])).toBe(true); + }); + + it('includes exactly 5.2.0', () => { + expect(shouldFanOutSetConfiguration([ + { status: 'RUNNING', version: '5.2.0' }, + { status: 'RUNNING', version: '5.2.0' }, + ])).toBe(true); + }); + + it('is false when any live instance is older', () => { + expect(shouldFanOutSetConfiguration([ + { status: 'RUNNING', version: '5.2.0' }, + { status: 'RUNNING', version: '5.1.29' }, + ])).toBe(false); + }); + + it('is false for a 5.2.0 prerelease', () => { + expect(shouldFanOutSetConfiguration([ + { status: 'RUNNING', version: '5.2.0' }, + { status: 'RUNNING', version: '5.2.0-beta.4' }, + ])).toBe(false); + }); + + it('is false when any live instance reports no version', () => { + expect(shouldFanOutSetConfiguration([ + { status: 'RUNNING', version: '5.2.0' }, + { status: 'RUNNING', version: undefined }, + ])).toBe(false); + }); + + it('ignores instances that are leaving the cluster', () => { + expect(shouldFanOutSetConfiguration([ + { status: 'RUNNING', version: '5.2.0' }, + { status: 'STOPPED', version: '5.2.0' }, + { status: 'TERMINATING', version: '5.1.29' }, + { status: 'TERMINATED', version: undefined }, + { status: 'REMOVED', version: '4.7.36' }, + ])).toBe(true); + }); + + it('is false with nothing else live to reach', () => { + expect(shouldFanOutSetConfiguration([])).toBe(false); + expect(shouldFanOutSetConfiguration([{ status: 'RUNNING', version: '5.2.0' }])).toBe(false); + expect(shouldFanOutSetConfiguration([ + { status: 'RUNNING', version: '5.2.0' }, + { status: 'TERMINATED', version: '5.2.0' }, + ])).toBe(false); + }); +}); diff --git a/src/integrations/api/instance/status/setConfiguration.ts b/src/integrations/api/instance/status/setConfiguration.ts index 17f4b9530..bb8b55062 100644 --- a/src/integrations/api/instance/status/setConfiguration.ts +++ b/src/integrations/api/instance/status/setConfiguration.ts @@ -1,5 +1,29 @@ +import { deletedClusterStatuses } from '@/config/clusterStatuses'; import { InstanceClientConfig } from '@/config/instanceClientConfig'; +import { Instance } from '@/integrations/api/api.patch'; import { ReplicatedResponse } from '@/integrations/api/replication'; +import { wasAReleasedBeforeB } from '@/lib/string/wasAReleasedBeforeB'; + +/** + * The first Harper release whose `set_configuration` takes `replicated: true` and fans the write out + * to every replication peer (HarperFast/harper#1556). 5.0 and 5.1 apply the change locally and report + * no peers. Prereleases of 5.2.0 read as unsupported. + */ +const REPLICATED_SET_CONFIGURATION_MIN_VERSION = '5.2.0'; + +/** + * Whether a cluster-wide config save should fan its write out: some other live member exists to + * reach — including ones Studio will not restart — and every live member runs a Harper that can. + */ +export function shouldFanOutSetConfiguration( + instances: ReadonlyArray>, +): boolean { + const live = instances.filter(instance => !(instance.status && deletedClusterStatuses.includes(instance.status))); + return live.length > 1 + && live.every(instance => + !!instance.version && wasAReleasedBeforeB(REPLICATED_SET_CONFIGURATION_MIN_VERSION, instance.version) + ); +} interface SetConfigurationParams extends InstanceClientConfig { [key: string]: unknown; diff --git a/src/integrations/api/replication.test.ts b/src/integrations/api/replication.test.ts index b87ff81d1..4cc4f7bb6 100644 --- a/src/integrations/api/replication.test.ts +++ b/src/integrations/api/replication.test.ts @@ -1,4 +1,5 @@ import { + acknowledgedReplicationNodes, isReplicatedResponseFailure, isReplicatedResponseSuccess, rejectReplicationFailures, @@ -127,7 +128,52 @@ describe('rejectReplicationFailures', () => { }); }); -function success(node: string, requestId = 1, message = true): ReplicatedResponseSuccess { +describe('acknowledgedReplicationNodes', () => { + it('names every peer that applied the operation, lower-cased', () => { + expect( + acknowledgedReplicationNodes({ message: 'ok', replicated: [answered('Node-A.example.com'), answered('node-b')] }), + ) + .toEqual(new Set(['node-a.example.com', 'node-b'])); + }); + + it('leaves out failed peers', () => { + expect( + acknowledgedReplicationNodes({ message: 'ok', replicated: [answered('node-a'), failure('node-b', 'timeout')] }), + ) + .toEqual(new Set(['node-a'])); + }); + + it.each([ + ['no replicated array', { message: 'ok' }], + ['a non-array replicated', { message: 'ok', replicated: 'yes' }], + ['a bare string', 'Configuration successfully set.'], + ['undefined', undefined], + ['null', null], + [ + 'entries with no usable node name', + { message: 'ok', replicated: [null, 7, {}, { node: '', message: 'ok' }, { node: 42, message: 'ok' }] }, + ], + ['entries with no answer from the peer', { + message: 'ok', + replicated: [{ node: 'node-a' }, { node: 'node-b', requestId: 1 }], + }], + ['entries whose answer is not a message', { + message: 'ok', + replicated: [{ node: 'a', message: {} }, { node: 'b', message: 0 }, { node: 'c', message: true }, { + node: 'd', + message: '', + }], + }], + ])('reads %s as reaching no one', (_label, response) => { + expect(acknowledgedReplicationNodes(response)).toEqual(new Set()); + }); +}); + +function answered(node: string): ReplicatedResponseSuccess { + return success(node, 1, 'Configuration successfully set.'); +} + +function success(node: string, requestId = 1, message: boolean | string = true): ReplicatedResponseSuccess { return { node, requestId, message }; } diff --git a/src/integrations/api/replication.ts b/src/integrations/api/replication.ts index a37348309..bf56c7cfe 100644 --- a/src/integrations/api/replication.ts +++ b/src/integrations/api/replication.ts @@ -12,7 +12,7 @@ interface ReplicatedResponseNarrowed extends ReplicatedResponse { export interface ReplicatedResponseSuccess { node: string; - message: boolean; + message: boolean | string; requestId: number; } @@ -38,6 +38,28 @@ export function isReplicatedResponseFailure( return (message as ReplicatedResponseFailure).status === 'failed'; } +/** + * The lower-cased names of the peers a replicated operation reports as applied: entries carrying the + * peer's own answer (a `message` string) and the `node` Harper attached to it. Anything else is left + * out, so an unrecognized response reads as reaching no one. + */ +export function acknowledgedReplicationNodes(response: unknown): Set { + const acknowledged = new Set(); + const replicated: unknown = (response as ReplicatedResponse | null | undefined)?.replicated; + if (!Array.isArray(replicated)) { + return acknowledged; + } + for (const entry of replicated) { + if ( + entry && typeof entry === 'object' && isReplicatedResponseSuccess(entry) + && typeof entry.message === 'string' && entry.message && typeof entry.node === 'string' && entry.node + ) { + acknowledged.add(entry.node.toLowerCase()); + } + } + return acknowledged; +} + export function rejectReplicationFailures(response: Pick, 'data'>) { if (replicationFailed(response.data)) { const successes = response.data.replicated.filter(isReplicatedResponseSuccess);