diff --git a/hasura/metadata/actions.graphql b/hasura/metadata/actions.graphql index bc7c827a..87f006a3 100644 --- a/hasura/metadata/actions.graphql +++ b/hasura/metadata/actions.graphql @@ -127,6 +127,10 @@ type Mutation { ): SuccessOutput } +type Mutation { + cleanupRemovedNodes: CleanupRemovedNodesOutput +} + type Mutation { clearClipRenderBatch( match_map_id: uuid! @@ -1168,6 +1172,16 @@ type SetupGameServeOutput { gameServerId: String! } +type CleanupRemovedNodesOutput { + nodes: Int! + jobs: Int! + volume_claims: Int! + volumes: Int! + failed: Int! + node_delete_forbidden: Boolean! + recently_ready: Int! +} + type SampleOutput { accessToken: String! } diff --git a/hasura/metadata/actions.yaml b/hasura/metadata/actions.yaml index 40f20567..d43f46dd 100644 --- a/hasura/metadata/actions.yaml +++ b/hasura/metadata/actions.yaml @@ -222,6 +222,14 @@ actions: permissions: - role: user comment: checkIntoMatch + - name: cleanupRemovedNodes + definition: + kind: synchronous + handler: '{{HASURA_GRAPHQL_ACTIONS_HOOK}}' + forward_client_headers: true + permissions: + - role: administrator + comment: Delete the Jobs, volume claims, volumes and k8s Nodes left behind by removed game server nodes - name: clearClipRenderBatch definition: kind: synchronous @@ -2084,6 +2092,7 @@ custom_types: - name: DeleteOrphansOutput - name: WatchDemoOutput - name: SetupGameServeOutput + - name: CleanupRemovedNodesOutput - name: SampleOutput - name: CpuStat - name: MemoryStat diff --git a/hasura/metadata/databases/default/tables/public_game_server_nodes.yaml b/hasura/metadata/databases/default/tables/public_game_server_nodes.yaml index f8c5995f..5f6cfc20 100644 --- a/hasura/metadata/databases/default/tables/public_game_server_nodes.yaml +++ b/hasura/metadata/databases/default/tables/public_game_server_nodes.yaml @@ -159,6 +159,16 @@ event_triggers: num_retries: 6 timeout_sec: 60 webhook: '{{HASURA_GRAPHQL_EVENT_HOOK}}' + - name: game_server_node_removed + definition: + delete: + columns: '*' + enable_manual: false + retry_conf: + interval_sec: 10 + num_retries: 6 + timeout_sec: 60 + webhook: '{{HASURA_GRAPHQL_EVENT_HOOK}}' - name: node_server_availability definition: enable_manual: false diff --git a/src/game-server-node/game-server-node.controller.spec.ts b/src/game-server-node/game-server-node.controller.spec.ts index ed2d9843..0e31977f 100644 --- a/src/game-server-node/game-server-node.controller.spec.ts +++ b/src/game-server-node/game-server-node.controller.spec.ts @@ -1,4 +1,6 @@ import { GameServerNodeController } from "./game-server-node.controller"; +import { CleanupRemovedNode } from "./jobs/CleanupRemovedNode"; +import { CleanupRemovedNodesOutput } from "./node-cleanup.service"; describe("GameServerNodeController ping disk alerts", () => { let gameServerNodeService: { updateStatus: jest.Mock }; @@ -32,6 +34,7 @@ describe("GameServerNodeController ping disk alerts", () => { queue as any, queue as any, {} as any, + {} as any, ); }); @@ -69,3 +72,127 @@ describe("GameServerNodeController ping disk alerts", () => { expect(notifications.send).not.toHaveBeenCalled(); }); }); + +describe("GameServerNodeController removed node cleanup", () => { + let nodeCleanup: { cleanupRemovedNodes: jest.Mock }; + let nodeOfflineQueue: { add: jest.Mock; remove: jest.Mock }; + let controller: GameServerNodeController; + + const result = (counts: Partial = {}) => ({ + nodes: 0, + jobs: 0, + volume_claims: 0, + volumes: 0, + failed: 0, + node_delete_forbidden: false, + recently_ready: 0, + ...counts, + }); + + const removed = (old: Record) => + controller.game_server_node_removed({ + op: "DELETE", + old, + new: {}, + } as any); + + beforeEach(() => { + nodeCleanup = { + cleanupRemovedNodes: jest.fn().mockResolvedValue(result()), + }; + nodeOfflineQueue = { + add: jest.fn().mockResolvedValue({}), + remove: jest.fn().mockResolvedValue(1), + }; + + controller = new GameServerNodeController( + {} as any, + {} as any, + { get: jest.fn().mockReturnValue({}) } as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + {} as any, + nodeOfflineQueue as any, + {} as any, + {} as any, + {} as any, + {} as any, + nodeCleanup as any, + ); + }); + + it("queues a retried cleanup of the deleted row's node instead of cleaning inline", async () => { + await expect(removed({ id: "node-1" })).resolves.toBeUndefined(); + + expect(nodeOfflineQueue.add).toHaveBeenCalledTimes(1); + expect(nodeOfflineQueue.add).toHaveBeenCalledWith( + CleanupRemovedNode.name, + { nodeId: "node-1" }, + { + attempts: 6, + backoff: { type: "exponential", delay: 10 * 1000 }, + removeOnFail: false, + removeOnComplete: true, + jobId: "node-cleanup.node-1", + }, + ); + + // HasuraController swallows handler errors, so the job does the cleanup + expect(nodeCleanup.cleanupRemovedNodes).not.toHaveBeenCalled(); + }); + + it("removes an earlier cleanup job of the node before queueing, since a taken job id makes the add a no-op", async () => { + await removed({ id: "node-1" }); + + expect(nodeOfflineQueue.remove).toHaveBeenCalledWith("node-cleanup.node-1"); + expect(nodeOfflineQueue.remove.mock.invocationCallOrder[0]).toBeLessThan( + nodeOfflineQueue.add.mock.invocationCallOrder[0], + ); + }); + + it("queues a job of its own when the earlier cleanup of the node is running, since a running job cannot be removed", async () => { + nodeOfflineQueue.remove.mockResolvedValue(0); + + await removed({ id: "node-1" }); + + expect(nodeOfflineQueue.add).toHaveBeenCalledWith( + CleanupRemovedNode.name, + { nodeId: "node-1" }, + expect.objectContaining({ + jobId: expect.stringMatching(/^node-cleanup\.node-1\.\d+$/), + }), + ); + }); + + it("ignores an event without the id of the deleted row", async () => { + await expect(removed({})).resolves.toBeUndefined(); + + expect(nodeOfflineQueue.remove).not.toHaveBeenCalled(); + expect(nodeOfflineQueue.add).not.toHaveBeenCalled(); + expect(nodeCleanup.cleanupRemovedNodes).not.toHaveBeenCalled(); + }); + + it("returns the sweep result from the admin action", async () => { + nodeCleanup.cleanupRemovedNodes.mockResolvedValue( + result({ nodes: 2, failed: 1, node_delete_forbidden: true }), + ); + + // HasuraController binds the action input the way it does here, and that + // input must never be taken for a node id + const action = (controller as any).cleanupRemovedNodes.bind(controller, { + user: { role: "administrator" }, + session: {}, + }); + + await expect(action()).resolves.toEqual( + result({ nodes: 2, failed: 1, node_delete_forbidden: true }), + ); + + expect(nodeCleanup.cleanupRemovedNodes).toHaveBeenCalledWith(); + }); +}); diff --git a/src/game-server-node/game-server-node.controller.ts b/src/game-server-node/game-server-node.controller.ts index b30782ec..72fd5e35 100644 --- a/src/game-server-node/game-server-node.controller.ts +++ b/src/game-server-node/game-server-node.controller.ts @@ -1,6 +1,7 @@ import { Controller, Get, Logger, Req, Res } from "@nestjs/common"; import { HasuraAction, HasuraEvent } from "../hasura/hasura.controller"; import { GameServerNodeService } from "./game-server-node.service"; +import { NodeCleanupService } from "./node-cleanup.service"; import { TailscaleService } from "../tailscale/tailscale.service"; import { HasuraService } from "../hasura/hasura.service"; import { InjectQueue } from "@nestjs/bullmq"; @@ -24,6 +25,7 @@ import { NodeStats } from "./interfaces/NodeStats"; import { PodStats } from "./interfaces/PodStats"; import { MarkGameServerNodeOffline } from "./jobs/MarkGameServerNodeOffline"; import { MarkGameServerNodeOnline } from "./jobs/MarkGameServerNodeOnline"; +import { CleanupRemovedNode } from "./jobs/CleanupRemovedNode"; import { HasuraEventData } from "src/hasura/types/HasuraEventData"; import { game_server_nodes_set_input } from "generated/schema"; import { NotificationsService } from "../notifications/notifications.service"; @@ -59,6 +61,7 @@ export class GameServerNodeController { @InjectQueue(GameServerQueues.ValidateGamedata) private readonly validateGamedataQueue: Queue, protected readonly mapAssets: MapAssetsService, + protected readonly nodeCleanup: NodeCleanupService, ) { this.appConfig = this.config.get("app"); } @@ -813,6 +816,46 @@ UNIT ); } + @HasuraAction() + public async cleanupRemovedNodes() { + return await this.nodeCleanup.cleanupRemovedNodes(); + } + + @HasuraEvent() + public async game_server_node_removed( + data: HasuraEventData, + ) { + const nodeId = data.old?.id; + if (!nodeId) { + return; + } + + // HasuraController answers every event with a success, so an error thrown + // here would never be retried. The job retries instead. A waiting or failed + // job of the node is replaced, which restarts its checks, since a taken job + // id makes the add a no-op. A running one cannot be removed, so this + // removal gets a job of its own. + const jobId = `node-cleanup.${nodeId}`; + const removed = await this.nodeOfflineQueue.remove(jobId); + + await this.nodeOfflineQueue.add( + CleanupRemovedNode.name, + { + nodeId, + }, + { + attempts: 6, + backoff: { + type: "exponential", + delay: 10 * 1000, + }, + removeOnFail: false, + removeOnComplete: true, + jobId: removed === 0 ? `${jobId}.${Date.now()}` : jobId, + }, + ); + } + @Get("/ping/:serverId") public async ping(@Req() request: Request) { const map = request.query.map; diff --git a/src/game-server-node/game-server-node.module.ts b/src/game-server-node/game-server-node.module.ts index 004340a3..83350d20 100644 --- a/src/game-server-node/game-server-node.module.ts +++ b/src/game-server-node/game-server-node.module.ts @@ -35,10 +35,13 @@ import { BakeShaders } from "./jobs/BakeShaders"; import { ValidateGamedata } from "./jobs/ValidateGamedata"; import { MapAssetsModule } from "src/map-assets/map-assets.module"; import { PostgresModule } from "src/postgres/postgres.module"; +import { NodeCleanupService } from "./node-cleanup.service"; +import { CleanupRemovedNode } from "./jobs/CleanupRemovedNode"; @Module({ providers: [ GameServerNodeService, + NodeCleanupService, CheckGameUpdate, GetPluginVersions, MarkGameServerNodeOffline, @@ -47,6 +50,7 @@ import { PostgresModule } from "src/postgres/postgres.module"; CheckServerPluginVersions, BakeShaders, ValidateGamedata, + CleanupRemovedNode, ...getQueuesProcessors("GameServerNode"), loggerFactory(), ], diff --git a/src/game-server-node/game-server-node.service.ts b/src/game-server-node/game-server-node.service.ts index 824ddcbc..39779240 100644 --- a/src/game-server-node/game-server-node.service.ts +++ b/src/game-server-node/game-server-node.service.ts @@ -312,6 +312,12 @@ export class GameServerNodeService { this.logger.log(`Creating volumes for node ${node}`); await this.createVolumes(node); } + // A node that registers again after the cleanup of its removal still has + // its CS:GO install on disk, but no volume for it anymore. + if (csgoBuildId && !game_server_nodes_by_pk) { + this.logger.log(`Creating CS:GO volume for node ${node}`); + await this.createVolumes(node, "csgo"); + } if (game_server_nodes_by_pk?.status === "NotAcceptingNewMatches") { status = "NotAcceptingNewMatches"; } diff --git a/src/game-server-node/game-server-node.volumes.spec.ts b/src/game-server-node/game-server-node.volumes.spec.ts new file mode 100644 index 00000000..dfa41746 --- /dev/null +++ b/src/game-server-node/game-server-node.volumes.spec.ts @@ -0,0 +1,102 @@ +jest.mock("@kubernetes/client-node", () => ({ + BatchV1Api: class BatchV1Api {}, + CoreV1Api: class CoreV1Api {}, + KubeConfig: class KubeConfig { + loadFromDefault() {} + makeApiClient(ctor: new () => unknown) { + return new ctor(); + } + }, +})); + +import { GameServerNodeService } from "./game-server-node.service"; + +describe("GameServerNodeService volumes of a registering node", () => { + const NODE_ID = "a1b2c3d4"; + const CS_BUILD = 21000000; + const CSGO_BUILD = 7000000; + + let service: GameServerNodeService; + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let createVolumes: jest.SpyInstance; + let create: jest.SpyInstance; + + const ping = (csgoBuild: number | undefined) => + service.updateStatus( + NODE_ID, + "10.0.0.2", + "192.168.1.2", + "203.0.113.2", + CS_BUILD, + csgoBuild, + false, + false, + { sockets: 1, coresPerSocket: 8, threadsPerCore: 2 }, + { governor: "performance", cpus: {} }, + { cpus: {}, frequency: 0 }, + undefined, + "Online", + ); + + beforeEach(() => { + hasura = { + query: jest.fn().mockResolvedValue({ game_server_nodes_by_pk: null }), + mutation: jest.fn().mockResolvedValue({}), + }; + + service = new GameServerNodeService( + { log: jest.fn(), warn: jest.fn(), error: jest.fn() } as any, + { + get: (key: string) => + key === "gameServers" ? { namespace: "5stack" } : {}, + } as any, + hasura as any, + { getConnection: () => ({}) } as any, + {} as any, + {} as any, + {} as any, + {} as any, + { query: jest.fn().mockResolvedValue([]) } as any, + {} as any, + ); + + createVolumes = jest + .spyOn(service as any, "createVolumes") + .mockResolvedValue(undefined); + create = jest.spyOn(service, "create").mockResolvedValue(undefined); + }); + + it("creates the CS:GO volume too for a node without a row that still has its CS:GO install", async () => { + await ping(CSGO_BUILD); + + expect(createVolumes).toHaveBeenCalledTimes(2); + expect(createVolumes).toHaveBeenCalledWith(NODE_ID); + expect(createVolumes).toHaveBeenCalledWith(NODE_ID, "csgo"); + expect(create).toHaveBeenCalledWith(undefined, NODE_ID, "Online"); + }); + + it("creates no CS:GO volume for a node without a CS:GO install", async () => { + await ping(undefined); + + expect(createVolumes).toHaveBeenCalledTimes(1); + expect(createVolumes).toHaveBeenCalledWith(NODE_ID); + }); + + it("creates no volumes for a node that has a row", async () => { + hasura.query.mockResolvedValue({ + game_server_nodes_by_pk: { + status: "Online", + build_id: CS_BUILD, + csgo_build_id: CSGO_BUILD, + update_status: null, + enabled: true, + servers: [], + }, + }); + + await ping(CSGO_BUILD); + + expect(createVolumes).not.toHaveBeenCalled(); + expect(create).not.toHaveBeenCalled(); + }); +}); diff --git a/src/game-server-node/jobs/CleanupRemovedNode.spec.ts b/src/game-server-node/jobs/CleanupRemovedNode.spec.ts new file mode 100644 index 00000000..e212e181 --- /dev/null +++ b/src/game-server-node/jobs/CleanupRemovedNode.spec.ts @@ -0,0 +1,166 @@ +import { DelayedError } from "bullmq"; +import { CleanupRemovedNode } from "./CleanupRemovedNode"; +import { + CleanupRemovedNodesOutput, + NodeCleanupService, +} from "../node-cleanup.service"; + +describe("CleanupRemovedNode", () => { + let nodeCleanup: { cleanupRemovedNodes: jest.Mock }; + let logger: { warn: jest.Mock }; + let job: CleanupRemovedNode; + + const result = (counts: Partial = {}) => ({ + nodes: 0, + jobs: 0, + volume_claims: 0, + volumes: 0, + failed: 0, + node_delete_forbidden: false, + recently_ready: 0, + ...counts, + }); + + const queueJob = (data: { nodeId: string; readyChecks?: number }) => ({ + data, + token: "token-1", + updateData: jest.fn().mockResolvedValue(undefined), + moveToDelayed: jest.fn().mockResolvedValue(undefined), + }); + + const run = (queued = queueJob({ nodeId: "node-1" })) => + job.process(queued as any); + + beforeEach(() => { + nodeCleanup = { + cleanupRemovedNodes: jest.fn().mockResolvedValue(result()), + }; + logger = { warn: jest.fn() }; + job = new CleanupRemovedNode(logger as any, nodeCleanup as any); + }); + + it("cleans up only the removed node", async () => { + nodeCleanup.cleanupRemovedNodes.mockResolvedValue( + result({ nodes: 1, jobs: 2, volume_claims: 4, volumes: 4 }), + ); + + await expect(run()).resolves.toBeUndefined(); + + expect(nodeCleanup.cleanupRemovedNodes).toHaveBeenCalledTimes(1); + expect(nodeCleanup.cleanupRemovedNodes).toHaveBeenCalledWith("node-1"); + }); + + it("throws when a delete failed, so the queue retries the cleanup", async () => { + nodeCleanup.cleanupRemovedNodes.mockResolvedValue( + result({ jobs: 1, failed: 2 }), + ); + + await expect(run()).rejects.toThrow( + "unable to clean up removed node node-1: 2 delete(s) failed", + ); + }); + + it("rethrows when the cluster could not be read, so the queue retries the cleanup", async () => { + nodeCleanup.cleanupRemovedNodes.mockRejectedValue( + new Error("unable to list cluster objects"), + ); + + await expect(run()).rejects.toThrow("unable to list cluster objects"); + }); + + it("succeeds when the node registered again and nothing was deleted", async () => { + // the service skips an id whose row is back + await expect(run()).resolves.toBeUndefined(); + }); + + it("does not retry a Node delete the RBAC forbids, since retries cannot fix it", async () => { + nodeCleanup.cleanupRemovedNodes.mockResolvedValue( + result({ + jobs: 2, + volume_claims: 1, + volumes: 1, + node_delete_forbidden: true, + }), + ); + + await expect(run()).resolves.toBeUndefined(); + }); + + it("checks a recently Ready node again later, without failing the job", async () => { + nodeCleanup.cleanupRemovedNodes.mockResolvedValue( + result({ recently_ready: 1 }), + ); + const queued = queueJob({ nodeId: "node-1", readyChecks: 2 }); + + await expect(run(queued)).rejects.toBeInstanceOf(DelayedError); + + expect(queued.updateData).toHaveBeenCalledWith({ + nodeId: "node-1", + readyChecks: 3, + }); + expect(queued.moveToDelayed).toHaveBeenCalledWith( + expect.any(Number), + "token-1", + ); + const [delayedUntil] = queued.moveToDelayed.mock.calls[0]; + expect(delayedUntil - Date.now()).toBeGreaterThan( + CleanupRemovedNode.READY_CHECK_DELAY_MS - 5000, + ); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it("keeps checking for twice the NotReady grace of the service", () => { + expect( + CleanupRemovedNode.READY_CHECKS * CleanupRemovedNode.READY_CHECK_DELAY_MS, + ).toBeGreaterThanOrEqual(2 * NodeCleanupService.NOT_READY_GRACE_MS); + }); + + it("still runs the last check before giving up", async () => { + nodeCleanup.cleanupRemovedNodes.mockResolvedValue( + result({ recently_ready: 1 }), + ); + const queued = queueJob({ + nodeId: "node-1", + readyChecks: CleanupRemovedNode.READY_CHECKS - 1, + }); + + await expect(run(queued)).rejects.toBeInstanceOf(DelayedError); + + expect(queued.updateData).toHaveBeenCalledWith({ + nodeId: "node-1", + readyChecks: CleanupRemovedNode.READY_CHECKS, + }); + expect(queued.moveToDelayed).toHaveBeenCalledTimes(1); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it("gives up quietly once the node stayed recently Ready for every check", async () => { + nodeCleanup.cleanupRemovedNodes.mockResolvedValue( + result({ recently_ready: 1 }), + ); + const queued = queueJob({ + nodeId: "node-1", + readyChecks: CleanupRemovedNode.READY_CHECKS, + }); + + await expect(run(queued)).resolves.toBeUndefined(); + + expect(queued.moveToDelayed).not.toHaveBeenCalled(); + expect(logger.warn).toHaveBeenCalledWith( + expect.stringContaining( + "node-1 is still Ready or went NotReady too recently", + ), + ); + }); + + it("retries failed deletes before checking the Ready state", async () => { + nodeCleanup.cleanupRemovedNodes.mockResolvedValue( + result({ failed: 1, recently_ready: 1 }), + ); + const queued = queueJob({ nodeId: "node-1" }); + + await expect(run(queued)).rejects.toThrow("1 delete(s) failed"); + + expect(queued.moveToDelayed).not.toHaveBeenCalled(); + }); +}); diff --git a/src/game-server-node/jobs/CleanupRemovedNode.ts b/src/game-server-node/jobs/CleanupRemovedNode.ts new file mode 100644 index 00000000..730261ee --- /dev/null +++ b/src/game-server-node/jobs/CleanupRemovedNode.ts @@ -0,0 +1,62 @@ +import { WorkerHost } from "@nestjs/bullmq"; +import { Logger } from "@nestjs/common"; +import { DelayedError, Job } from "bullmq"; +import { GameServerQueues } from "../enums/GameServerQueues"; +import { UseQueue } from "../../utilities/QueueProcessors"; +import { NodeCleanupService } from "../node-cleanup.service"; + +// Hasura is always told its event was handled, so the queue retries a cleanup +// that could not read the cluster or delete everything. Every attempt checks +// the row and the Ready state again, so a node that came back keeps what it has. +@UseQueue("GameServerNode", GameServerQueues.NodeOffline) +export class CleanupRemovedNode extends WorkerHost { + // A node removed right after it went down is still Ready at first, and the + // service then keeps it for NodeCleanupService.NOT_READY_GRACE_MS after k8s + // marks it NotReady. Checking for twice that grace cleans a node that goes + // NotReady within the grace after its removal. A node that is still running + // gets its row back from its next ping within 30 s. + public static readonly READY_CHECK_DELAY_MS = 60 * 1000; + public static readonly READY_CHECKS = Math.ceil( + (2 * NodeCleanupService.NOT_READY_GRACE_MS) / + CleanupRemovedNode.READY_CHECK_DELAY_MS, + ); + + constructor( + protected readonly logger: Logger, + protected readonly nodeCleanup: NodeCleanupService, + ) { + super(); + } + + async process( + job: Job<{ nodeId: string; readyChecks?: number }>, + ): Promise { + const { nodeId, readyChecks = 0 } = job.data; + + const { failed, recently_ready } = + await this.nodeCleanup.cleanupRemovedNodes(nodeId); + if (failed > 0) { + throw new Error( + `unable to clean up removed node ${nodeId}: ${failed} delete(s) failed`, + ); + } + + if (recently_ready === 0) { + return; + } + + if (readyChecks >= CleanupRemovedNode.READY_CHECKS) { + this.logger.warn( + `[node-cleanup] ${nodeId} is still Ready or went NotReady too recently, leaving it for the cleanup in the server settings`, + ); + return; + } + + await job.updateData({ ...job.data, readyChecks: readyChecks + 1 }); + await job.moveToDelayed( + Date.now() + CleanupRemovedNode.READY_CHECK_DELAY_MS, + job.token, + ); + throw new DelayedError(); + } +} diff --git a/src/game-server-node/node-cleanup.service.spec.ts b/src/game-server-node/node-cleanup.service.spec.ts new file mode 100644 index 00000000..de0d0662 --- /dev/null +++ b/src/game-server-node/node-cleanup.service.spec.ts @@ -0,0 +1,985 @@ +const listNode = jest.fn(); +const readNode = jest.fn(); +const deleteNode = jest.fn(); +const listPersistentVolume = jest.fn(); +const deletePersistentVolume = jest.fn(); +const listNamespacedPersistentVolumeClaim = jest.fn(); +const deleteNamespacedPersistentVolumeClaim = jest.fn(); +const listNamespacedJob = jest.fn(); +const deleteNamespacedJob = jest.fn(); + +jest.mock("@kubernetes/client-node", () => ({ + BatchV1Api: class BatchV1Api { + listNamespacedJob = listNamespacedJob; + deleteNamespacedJob = deleteNamespacedJob; + }, + CoreV1Api: class CoreV1Api { + listNode = listNode; + readNode = readNode; + deleteNode = deleteNode; + listPersistentVolume = listPersistentVolume; + deletePersistentVolume = deletePersistentVolume; + listNamespacedPersistentVolumeClaim = listNamespacedPersistentVolumeClaim; + deleteNamespacedPersistentVolumeClaim = + deleteNamespacedPersistentVolumeClaim; + }, + KubeConfig: class KubeConfig { + loadFromDefault() {} + makeApiClient(ctor: new () => unknown) { + return new ctor(); + } + }, +})); + +import { + CleanupRemovedNodesOutput, + NodeCleanupService, +} from "./node-cleanup.service"; + +describe("NodeCleanupService removed node cleanup", () => { + const NAMESPACE = "test"; + const NODE = "a1b2c3d4"; + const OTHER_NODE = "e5f6a7b8"; + + let service: NodeCleanupService; + let logger: { log: jest.Mock; warn: jest.Mock; error: jest.Mock }; + let hasura: { query: jest.Mock }; + let nodes: Array>; + let jobs: Array>; + let volumes: Array>; + let claims: Array>; + let rows: Array; + let deletes: Array; + + const result = (counts: Partial = {}) => ({ + nodes: 0, + jobs: 0, + volume_claims: 0, + volumes: 0, + failed: 0, + node_delete_forbidden: false, + recently_ready: 0, + ...counts, + }); + + const apiError = (code: number) => + Object.assign(new Error(`HTTP-Code: ${code}`), { code }); + + // Records every delete in call order, and rejects the ones named in + // `errors` with that status code. + const recordDeletes = + (kind: string, errors: Record = {}) => + async ({ name }: { name: string }) => { + deletes.push(`${kind} ${name}`); + if (errors[name]) { + throw apiError(errors[name]); + } + return {}; + }; + + const minutesAgo = (minutes: number) => + new Date(Date.now() - minutes * 60 * 1000); + + // Went to `ready` an hour ago unless told otherwise, so past the grace + // period. A null time leaves it out. + const node = ( + name: string, + ready: "True" | "False" | "Unknown", + metadata: Record = {}, + lastTransitionTime: Date | null = minutesAgo(60), + ) => ({ + metadata: { + name, + uid: `node-uid-${name}`, + labels: { "5stack-id": name }, + ...metadata, + }, + status: { + conditions: [ + { + type: "Ready", + status: ready, + ...(lastTransitionTime ? { lastTransitionTime } : {}), + }, + ], + }, + }); + + const hostnamePin = (values: Array) => ({ + nodeAffinity: { + requiredDuringSchedulingIgnoredDuringExecution: { + nodeSelectorTerms: [ + { + matchExpressions: [ + { key: "kubernetes.io/hostname", operator: "In", values }, + ], + }, + ], + }, + }, + }); + + const job = ( + name: string, + labels: Record, + podSpec: Record, + metadata: Record = {}, + ) => ({ + metadata: { name, uid: `job-uid-${name}`, labels, ...metadata }, + spec: { template: { metadata: { labels }, spec: podSpec } }, + }); + + const updateJob = (nodeId: string, game = "cs") => + job( + `update-${game}-server-${nodeId.replaceAll(".", "-")}`, + { app: "update-cs-server" }, + { affinity: hostnamePin([nodeId]) }, + ); + + // The api only labels the pod template of the validation Job. + const validateJob = (name: string, nodeId: string) => ({ + metadata: { name, uid: `job-uid-${name}` }, + spec: { + template: { + metadata: { labels: { app: "validate-gamedata" } }, + spec: { affinity: hostnamePin([nodeId]) }, + }, + }, + }); + + const streamerJob = (name: string, podSpec: Record) => + job(name, { app: "game-streamer", role: "live" }, podSpec); + + const volumeTerm = (values: Array, key = "5stack-id") => ({ + matchExpressions: [{ key, operator: "In", values }], + }); + + const volume = ( + name: string, + nodeId: string, + spec: Record = {}, + metadata: Record = {}, + ) => ({ + metadata: { name, uid: `pv-uid-${name}`, ...metadata }, + spec: { + storageClassName: "local-storage", + persistentVolumeReclaimPolicy: "Retain", + claimRef: { namespace: NAMESPACE, name: `${name}-claim` }, + nodeAffinity: { + required: { nodeSelectorTerms: [volumeTerm([nodeId])] }, + }, + ...spec, + }, + }); + + // A claim bound to `volumeName`, or pre-bound to it the way the api creates + // its own `-claim`. + const claim = ( + name: string, + volumeName = name.replace(/-claim$/, ""), + metadata: Record = {}, + ) => ({ + metadata: { + name, + namespace: NAMESPACE, + uid: `pvc-uid-${name}`, + ...metadata, + }, + spec: { volumeName }, + }); + + // What a node the api set up leaves behind: its two update Jobs and a + // volume with its claim. + const leftoversOf = (nodeId: string) => { + jobs.push(updateJob(nodeId), updateJob(nodeId, "csgo")); + volumes.push(volume(`demos-${nodeId}`, nodeId)); + claims.push(claim(`demos-${nodeId}-claim`)); + }; + + const deletesOfLeftovers = (nodeId: string) => [ + `job update-cs-server-${nodeId}`, + `job update-csgo-server-${nodeId}`, + `claim demos-${nodeId}-claim`, + `volume demos-${nodeId}`, + ]; + + beforeEach(() => { + for (const fn of [ + listNode, + readNode, + deleteNode, + listPersistentVolume, + deletePersistentVolume, + listNamespacedPersistentVolumeClaim, + deleteNamespacedPersistentVolumeClaim, + listNamespacedJob, + deleteNamespacedJob, + ]) { + fn.mockReset(); + } + + nodes = []; + jobs = []; + volumes = []; + claims = []; + rows = []; + deletes = []; + + listNode.mockImplementation(async () => ({ items: nodes })); + readNode.mockImplementation(async ({ name }: { name: string }) => { + const found = nodes.find((candidate) => candidate.metadata.name === name); + if (!found) { + throw apiError(404); + } + return found; + }); + listNamespacedJob.mockImplementation(async () => ({ items: jobs })); + listPersistentVolume.mockImplementation(async () => ({ items: volumes })); + listNamespacedPersistentVolumeClaim.mockImplementation(async () => ({ + items: claims, + })); + + deleteNamespacedJob.mockImplementation(recordDeletes("job")); + deleteNamespacedPersistentVolumeClaim.mockImplementation( + recordDeletes("claim"), + ); + deletePersistentVolume.mockImplementation(recordDeletes("volume")); + deleteNode.mockImplementation(recordDeletes("node")); + + logger = { log: jest.fn(), warn: jest.fn(), error: jest.fn() }; + hasura = { + query: jest.fn(async (request: any) => ({ + game_server_nodes: request.game_server_nodes.__args.where.id._in + .filter((id: string) => rows.includes(id)) + .map((id: string) => ({ id })), + })), + }; + + service = new NodeCleanupService( + logger as any, + { + get: jest.fn((key: string) => + key === "gameServers" ? { namespace: NAMESPACE } : {}, + ), + } as any, + hasura as any, + ); + }); + + it("cleans a removed NotReady node: its Node, then its Jobs, then each volume's claims before the volume", async () => { + nodes.push(node(NODE, "Unknown"), node(OTHER_NODE, "True")); + rows.push(OTHER_NODE); + jobs.push( + updateJob(NODE), + updateJob(NODE, "csgo"), + validateJob("validate-gamedata-21000000-public", NODE), + streamerJob("streamer-live-1", { affinity: hostnamePin([NODE]) }), + ); + volumes.push( + volume(`demos-${NODE}`, NODE), + volume(`steamcmd-${NODE}`, NODE), + // not bound yet, so only the api's claim, pre-bound to it, is its claim + volume(`serverfiles-${NODE}`, NODE, { claimRef: undefined }), + ); + claims.push( + claim(`demos-${NODE}-claim`), + claim(`steamcmd-${NODE}-claim`), + claim(`serverfiles-${NODE}-claim`), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ nodes: 1, jobs: 4, volume_claims: 3, volumes: 3 }), + ); + + expect(deletes).toEqual([ + `node ${NODE}`, + `job update-cs-server-${NODE}`, + `job update-csgo-server-${NODE}`, + "job validate-gamedata-21000000-public", + "job streamer-live-1", + `claim demos-${NODE}-claim`, + `volume demos-${NODE}`, + `claim steamcmd-${NODE}-claim`, + `volume steamcmd-${NODE}`, + `claim serverfiles-${NODE}-claim`, + `volume serverfiles-${NODE}`, + ]); + + expect(hasura.query).toHaveBeenCalledWith({ + game_server_nodes: { + __args: { where: { id: { _in: [NODE, OTHER_NODE] } } }, + id: true, + }, + }); + + // Jobs would orphan their pods without Background propagation, and the + // uid keeps each delete off a newer object with the same name. + expect(deleteNamespacedJob).toHaveBeenCalledWith({ + name: `update-cs-server-${NODE}`, + namespace: NAMESPACE, + body: { + propagationPolicy: "Background", + preconditions: { uid: `job-uid-update-cs-server-${NODE}` }, + }, + }); + expect(deleteNamespacedPersistentVolumeClaim).toHaveBeenCalledWith({ + name: `demos-${NODE}-claim`, + namespace: NAMESPACE, + body: { + propagationPolicy: "Background", + preconditions: { uid: `pvc-uid-demos-${NODE}-claim` }, + }, + }); + expect(deletePersistentVolume).toHaveBeenCalledWith({ + name: `demos-${NODE}`, + body: { + propagationPolicy: "Background", + preconditions: { uid: `pv-uid-demos-${NODE}` }, + }, + }); + expect(deleteNode).toHaveBeenCalledWith({ + name: NODE, + body: { + propagationPolicy: "Background", + preconditions: { uid: `node-uid-${NODE}` }, + }, + }); + }); + + it("cleans the leftovers of a removed node whose k8s Node is already gone", async () => { + leftoversOf(NODE); + jobs.push( + streamerJob("streamer-live-1", { affinity: hostnamePin([NODE]) }), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ jobs: 3, volume_claims: 1, volumes: 1 }), + ); + + expect(deletes).toEqual([ + `job update-cs-server-${NODE}`, + `job update-csgo-server-${NODE}`, + "job streamer-live-1", + `claim demos-${NODE}-claim`, + `volume demos-${NODE}`, + ]); + expect(deleteNode).not.toHaveBeenCalled(); + }); + + it("removes the labelled Node of a removed node that has nothing else left", async () => { + nodes.push(node(NODE, "False")); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ nodes: 1 }), + ); + + expect(deletes).toEqual([`node ${NODE}`]); + }); + + it("skips a Ready node without a row and reports it, since it may still be running", async () => { + nodes.push(node(NODE, "True", {}, minutesAgo(60 * 24))); + leftoversOf(NODE); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ recently_ready: 1 }), + ); + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual( + result({ recently_ready: 1 }), + ); + + expect(deletes).toEqual([]); + expect(logger.log).toHaveBeenCalledWith( + `[node-cleanup] ${NODE} is still Ready, skipping`, + ); + }); + + it.each(["False", "Unknown"] as const)( + "keeps a node that went %s less than 10 minutes ago, since it may only be restarting", + async (ready) => { + nodes.push(node(NODE, ready, {}, minutesAgo(9))); + leftoversOf(NODE); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ recently_ready: 1 }), + ); + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual( + result({ recently_ready: 1 }), + ); + + expect(deletes).toEqual([]); + expect(logger.log).toHaveBeenCalledWith( + `[node-cleanup] ${NODE} went NotReady less than 10 minutes ago, skipping`, + ); + }, + ); + + it.each([ + ["10 minutes ago", minutesAgo(10)], + ["at an unknown time", null], + ])( + "cleans a node that went NotReady %s", + async (_when, lastTransitionTime) => { + nodes.push(node(NODE, "Unknown", {}, lastTransitionTime)); + leftoversOf(NODE); + + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual( + result({ nodes: 1, jobs: 2, volume_claims: 1, volumes: 1 }), + ); + + expect(deletes).toEqual([`node ${NODE}`, ...deletesOfLeftovers(NODE)]); + }, + ); + + it.each([ + "node-role.kubernetes.io/control-plane", + "node-role.kubernetes.io/master", + ])( + "never cleans the control plane node, even without a row (%s)", + async (role) => { + nodes.push( + node(NODE, "Unknown", { + labels: { "5stack-id": NODE, [role]: "true" }, + }), + ); + leftoversOf(NODE); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual(result()); + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual( + result(), + ); + + expect(deletes).toEqual([]); + expect(logger.warn).toHaveBeenCalledTimes(2); + expect(logger.warn).toHaveBeenCalledWith( + `[node-cleanup] ${NODE} is a control plane node, skipping`, + ); + }, + ); + + it("skips a node that still has a row", async () => { + nodes.push(node(NODE, "Unknown")); + rows.push(NODE); + leftoversOf(NODE); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual(result()); + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual(result()); + + expect(deletes).toEqual([]); + expect(logger.log).toHaveBeenCalledWith( + `[node-cleanup] ${NODE} is registered again, skipping`, + ); + }); + + it("never deletes a NotReady Node that is not a game server node", async () => { + nodes.push(node("k3s-worker", "Unknown", { labels: {} })); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual(result()); + + expect(deletes).toEqual([]); + expect(hasura.query).not.toHaveBeenCalled(); + }); + + it("only deletes Retain local volumes pinned to the removed node, with their claims", async () => { + nodes.push(node(NODE, "Unknown"), node(OTHER_NODE, "True")); + rows.push(OTHER_NODE); + volumes.push( + volume(`steamcmd-${NODE}`, NODE), + volume(`serverfiles-${OTHER_NODE}`, OTHER_NODE), + // the panel's database volume is local too, but pinned by its own label + volume("timescaledb-pv", "true", { + claimRef: { namespace: NAMESPACE, name: "timescaledb-pvc" }, + nodeAffinity: { + required: { + nodeSelectorTerms: [volumeTerm(["true"], "5stack-timescaledb")], + }, + }, + }), + volume(`longhorn-${NODE}`, NODE, { storageClassName: "longhorn" }), + volume(`demos-${NODE}`, NODE, { + persistentVolumeReclaimPolicy: "Delete", + }), + // terms are ORed, so this volume can also live on the other node + volume(`either-${NODE}`, NODE, { + nodeAffinity: { + required: { + nodeSelectorTerms: [volumeTerm([NODE]), volumeTerm([OTHER_NODE])], + }, + }, + }), + volume(`shared-${NODE}`, NODE, { + nodeAffinity: { + required: { nodeSelectorTerms: [volumeTerm([NODE, OTHER_NODE])] }, + }, + }), + ); + claims.push( + claim(`steamcmd-${NODE}-claim`), + claim(`serverfiles-${OTHER_NODE}-claim`), + claim("timescaledb-pvc", "timescaledb-pv"), + claim(`longhorn-${NODE}-claim`), + claim(`demos-${NODE}-claim`), + claim(`either-${NODE}-claim`), + claim(`shared-${NODE}-claim`), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ nodes: 1, volume_claims: 1, volumes: 1 }), + ); + + expect(deletes).toEqual([ + `node ${NODE}`, + `claim steamcmd-${NODE}-claim`, + `volume steamcmd-${NODE}`, + ]); + expect(logger.warn).toHaveBeenCalledWith( + `[node-cleanup] ${NODE}: skipping volume demos-${NODE} and its claims, its reclaim policy is Delete`, + ); + }); + + it("deletes the claim a volume is bound to and the api's own claim once each, in the api namespace only", async () => { + volumes.push( + volume(`demos-${NODE}`, NODE), + volume(`steamcmd-${NODE}`, NODE, { + claimRef: { + namespace: NAMESPACE, + name: "steamcmd-restore", + uid: "pvc-uid-steamcmd-restore", + }, + }), + // released: its claim is already gone + volume(`serverfiles-csgo-${NODE}`, NODE), + ); + claims.push( + claim(`demos-${NODE}-claim`), + // pre-bound to its volume, which an operator's claim holds instead + claim(`steamcmd-${NODE}-claim`), + claim("steamcmd-restore", `steamcmd-${NODE}`), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ volume_claims: 3, volumes: 3 }), + ); + + expect(deletes).toEqual([ + `claim demos-${NODE}-claim`, + `volume demos-${NODE}`, + `claim steamcmd-${NODE}-claim`, + "claim steamcmd-restore", + `volume steamcmd-${NODE}`, + `volume serverfiles-csgo-${NODE}`, + ]); + for (const [request] of deleteNamespacedPersistentVolumeClaim.mock.calls) { + expect(request.namespace).toBe(NAMESPACE); + } + }); + + it("keeps a volume whose claimRef is in another namespace, with its claims", async () => { + volumes.push( + volume(`demos-${NODE}`, NODE), + // pre-bound by name only to a claim in another namespace, so no uid + volume(`serverfiles-${NODE}`, NODE, { + claimRef: { namespace: "backups", name: "serverfiles-copy" }, + }), + ); + claims.push( + claim(`demos-${NODE}-claim`), + claim(`serverfiles-${NODE}-claim`), + // the name of the claim the volume is bound to, even pre-bound to the + // volume, but in the api namespace and not the claimRef's + claim("serverfiles-copy", `serverfiles-${NODE}`), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ volume_claims: 1, volumes: 1 }), + ); + + expect(deletes).toEqual([ + `claim demos-${NODE}-claim`, + `volume demos-${NODE}`, + ]); + expect(logger.warn).toHaveBeenCalledWith( + `[node-cleanup] ${NODE}: skipping volume serverfiles-${NODE} and its claims, its claimRef is in namespace backups`, + ); + }); + + it("leaves a claim alone that only shares a name with a volume's claim", async () => { + volumes.push( + // released: its claimRef still names the restore claim deleted since + volume(`steamcmd-${NODE}`, NODE, { + claimRef: { + namespace: NAMESPACE, + name: "steamcmd-restore", + uid: "pvc-uid-deleted-steamcmd-restore", + }, + }), + volume(`demos-${NODE}`, NODE), + volume(`serverfiles-${NODE}`, NODE, { + claimRef: { + namespace: NAMESPACE, + name: "serverfiles-restore", + uid: "pvc-uid-deleted-serverfiles-restore", + }, + }), + ); + claims.push( + // a newer restore claim with the old name, bound to another node's + // volume, which may not even retain its data + claim("steamcmd-restore", `steamcmd-${OTHER_NODE}`), + // named like the api's claim for the volume, but bound to another one + claim(`demos-${NODE}-claim`, `demos-${OTHER_NODE}`), + // pre-bound to the volume, but not the claim its claimRef names + claim("serverfiles-restore", `serverfiles-${NODE}`), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ volumes: 3 }), + ); + + expect(deletes).toEqual([ + `volume steamcmd-${NODE}`, + `volume demos-${NODE}`, + `volume serverfiles-${NODE}`, + ]); + expect(deleteNamespacedPersistentVolumeClaim).not.toHaveBeenCalled(); + }); + + it("leaves match server Jobs, other Jobs and the Jobs of other nodes alone", async () => { + nodes.push(node(NODE, "Unknown"), node(OTHER_NODE, "True")); + rows.push(OTHER_NODE); + jobs.push( + updateJob(NODE), + job( + "m-0b6f3c9e-4a57-4d2f-9f0e-3c2b1a0d9e8f", + { app: "game-server", role: "match" }, + { affinity: hostnamePin([NODE]) }, + ), + job( + "map-assets-de-dust2", + { app: "map-assets" }, + { affinity: hostnamePin([NODE]) }, + ), + updateJob(OTHER_NODE), + streamerJob("streamer-live-2", { affinity: hostnamePin([OTHER_NODE]) }), + // not pinned to a single node + streamerJob("streamer-live-3", { + affinity: hostnamePin([NODE, OTHER_NODE]), + }), + streamerJob("streamer-live-4", {}), + // the api pins its Jobs by affinity, never by nodeName + streamerJob("streamer-live-5", { nodeName: NODE }), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ nodes: 1, jobs: 1 }), + ); + + expect(deletes).toEqual([`node ${NODE}`, `job update-cs-server-${NODE}`]); + }); + + it("finds the update Jobs of a node id with dots by their sanitized name", async () => { + const dottedNode = "gpu.lan"; + jobs.push(updateJob(dottedNode), updateJob(dottedNode, "csgo")); + + await expect(service.cleanupRemovedNodes(dottedNode)).resolves.toEqual( + result({ jobs: 2 }), + ); + + expect(deletes).toEqual([ + "job update-cs-server-gpu-lan", + "job update-csgo-server-gpu-lan", + ]); + }); + + it("does not count objects that are already gone or were replaced", async () => { + nodes.push(node(NODE, "Unknown")); + leftoversOf(NODE); + deleteNamespacedJob.mockImplementation( + recordDeletes("job", { + [`update-cs-server-${NODE}`]: 404, + [`update-csgo-server-${NODE}`]: 409, + }), + ); + deleteNamespacedPersistentVolumeClaim.mockImplementation( + recordDeletes("claim", { [`demos-${NODE}-claim`]: 404 }), + ); + deletePersistentVolume.mockImplementation( + recordDeletes("volume", { [`demos-${NODE}`]: 404 }), + ); + deleteNode.mockImplementation(recordDeletes("node", { [NODE]: 404 })); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual(result()); + + // a claim that is already gone does not hold its volume back + expect(deletes).toEqual([`node ${NODE}`, ...deletesOfLeftovers(NODE)]); + expect(logger.error).not.toHaveBeenCalled(); + }); + + it("keeps everything when the host registered a new Node since the listing", async () => { + nodes.push(node(NODE, "Unknown")); + leftoversOf(NODE); + deleteNode.mockImplementation(recordDeletes("node", { [NODE]: 409 })); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ recently_ready: 1 }), + ); + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual( + result({ recently_ready: 1 }), + ); + + expect(deletes).toEqual([`node ${NODE}`, `node ${NODE}`]); + expect(logger.log).toHaveBeenCalledWith( + `[node-cleanup] ${NODE} registered a new Node, skipping`, + ); + expect(logger.error).not.toHaveBeenCalled(); + }); + + it("flags a forbidden Node delete, still deletes the Jobs of every removed node and keeps their volumes", async () => { + nodes.push(node(NODE, "Unknown"), node(OTHER_NODE, "False")); + leftoversOf(NODE); + leftoversOf(OTHER_NODE); + deleteNode.mockImplementation( + recordDeletes("node", { [NODE]: 403, [OTHER_NODE]: 403 }), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ jobs: 4, node_delete_forbidden: true }), + ); + + expect(deletes).toEqual([ + `node ${NODE}`, + `job update-cs-server-${NODE}`, + `job update-csgo-server-${NODE}`, + `node ${OTHER_NODE}`, + `job update-cs-server-${OTHER_NODE}`, + `job update-csgo-server-${OTHER_NODE}`, + ]); + expect(logger.warn).toHaveBeenCalledWith( + `[node-cleanup] ${NODE}: not allowed to delete the Node, the api ClusterRole needs the delete verb on nodes. Keeping its volumes; run the cleanup in the server settings once git pull && ./update.sh in the panel has applied that ClusterRole`, + ); + expect(logger.error).not.toHaveBeenCalled(); + }); + + it("counts a failed Node delete, still deletes the Jobs and keeps the volumes for the retry", async () => { + nodes.push(node(NODE, "Unknown")); + leftoversOf(NODE); + deleteNode.mockImplementation(recordDeletes("node", { [NODE]: 500 })); + + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual( + result({ jobs: 2, failed: 1 }), + ); + + expect(deletes).toEqual([ + `node ${NODE}`, + `job update-cs-server-${NODE}`, + `job update-csgo-server-${NODE}`, + ]); + expect(logger.error).toHaveBeenCalledWith( + `[node-cleanup] ${NODE}: unable to delete the Node`, + "HTTP-Code: 500", + ); + }); + + it("counts any other error as failed and keeps a volume whose claim could not be deleted", async () => { + nodes.push(node(NODE, "Unknown")); + jobs.push(updateJob(NODE), updateJob(NODE, "csgo")); + volumes.push( + volume(`demos-${NODE}`, NODE), + volume(`steamcmd-${NODE}`, NODE), + ); + claims.push(claim(`demos-${NODE}-claim`), claim(`steamcmd-${NODE}-claim`)); + deleteNamespacedJob.mockImplementation( + recordDeletes("job", { [`update-cs-server-${NODE}`]: 500 }), + ); + deleteNamespacedPersistentVolumeClaim.mockImplementation( + recordDeletes("claim", { [`demos-${NODE}-claim`]: 500 }), + ); + deletePersistentVolume.mockImplementation( + recordDeletes("volume", { [`steamcmd-${NODE}`]: 500 }), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ nodes: 1, jobs: 1, volume_claims: 1, failed: 3 }), + ); + + expect(deletes).toEqual([ + `node ${NODE}`, + `job update-cs-server-${NODE}`, + `job update-csgo-server-${NODE}`, + `claim demos-${NODE}-claim`, + `claim steamcmd-${NODE}-claim`, + `volume steamcmd-${NODE}`, + ]); + expect(logger.error).toHaveBeenCalledTimes(3); + }); + + it("logs how many removed node ids a sweep cleaned without failures, skips or a forbidden Node delete", async () => { + const FAILING_NODE = "c9d0e1f2"; + const FORBIDDEN_NODE = "d7e8f9a0"; + const LIVE_NODE = "f3a4b5c6"; + nodes.push( + node(NODE, "Unknown"), + node(FAILING_NODE, "Unknown"), + node(FORBIDDEN_NODE, "Unknown"), + node(OTHER_NODE, "True"), + node(LIVE_NODE, "True"), + ); + rows.push(LIVE_NODE); + leftoversOf(NODE); + leftoversOf(FAILING_NODE); + leftoversOf(FORBIDDEN_NODE); + deleteNamespacedJob.mockImplementation( + recordDeletes("job", { [`update-cs-server-${FAILING_NODE}`]: 500 }), + ); + deleteNode.mockImplementation( + recordDeletes("node", { [FORBIDDEN_NODE]: 403 }), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ + nodes: 2, + jobs: 5, + volume_claims: 2, + volumes: 2, + failed: 1, + node_delete_forbidden: true, + recently_ready: 1, + }), + ); + + expect(logger.log).toHaveBeenCalledWith( + `[node-cleanup] ${OTHER_NODE} is still Ready, skipping`, + ); + expect(logger.log).toHaveBeenCalledWith( + "[node-cleanup] sweep cleaned 1 of 4 removed node id(s): deleted 5 job(s), 2 volume claim(s), 2 volume(s) and 2 node(s), 1 failed, node delete forbidden, 1 recently Ready", + ); + }); + + it("cleans only the given node when called for one removed node", async () => { + nodes.push(node(NODE, "Unknown"), node(OTHER_NODE, "Unknown")); + leftoversOf(NODE); + leftoversOf(OTHER_NODE); + + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual( + result({ nodes: 1, jobs: 2, volume_claims: 1, volumes: 1 }), + ); + + expect(readNode).toHaveBeenCalledWith({ name: NODE }); + expect(listNode).not.toHaveBeenCalled(); + expect(hasura.query).toHaveBeenCalledWith({ + game_server_nodes: { + __args: { where: { id: { _in: [NODE] } } }, + id: true, + }, + }); + expect(deletes).toEqual([`node ${NODE}`, ...deletesOfLeftovers(NODE)]); + }); + + it("still cleans one removed node whose k8s Node is already gone", async () => { + leftoversOf(NODE); + + await expect(service.cleanupRemovedNodes(NODE)).resolves.toEqual( + result({ jobs: 2, volume_claims: 1, volumes: 1 }), + ); + + expect(deletes).toEqual(deletesOfLeftovers(NODE)); + expect(deleteNode).not.toHaveBeenCalled(); + }); + + it("does not delete terminating objects again, but still deletes a live claim of a terminating volume", async () => { + const deletionTimestamp = new Date("2026-09-30T12:00:00Z"); + nodes.push(node(NODE, "Unknown", { deletionTimestamp })); + jobs.push( + job( + `update-cs-server-${NODE}`, + { app: "update-cs-server" }, + { affinity: hostnamePin([NODE]) }, + { deletionTimestamp }, + ), + updateJob(NODE, "csgo"), + ); + volumes.push( + volume(`demos-${NODE}`, NODE, {}, { deletionTimestamp }), + volume(`steamcmd-${NODE}`, NODE), + ); + claims.push( + claim(`demos-${NODE}-claim`), + claim(`steamcmd-${NODE}-claim`, `steamcmd-${NODE}`, { + deletionTimestamp, + }), + ); + + await expect(service.cleanupRemovedNodes()).resolves.toEqual( + result({ jobs: 1, volume_claims: 1, volumes: 1 }), + ); + + expect(deletes).toEqual([ + `job update-csgo-server-${NODE}`, + `claim demos-${NODE}-claim`, + `volume steamcmd-${NODE}`, + ]); + }); + + const listFailures: Array<[string, () => void, string | undefined]> = [ + ["Nodes", () => listNode.mockRejectedValue(apiError(500)), undefined], + [ + "Jobs", + () => listNamespacedJob.mockRejectedValue(apiError(500)), + undefined, + ], + [ + "volumes", + () => listPersistentVolume.mockRejectedValue(apiError(500)), + undefined, + ], + [ + "volume claims", + () => + listNamespacedPersistentVolumeClaim.mockRejectedValue(apiError(500)), + undefined, + ], + [ + "removed node's Node", + () => readNode.mockRejectedValue(apiError(500)), + NODE, + ], + ]; + + it.each(listFailures)( + "fails without deleting anything when reading the %s fails", + async (_what, fail, onlyNodeId) => { + nodes.push(node(NODE, "Unknown")); + leftoversOf(NODE); + fail(); + + await expect(service.cleanupRemovedNodes(onlyNodeId)).rejects.toThrow( + "unable to list cluster objects", + ); + + expect(deletes).toEqual([]); + expect(hasura.query).not.toHaveBeenCalled(); + expect(logger.error).toHaveBeenCalledWith( + "[node-cleanup] unable to list cluster objects", + "HTTP-Code: 500", + ); + }, + ); + + it("fails without deleting anything when the game server node rows cannot be read", async () => { + nodes.push(node(NODE, "Unknown")); + leftoversOf(NODE); + hasura.query.mockRejectedValue("database is unavailable"); + + await expect(service.cleanupRemovedNodes()).rejects.toThrow( + "unable to read game server nodes", + ); + await expect(service.cleanupRemovedNodes(NODE)).rejects.toThrow( + "unable to read game server nodes", + ); + + expect(deletes).toEqual([]); + expect(logger.error).toHaveBeenCalledWith( + "[node-cleanup] unable to read game server nodes", + "database is unavailable", + ); + }); +}); diff --git a/src/game-server-node/node-cleanup.service.ts b/src/game-server-node/node-cleanup.service.ts new file mode 100644 index 00000000..f5652ece --- /dev/null +++ b/src/game-server-node/node-cleanup.service.ts @@ -0,0 +1,615 @@ +import { Injectable, Logger } from "@nestjs/common"; +import { ConfigService } from "@nestjs/config"; +import { + BatchV1Api, + CoreV1Api, + KubeConfig, + V1DeleteOptions, + V1Job, + V1Node, + V1NodeSelectorTerm, + V1ObjectMeta, + V1PersistentVolume, + V1PersistentVolumeClaim, +} from "@kubernetes/client-node"; +import { HasuraService } from "../hasura/hasura.service"; +import { GameServersConfig } from "src/configs/types/GameServersConfig"; +import { GameServerNodeService } from "./game-server-node.service"; + +export type CleanupRemovedNodesOutput = { + nodes: number; + jobs: number; + volume_claims: number; + volumes: number; + failed: number; + node_delete_forbidden: boolean; + // Removed nodes that were skipped because they were Ready within the grace + // period, so they may still come back. + recently_ready: number; +}; + +type Inventory = { + nodes: Map; + jobs: Array; + volumes: Array; + claims: Map; +}; + +type DeleteOutcome = "deleted" | "gone" | "failed"; + +// Removing a game server node only deletes its row. This removes what the +// cluster still holds for it: its k8s Node, Jobs, volume claims and volumes. +@Injectable() +export class NodeCleanupService { + // A node that is still running re-creates its row on its next ping, within + // 30 s. One that went down may only be restarting, so it keeps everything + // until it has been NotReady this long. k8s marks a node that went down + // NotReady within about a minute. + public static readonly NOT_READY_GRACE_MS = 10 * 60 * 1000; + + private readonly namespace: string; + private readonly coreApi: CoreV1Api; + private readonly batchApi: BatchV1Api; + + constructor( + protected readonly logger: Logger, + protected readonly config: ConfigService, + protected readonly hasura: HasuraService, + ) { + this.namespace = + this.config.get("gameServers").namespace; + + const kc = new KubeConfig(); + kc.loadFromDefault(); + + this.coreApi = kc.makeApiClient(CoreV1Api); + this.batchApi = kc.makeApiClient(BatchV1Api); + } + + // With an id only that node is cleaned. Without one, every node id the + // cluster still has a labelled Node, a pinned Job or a local volume for is. + public async cleanupRemovedNodes( + onlyNodeId?: string, + ): Promise { + const result = NodeCleanupService.emptyResult(); + + const inventory = await this.getInventory(onlyNodeId); + + const candidates = onlyNodeId + ? [onlyNodeId] + : NodeCleanupService.getCandidates(inventory); + + const registered = await this.getRegisteredNodeIds(candidates); + + let removed = 0; + let cleaned = 0; + for (const nodeId of candidates) { + if (registered.has(nodeId)) { + if (onlyNodeId) { + this.logger.log( + `[node-cleanup] ${nodeId} is registered again, skipping`, + ); + } + continue; + } + removed++; + + const node = inventory.nodes.get(nodeId); + + // The panel's own host runs game servers too, but its Node is never one + // to delete, even without a row. + if (NodeCleanupService.isControlPlane(node)) { + this.logger.warn( + `[node-cleanup] ${nodeId} is a control plane node, skipping`, + ); + continue; + } + + const recentlyReady = NodeCleanupService.recentlyReady(node); + if (recentlyReady) { + result.recently_ready++; + this.logger.log(`[node-cleanup] ${nodeId} ${recentlyReady}, skipping`); + continue; + } + + const counts = await this.cleanupNode(nodeId, node, inventory); + this.logger.log( + `[node-cleanup] ${nodeId}: ${NodeCleanupService.describe(counts)}`, + ); + NodeCleanupService.addTo(result, counts); + if ( + counts.failed === 0 && + !counts.node_delete_forbidden && + counts.recently_ready === 0 + ) { + cleaned++; + } + } + + if (!onlyNodeId) { + this.logger.log( + `[node-cleanup] sweep cleaned ${cleaned} of ${removed} removed node id(s): ${NodeCleanupService.describe(result)}`, + ); + } + + return result; + } + + private async cleanupNode( + nodeId: string, + node: V1Node | undefined, + inventory: Inventory, + ): Promise { + const counts = NodeCleanupService.emptyResult(); + + const nodeState = await this.deleteNode(nodeId, node, counts); + if (nodeState === "replaced") { + return counts; + } + + for (const job of inventory.jobs) { + if ( + NodeCleanupService.ownedJobNodeId(job) !== nodeId || + NodeCleanupService.isTerminating(job.metadata) + ) { + continue; + } + + const outcome = await this.deleteObject( + nodeId, + `job ${job.metadata.name}`, + () => + this.batchApi.deleteNamespacedJob({ + name: job.metadata.name, + namespace: this.namespace, + body: NodeCleanupService.deleteOptions(job.metadata), + }), + ); + NodeCleanupService.count(counts, "jobs", outcome); + } + + // While the Node stays, the pods k8s still lists on it hold its claims and + // volumes, so deleting them now would only leave them terminating. A host + // that came back would then create no new ones, and lose them once its + // pods stopped. + if (nodeState === "kept") { + return counts; + } + + for (const volume of inventory.volumes) { + if (NodeCleanupService.volumeNodeId(volume) !== nodeId) { + continue; + } + + const volumeName = volume.metadata.name; + const reclaimPolicy = volume.spec.persistentVolumeReclaimPolicy; + + // Releasing a volume with any other policy can have k8s delete or scrub + // its data. + if (reclaimPolicy !== "Retain") { + this.logger.warn( + `[node-cleanup] ${nodeId}: skipping volume ${volumeName} and its claims, its reclaim policy is ${reclaimPolicy}`, + ); + continue; + } + + // A claim in another namespace is not the api's, and deleting its volume + // would leave that claim lost. + const claimNamespace = volume.spec.claimRef?.namespace; + if (claimNamespace && claimNamespace !== this.namespace) { + this.logger.warn( + `[node-cleanup] ${nodeId}: skipping volume ${volumeName} and its claims, its claimRef is in namespace ${claimNamespace}`, + ); + continue; + } + + let claimFailed = false; + for (const claim of this.getClaims(volume, inventory.claims)) { + const outcome = await this.deleteObject( + nodeId, + `volume claim ${claim.metadata.name}`, + () => + this.coreApi.deleteNamespacedPersistentVolumeClaim({ + name: claim.metadata.name, + namespace: this.namespace, + body: NodeCleanupService.deleteOptions(claim.metadata), + }), + ); + NodeCleanupService.count(counts, "volume_claims", outcome); + claimFailed ||= outcome === "failed"; + } + + // The volume goes only after its claims, so a failed claim leaves both + // for the next run. + if (claimFailed || NodeCleanupService.isTerminating(volume.metadata)) { + continue; + } + + const outcome = await this.deleteObject( + nodeId, + `volume ${volumeName}`, + () => + this.coreApi.deletePersistentVolume({ + name: volumeName, + body: NodeCleanupService.deleteOptions(volume.metadata), + }), + ); + NodeCleanupService.count(counts, "volumes", outcome); + } + + return counts; + } + + // The Node goes first. Once it is gone, k8s removes the pods it still lists + // on it, so the claims and volumes those hold can finish deleting. + private async deleteNode( + nodeId: string, + node: V1Node | undefined, + counts: CleanupRemovedNodesOutput, + ): Promise<"gone" | "replaced" | "kept"> { + if (!node || NodeCleanupService.isTerminating(node.metadata)) { + return "gone"; + } + + try { + await this.coreApi.deleteNode({ + name: nodeId, + body: NodeCleanupService.deleteOptions(node.metadata), + }); + counts.nodes++; + return "gone"; + } catch (error) { + const code = NodeCleanupService.statusCode(error); + if (code === "404") { + return "gone"; + } + + // The uid precondition found a newer Node, so the host registered again + // and keeps everything. + if (code === "409") { + counts.recently_ready++; + this.logger.log( + `[node-cleanup] ${nodeId} registered a new Node, skipping`, + ); + return "replaced"; + } + + if (code === "403") { + counts.node_delete_forbidden = true; + this.logger.warn( + `[node-cleanup] ${nodeId}: not allowed to delete the Node, the api ClusterRole needs the delete verb on nodes. Keeping its volumes; run the cleanup in the server settings once git pull && ./update.sh in the panel has applied that ClusterRole`, + ); + } else { + counts.failed++; + this.logger.error( + `[node-cleanup] ${nodeId}: unable to delete the Node`, + error?.message ?? error, + ); + } + return "kept"; + } + } + + // Without a full picture nothing is deleted, so a failed read rejects. + private async getInventory(onlyNodeId?: string): Promise { + try { + const [nodes, jobs, volumes, claims] = await Promise.all([ + onlyNodeId + ? this.readNode(onlyNodeId) + : this.coreApi.listNode().then(({ items }) => items), + this.batchApi.listNamespacedJob({ namespace: this.namespace }), + this.coreApi.listPersistentVolume(), + this.coreApi.listNamespacedPersistentVolumeClaim({ + namespace: this.namespace, + }), + ]); + + return { + nodes: new Map(nodes.map((node) => [node.metadata.name, node])), + jobs: jobs.items, + volumes: volumes.items, + claims: new Map( + claims.items.map((claim) => [claim.metadata.name, claim]), + ), + }; + } catch (error) { + this.logger.error( + `[node-cleanup] unable to list cluster objects`, + error?.message ?? error, + ); + throw new Error("unable to list cluster objects"); + } + } + + private async readNode(name: string): Promise> { + try { + return [await this.coreApi.readNode({ name })]; + } catch (error) { + if (NodeCleanupService.statusCode(error) === "404") { + return []; + } + throw error; + } + } + + private async getRegisteredNodeIds( + nodeIds: Array, + ): Promise> { + if (nodeIds.length === 0) { + return new Set(); + } + + try { + const { game_server_nodes } = await this.hasura.query({ + game_server_nodes: { + __args: { + where: { + id: { + _in: nodeIds, + }, + }, + }, + id: true, + }, + }); + + return new Set(game_server_nodes.map(({ id }) => id)); + } catch (error) { + this.logger.error( + `[node-cleanup] unable to read game server nodes`, + error?.message ?? error, + ); + throw new Error("unable to read game server nodes"); + } + } + + // The claim the volume is bound to, plus the `-claim` the api + // creates for it, which may never have bound. A name alone is not enough: a + // released volume keeps the claimRef of its deleted claim, and a newer claim + // with that name can hold another volume. So a claim counts only when it is + // bound, or pre-bound like the api's, to this volume, and the one the + // claimRef names must also have its uid, when the claimRef records one. + private getClaims( + volume: V1PersistentVolume, + claims: Map, + ): Array { + const found = new Map(); + + const add = (name: string, uid?: string) => { + const claim = claims.get(name); + if ( + claim && + claim.spec?.volumeName === volume.metadata.name && + (!uid || claim.metadata.uid === uid) && + !NodeCleanupService.isTerminating(claim.metadata) + ) { + found.set(name, claim); + } + }; + + add(`${volume.metadata.name}-claim`); + + const claimRef = volume.spec.claimRef; + if (claimRef?.namespace === this.namespace && claimRef.name) { + add(claimRef.name, claimRef.uid); + } + + return [...found.values()]; + } + + // A 404 means already gone and a 409 that the uid precondition found a + // newer object with the same name, so neither is a delete or a failure. + private async deleteObject( + nodeId: string, + what: string, + request: () => Promise, + ): Promise { + try { + await request(); + return "deleted"; + } catch (error) { + if (NodeCleanupService.isGone(error)) { + return "gone"; + } + + this.logger.error( + `[node-cleanup] ${nodeId}: unable to delete ${what}`, + error?.message ?? error, + ); + return "failed"; + } + } + + private static getCandidates(inventory: Inventory): Array { + const nodeIds = new Set(); + + for (const [name, node] of inventory.nodes) { + if (node.metadata.labels?.["5stack-id"]) { + nodeIds.add(name); + } + } + + for (const job of inventory.jobs) { + const nodeId = NodeCleanupService.ownedJobNodeId(job); + if (nodeId) { + nodeIds.add(nodeId); + } + } + + for (const volume of inventory.volumes) { + const nodeId = NodeCleanupService.volumeNodeId(volume); + if (nodeId) { + nodeIds.add(nodeId); + } + } + + return [...nodeIds].sort(); + } + + // Only the Jobs the api runs for the node itself. Match server Jobs are + // pinned to a node too, but they belong to the match flow. + private static ownedJobNodeId(job: V1Job): string | null { + const nodeId = NodeCleanupService.pinnedNodeId(job); + if (!nodeId) { + return null; + } + + if ( + job.metadata.name === GameServerNodeService.GET_UPDATE_JOB_NAME(nodeId) || + job.metadata.name === + GameServerNodeService.GET_UPDATE_JOB_NAME(nodeId, "csgo") + ) { + return nodeId; + } + + // The validation Job is named after the build, so only its label and pin + // tie it to this node. + const app = + job.metadata.labels?.app ?? job.spec?.template?.metadata?.labels?.app; + if (app === "validate-gamedata" || app === "game-streamer") { + return nodeId; + } + + return null; + } + + private static pinnedNodeId(job: V1Job): string | null { + return NodeCleanupService.pinnedValue( + job.spec?.template?.spec?.affinity?.nodeAffinity + ?.requiredDuringSchedulingIgnoredDuringExecution?.nodeSelectorTerms, + "kubernetes.io/hostname", + ); + } + + private static volumeNodeId(volume: V1PersistentVolume): string | null { + if (volume.spec?.storageClassName !== "local-storage") { + return null; + } + + return NodeCleanupService.pinnedValue( + volume.spec.nodeAffinity?.required?.nodeSelectorTerms, + "5stack-id", + ); + } + + // Terms are ORed, so an object is pinned only when every term requires the + // key to be exactly the same one value. + private static pinnedValue( + terms: Array | undefined, + key: string, + ): string | null { + let pinned: string | null = null; + + for (const term of terms ?? []) { + const values = term.matchExpressions?.find( + (expression) => expression.key === key && expression.operator === "In", + )?.values; + + if (values?.length !== 1 || (pinned !== null && pinned !== values[0])) { + return null; + } + + pinned = values[0]; + } + + return pinned; + } + + private static isControlPlane(node?: V1Node): boolean { + const labels = node?.metadata?.labels ?? {}; + return ( + "node-role.kubernetes.io/control-plane" in labels || + "node-role.kubernetes.io/master" in labels + ); + } + + // Why the node still counts as recently Ready, or null once it has been + // NotReady for the grace period. A missing Node, or one without a transition + // time, does not count. + private static recentlyReady(node?: V1Node): string | null { + const ready = node?.status?.conditions?.find( + (condition) => condition.type === "Ready", + ); + if (ready?.status === "True") { + return "is still Ready"; + } + + const notReadyFor = + Date.now() - new Date(ready?.lastTransitionTime ?? NaN).getTime(); + if (notReadyFor < NodeCleanupService.NOT_READY_GRACE_MS) { + return `went NotReady less than ${NodeCleanupService.NOT_READY_GRACE_MS / 60000} minutes ago`; + } + + return null; + } + + private static isTerminating(metadata?: V1ObjectMeta): boolean { + return !!metadata?.deletionTimestamp; + } + + // The API server ignores query params once a body is sent, so options go + // in the body. The uid keeps the delete off a newer object with the name. + private static deleteOptions(metadata: V1ObjectMeta): V1DeleteOptions { + return { + propagationPolicy: "Background", + ...(metadata.uid ? { preconditions: { uid: metadata.uid } } : {}), + }; + } + + private static statusCode(error: { code?: number | string }) { + return error?.code?.toString(); + } + + private static isGone(error: { code?: number | string }) { + const code = NodeCleanupService.statusCode(error); + return code === "404" || code === "409"; + } + + private static count( + counts: CleanupRemovedNodesOutput, + key: "jobs" | "volume_claims" | "volumes", + outcome: DeleteOutcome, + ) { + if (outcome === "deleted") { + counts[key]++; + } else if (outcome === "failed") { + counts.failed++; + } + } + + private static emptyResult(): CleanupRemovedNodesOutput { + return { + nodes: 0, + jobs: 0, + volume_claims: 0, + volumes: 0, + failed: 0, + node_delete_forbidden: false, + recently_ready: 0, + }; + } + + private static addTo( + total: CleanupRemovedNodesOutput, + counts: CleanupRemovedNodesOutput, + ) { + total.nodes += counts.nodes; + total.jobs += counts.jobs; + total.volume_claims += counts.volume_claims; + total.volumes += counts.volumes; + total.failed += counts.failed; + total.node_delete_forbidden ||= counts.node_delete_forbidden; + total.recently_ready += counts.recently_ready; + } + + private static describe(counts: CleanupRemovedNodesOutput): string { + return ( + `deleted ${counts.jobs} job(s), ${counts.volume_claims} volume claim(s), ` + + `${counts.volumes} volume(s) and ${counts.nodes} node(s), ${counts.failed} failed` + + (counts.node_delete_forbidden ? ", node delete forbidden" : "") + + (counts.recently_ready > 0 + ? `, ${counts.recently_ready} recently Ready` + : "") + ); + } +} diff --git a/test/node-scheduling.spec.ts b/test/node-scheduling.spec.ts index d5d69aff..2d554d2c 100644 --- a/test/node-scheduling.spec.ts +++ b/test/node-scheduling.spec.ts @@ -106,6 +106,7 @@ describe("node scheduling across an outage (SQL-driven)", () => { {} as never, {} as never, {} as never, + {} as never, ); });