From 7e175160a568fab913b05096fc8ce939b76813d7 Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Wed, 30 Sep 2026 05:35:58 -0400 Subject: [PATCH 1/3] bug: only send server and node status notifications that make sense - no offline alerts for disabled servers, disabled/unused nodes, or servers whose node is already down (the node alert covers it) - intentional restarts/rebuilds get a 5 minute grace before any down alert - RCON-down alerts from the minute ping only fire after 2 minutes unreachable - online alerts only close a matching offline alert (node and region) - a node set to not accept new matches keeps that state across an outage --- .../down.sql | 2 + .../up.sql | 6 + .../dedicated-servers.module.ts | 2 + .../dedicated-servers.service.spec.ts | 230 ++++++++++++++++- .../dedicated-servers.service.ts | 111 +++++++- .../jobs/PingDedicatedServers.spec.ts | 71 ++++++ .../jobs/PingDedicatedServers.ts | 7 +- .../game-server-node.controller.spec.ts | 71 ++++++ .../game-server-node.controller.ts | 40 +-- .../game-server-node.service.spec.ts | 67 +++++ .../game-server-node.service.ts | 111 ++++++-- .../jobs/CheckServerPluginVersions.spec.ts | 31 +++ .../jobs/CheckServerPluginVersions.ts | 3 + .../jobs/MarkDedicatedServerOffline.spec.ts | 173 +++++++++++++ .../jobs/MarkDedicatedServerOffline.ts | 88 ++++++- .../jobs/MarkGameServerNodeOffline.spec.ts | 236 ++++++++++++++++++ .../jobs/MarkGameServerNodeOffline.ts | 35 ++- .../jobs/MarkGameServerNodeOnline.spec.ts | 119 +++++++++ .../jobs/MarkGameServerNodeOnline.ts | 79 ++++-- .../notifications.service.spec.ts | 56 +++++ src/notifications/notifications.service.ts | 25 ++ src/rcon/rcon.service.spec.ts | 89 +++++++ src/rcon/rcon.service.ts | 28 ++- test/node-scheduling.spec.ts | 192 ++++++++++++++ 24 files changed, 1767 insertions(+), 105 deletions(-) create mode 100644 hasura/migrations/default/1889000000700_game_server_node_accepting_new_matches/down.sql create mode 100644 hasura/migrations/default/1889000000700_game_server_node_accepting_new_matches/up.sql create mode 100644 src/dedicated-servers/jobs/PingDedicatedServers.spec.ts create mode 100644 src/game-server-node/game-server-node.controller.spec.ts create mode 100644 src/game-server-node/jobs/CheckServerPluginVersions.spec.ts create mode 100644 src/game-server-node/jobs/MarkDedicatedServerOffline.spec.ts create mode 100644 src/game-server-node/jobs/MarkGameServerNodeOffline.spec.ts create mode 100644 src/game-server-node/jobs/MarkGameServerNodeOnline.spec.ts create mode 100644 src/rcon/rcon.service.spec.ts create mode 100644 test/node-scheduling.spec.ts diff --git a/hasura/migrations/default/1889000000700_game_server_node_accepting_new_matches/down.sql b/hasura/migrations/default/1889000000700_game_server_node_accepting_new_matches/down.sql new file mode 100644 index 000000000..fd4eab0d2 --- /dev/null +++ b/hasura/migrations/default/1889000000700_game_server_node_accepting_new_matches/down.sql @@ -0,0 +1,2 @@ +ALTER TABLE public.game_server_nodes + DROP COLUMN IF EXISTS accepting_new_matches; diff --git a/hasura/migrations/default/1889000000700_game_server_node_accepting_new_matches/up.sql b/hasura/migrations/default/1889000000700_game_server_node_accepting_new_matches/up.sql new file mode 100644 index 000000000..30af8c124 --- /dev/null +++ b/hasura/migrations/default/1889000000700_game_server_node_accepting_new_matches/up.sql @@ -0,0 +1,6 @@ +ALTER TABLE public.game_server_nodes + ADD COLUMN IF NOT EXISTS accepting_new_matches boolean NOT NULL DEFAULT true; + +UPDATE public.game_server_nodes + SET accepting_new_matches = false + WHERE status = 'NotAcceptingNewMatches'; diff --git a/src/dedicated-servers/dedicated-servers.module.ts b/src/dedicated-servers/dedicated-servers.module.ts index cdaa73cee..a207bc01d 100644 --- a/src/dedicated-servers/dedicated-servers.module.ts +++ b/src/dedicated-servers/dedicated-servers.module.ts @@ -18,6 +18,7 @@ import { SystemModule } from "src/system/system.module"; import { PluginRuntimeModule } from "src/plugin-runtime/plugin-runtime.module"; import { GamePluginsModule } from "../game-plugins/game-plugins.module"; import { PostgresModule } from "../postgres/postgres.module"; +import { NotificationsModule } from "../notifications/notifications.module"; @Module({ imports: [ @@ -36,6 +37,7 @@ import { PostgresModule } from "../postgres/postgres.module"; PluginRuntimeModule, GamePluginsModule, PostgresModule, + NotificationsModule, ], providers: [ DedicatedServersService, diff --git a/src/dedicated-servers/dedicated-servers.service.spec.ts b/src/dedicated-servers/dedicated-servers.service.spec.ts index b01aaf714..71d423248 100644 --- a/src/dedicated-servers/dedicated-servers.service.spec.ts +++ b/src/dedicated-servers/dedicated-servers.service.spec.ts @@ -10,7 +10,8 @@ describe("DedicatedServersService.rebuildDedicatedServer", () => { null as never, null as never, null as never, - { getConnection: () => ({}) } as never, + { getConnection: () => ({ set: jest.fn() }) } as never, + null as never, null as never, null as never, null as never, @@ -84,3 +85,230 @@ describe("DedicatedServersService.rebuildDedicatedServer", () => { expect(second).toEqual({ status: "fulfilled", value: true }); }); }); + +class FakeRedis { + private values = new Map(); + private hashes = new Map>(); + public now = Date.now(); + + async set(key: string, value: string, mode?: string, ms?: number) { + this.values.set(key, { + value, + expiresAt: mode === "PX" ? this.now + ms : undefined, + }); + return "OK"; + } + + async pttl(key: string) { + const entry = this.values.get(key); + if (!entry || (entry.expiresAt && entry.expiresAt <= this.now)) { + return -2; + } + return entry.expiresAt ? entry.expiresAt - this.now : -1; + } + + private hash(key: string) { + if (!this.hashes.has(key)) { + this.hashes.set(key, new Map()); + } + return this.hashes.get(key); + } + + async hsetnx(key: string, field: string, value: string) { + if (this.hash(key).has(field)) { + return 0; + } + this.hash(key).set(field, value); + return 1; + } + + async hset(key: string, field: string, value: string) { + this.hash(key).set(field, value); + return 1; + } + + async hget(key: string, field: string) { + return this.hash(key).get(field) ?? null; + } + + async hdel(key: string, field: string) { + return this.hash(key).delete(field) ? 1 : 0; + } + + async expire() { + return 1; + } +} + +describe("DedicatedServersService.pingDedicatedServer", () => { + let redis: FakeRedis; + let row: Record; + let reachable: boolean; + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let notifications: { send: jest.Mock }; + let service: DedicatedServersService; + + beforeEach(() => { + redis = new FakeRedis(); + reachable = true; + row = { + game: "cs2", + label: "Retakes #1", + enabled: true, + connected: true, + steam_relay: null, + game_server_node_id: "node-1", + server_region: { steam_relay: false }, + }; + hasura = { + query: jest.fn(async () => ({ servers_by_pk: { ...row } })), + mutation: jest.fn(async (mutation: Record) => { + Object.assign(row, mutation.update_servers_by_pk?.__args._set ?? {}); + return {}; + }), + }; + notifications = { send: jest.fn().mockResolvedValue(undefined) }; + const rcon = { + connect: jest.fn(async () => + reachable + ? { + send: async () => + JSON.stringify({ + server: { + steamid: null, + clients_human: 3, + map: "de_inferno", + }, + }), + } + : null, + ), + disconnect: jest.fn(), + }; + + service = new DedicatedServersService( + { log: jest.fn(), warn: jest.fn(), error: jest.fn() } as never, + { get: () => ({ namespace: "5stack" }) } as never, + hasura as never, + null as never, + rcon as never, + { getConnection: () => redis } as never, + { restartDeployment: jest.fn() } as never, + null as never, + null as never, + null as never, + notifications as never, + ); + }); + + const minutes = (count: number) => { + redis.now += count * 60 * 1000; + jest.setSystemTime(redis.now); + }; + + beforeAll(() => { + jest.useFakeTimers({ doNotFake: ["setTimeout", "setImmediate"] }); + }); + + afterAll(() => { + jest.useRealTimers(); + }); + + beforeEach(() => { + jest.setSystemTime(redis.now); + }); + + const alerts = () => notifications.send.mock.calls.length; + + it("does not mark an unreachable server connected", async () => { + row.connected = false; + reachable = false; + + await service.pingDedicatedServer("server-1"); + + expect(row.connected).toBe(false); + }); + + it("marks a reachable server connected", async () => { + row.connected = false; + + await service.pingDedicatedServer("server-1"); + + expect(row.connected).toBe(true); + }); + + it("stays quiet about a server that just stopped answering", async () => { + reachable = false; + + await service.pingDedicatedServer("server-1"); + minutes(1); + await service.pingDedicatedServer("server-1"); + + expect(alerts()).toBe(0); + expect(row.connected).toBe(true); + }); + + it("reports a server unreachable for two minutes, once", async () => { + reachable = false; + + await service.pingDedicatedServer("server-1"); + minutes(1); + await service.pingDedicatedServer("server-1"); + minutes(1); + await service.pingDedicatedServer("server-1"); + minutes(1); + await service.pingDedicatedServer("server-1"); + + expect(notifications.send).toHaveBeenCalledTimes(1); + expect(notifications.send).toHaveBeenCalledWith( + "DedicatedServerRconStatus", + expect.objectContaining({ title: "Dedicated Server RCON Error" }), + undefined, + expect.any(Number), + ); + expect(row.connected).toBe(false); + }); + + it("starts over once the server answers again", async () => { + reachable = false; + await service.pingDedicatedServer("server-1"); + minutes(1); + reachable = true; + await service.pingDedicatedServer("server-1"); + minutes(1); + reachable = false; + await service.pingDedicatedServer("server-1"); + minutes(1); + await service.pingDedicatedServer("server-1"); + + expect(alerts()).toBe(0); + }); + + it("waits out a restart before reporting", async () => { + await service.restartDedicatedServer("server-1"); + reachable = false; + + for (let minute = 0; minute < 4; minute++) { + await service.pingDedicatedServer("server-1"); + minutes(1); + } + expect(alerts()).toBe(0); + + minutes(1); + await service.pingDedicatedServer("server-1"); + expect(alerts()).toBe(1); + }); + + it("never reports a disabled external server", async () => { + row.enabled = false; + row.game_server_node_id = null; + reachable = false; + + for (let minute = 0; minute < 4; minute++) { + await service.pingDedicatedServer("server-1"); + minutes(1); + } + + expect(alerts()).toBe(0); + }); +}); diff --git a/src/dedicated-servers/dedicated-servers.service.ts b/src/dedicated-servers/dedicated-servers.service.ts index 370e4276e..dd626e4c0 100644 --- a/src/dedicated-servers/dedicated-servers.service.ts +++ b/src/dedicated-servers/dedicated-servers.service.ts @@ -13,11 +13,21 @@ import { SystemService } from "src/system/system.service"; import { PluginRuntimeService } from "src/plugin-runtime/plugin-runtime.service"; import { GameModesService } from "../game-plugins/game-modes.service"; import { MapRotationService } from "../game-plugins/map-rotation.service"; +import { NotificationsService } from "src/notifications/notifications.service"; +import { DISCORD_COLORS } from "src/notifications/utilities/constants"; +import { MarkDedicatedServerOffline } from "src/game-server-node/jobs/MarkDedicatedServerOffline"; @Injectable() export class DedicatedServersService { private static readonly rebuilds = new Map>(); + // One failed RCON ping says little: a node dying under the server is only + // marked Offline up to 90s later, and a restart takes a while to boot. A + // server is reported once it has stayed unreachable this long. + public static readonly UNREACHABLE_ALERT_AFTER_MS = 2 * 60 * 1000; + + private static readonly UNREACHABLE_KEY = "dedicated-servers:unreachable"; + private appConfig: AppConfig; private gameServerConfig: GameServersConfig; private readonly namespace: string; @@ -38,6 +48,7 @@ export class DedicatedServersService { private readonly pluginRuntimeService: PluginRuntimeService, private readonly gameModesService: GameModesService, private readonly mapRotationService: MapRotationService, + private readonly notifications: NotificationsService, ) { this.redis = this.redisManager.getConnection(); @@ -454,6 +465,10 @@ export class DedicatedServersService { const rebuild = previous .catch(() => false) .then(async () => { + if (start) { + await this.expectRestart(serverId); + } + await this.removeDedicatedServer(serverId); return start ? await this.setupDedicatedServer(serverId) : true; @@ -486,6 +501,7 @@ export class DedicatedServersService { } } finally { await this.redis.hdel("dedicated-servers:stats", serverId); + await this.redis.hdel(DedicatedServersService.UNREACHABLE_KEY, serverId); await this.hasura.mutation({ update_servers_by_pk: { @@ -786,6 +802,7 @@ export class DedicatedServersService { servers_by_pk: { __args: { id: serverId }, game: true, + label: true, enabled: true, connected: true, steam_relay: true, @@ -817,15 +834,6 @@ export class DedicatedServersService { return; } - if (!server.connected) { - await this.hasura.mutation({ - update_servers_by_pk: { - __args: { pk_columns: { id: serverId }, _set: { connected: true } }, - id: true, - }, - }); - } - // TODO - fix steam relay for csgo const steamRelayeEnabled = server.game === "csgo" ? false : server.server_region?.steam_relay; @@ -836,9 +844,24 @@ export class DedicatedServersService { ); if (!statusInfo) { + await this.recordUnreachable(serverId, server); return; } + await this.redis.hdel(DedicatedServersService.UNREACHABLE_KEY, serverId); + + if (!server.connected) { + await this.hasura.mutation({ + update_servers_by_pk: { + __args: { + pk_columns: { id: serverId }, + _set: { connected: true, offline_at: null }, + }, + id: true, + }, + }); + } + const { steamId, clients_human, map } = statusInfo; await this.redis.hset( @@ -871,7 +894,77 @@ export class DedicatedServersService { await this.RconService.disconnect(serverId); } + public async expectRestart(serverId: string): Promise { + await MarkDedicatedServerOffline.expectRestart(this.redis, serverId); + } + + private async recordUnreachable( + serverId: string, + server: { label: string; enabled: boolean; connected: boolean }, + ): Promise { + const now = Date.now(); + + await this.redis.hsetnx( + DedicatedServersService.UNREACHABLE_KEY, + serverId, + JSON.stringify({ since: now, reported: false }), + ); + + const streak: { since: number; reported: boolean } = JSON.parse( + await this.redis.hget(DedicatedServersService.UNREACHABLE_KEY, serverId), + ); + + if ( + now - streak.since < + DedicatedServersService.UNREACHABLE_ALERT_AFTER_MS + ) { + return; + } + + if (server.connected) { + await this.hasura.mutation({ + update_servers_by_pk: { + __args: { + pk_columns: { id: serverId }, + _set: { connected: false, offline_at: new Date().toISOString() }, + }, + id: true, + }, + }); + } + + if ( + streak.reported || + !server.enabled || + (await MarkDedicatedServerOffline.restartGraceRemaining( + this.redis, + serverId, + )) > 0 + ) { + return; + } + + await this.redis.hset( + DedicatedServersService.UNREACHABLE_KEY, + serverId, + JSON.stringify({ ...streak, reported: true }), + ); + + await this.notifications.send( + "DedicatedServerRconStatus", + { + message: `Dedicated Server (${NotificationsService.escapeHtml(server.label || serverId)}) is not able to connect to the RCON.`, + title: "Dedicated Server RCON Error", + role: "administrator", + entity_id: serverId, + }, + undefined, + DISCORD_COLORS.RED, + ); + } + public async restartDedicatedServer(serverId: string): Promise { + await this.expectRestart(serverId); await this.systemService.restartDeployment( this.getDedicatedServerDeploymentName(serverId), this.namespace, diff --git a/src/dedicated-servers/jobs/PingDedicatedServers.spec.ts b/src/dedicated-servers/jobs/PingDedicatedServers.spec.ts new file mode 100644 index 000000000..2dbb85aa8 --- /dev/null +++ b/src/dedicated-servers/jobs/PingDedicatedServers.spec.ts @@ -0,0 +1,71 @@ +import { PingDedicatedServers } from "./PingDedicatedServers"; + +describe("PingDedicatedServers", () => { + let servers: Array<{ + id: string; + game_server_node: { status: string } | null; + }>; + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let dedicatedServers: { + pingDedicatedServer: jest.Mock; + expectRestart: jest.Mock; + }; + let job: PingDedicatedServers; + + beforeEach(() => { + hasura = { + query: jest.fn(async () => ({ servers })), + mutation: jest.fn().mockResolvedValue({}), + }; + dedicatedServers = { + pingDedicatedServer: jest.fn().mockResolvedValue(undefined), + expectRestart: jest.fn().mockResolvedValue(undefined), + }; + job = new PingDedicatedServers(hasura as any, dedicatedServers as any); + }); + + it("pings a server on a node that stopped accepting new matches", async () => { + servers = [ + { + id: "server-1", + game_server_node: { status: "NotAcceptingNewMatches" }, + }, + ]; + + await job.process(); + + expect(dedicatedServers.pingDedicatedServer).toHaveBeenCalledWith( + "server-1", + ); + expect(hasura.mutation).not.toHaveBeenCalled(); + }); + + it("marks a server on an offline node disconnected and waits for it to reboot", async () => { + servers = [{ id: "server-1", game_server_node: { status: "Offline" } }]; + + await job.process(); + + expect(dedicatedServers.pingDedicatedServer).not.toHaveBeenCalled(); + expect(hasura.mutation).toHaveBeenCalledWith( + expect.objectContaining({ + update_servers_by_pk: expect.objectContaining({ + __args: { + pk_columns: { id: "server-1" }, + _set: { connected: false }, + }, + }), + }), + ); + expect(dedicatedServers.expectRestart).toHaveBeenCalledWith("server-1"); + }); + + it("pings an external server", async () => { + servers = [{ id: "server-1", game_server_node: null }]; + + await job.process(); + + expect(dedicatedServers.pingDedicatedServer).toHaveBeenCalledWith( + "server-1", + ); + }); +}); diff --git a/src/dedicated-servers/jobs/PingDedicatedServers.ts b/src/dedicated-servers/jobs/PingDedicatedServers.ts index 3db946a43..95be8095a 100644 --- a/src/dedicated-servers/jobs/PingDedicatedServers.ts +++ b/src/dedicated-servers/jobs/PingDedicatedServers.ts @@ -42,10 +42,7 @@ export class PingDedicatedServers extends WorkerHost { await Promise.all( servers.map(async (server) => { - if ( - server.game_server_node && - server.game_server_node.status !== "Online" - ) { + if (server.game_server_node?.status === "Offline") { await this.hasura.mutation({ update_servers_by_pk: { __args: { @@ -55,6 +52,8 @@ export class PingDedicatedServers extends WorkerHost { __typename: true, }, }); + // Its pod comes back up only once the node does. + await this.dedicatedServersService.expectRestart(server.id); return; } await this.dedicatedServersService.pingDedicatedServer(server.id); diff --git a/src/game-server-node/game-server-node.controller.spec.ts b/src/game-server-node/game-server-node.controller.spec.ts new file mode 100644 index 000000000..ed2d9843f --- /dev/null +++ b/src/game-server-node/game-server-node.controller.spec.ts @@ -0,0 +1,71 @@ +import { GameServerNodeController } from "./game-server-node.controller"; + +describe("GameServerNodeController ping disk alerts", () => { + let gameServerNodeService: { updateStatus: jest.Mock }; + let notifications: { send: jest.Mock }; + let controller: GameServerNodeController; + + beforeEach(() => { + gameServerNodeService = { updateStatus: jest.fn() }; + notifications = { send: jest.fn().mockResolvedValue(undefined) }; + const queue = { add: jest.fn(), remove: jest.fn() }; + + controller = new GameServerNodeController( + { warn: jest.fn(), log: jest.fn() } as any, + {} as any, + { get: jest.fn().mockReturnValue({}) } as any, + { mutation: jest.fn().mockResolvedValue({}) } as any, + { + remember: jest.fn().mockResolvedValue([ + { name: "disk_warning_percent", value: "75" }, + { name: "disk_critical_percent", value: "90" }, + ]), + } as any, + {} as any, + {} as any, + gameServerNodeService as any, + notifications as any, + {} as any, + queue as any, + queue as any, + {} as any, + queue as any, + queue as any, + {} as any, + ); + }); + + const ping = () => + controller.handleMessage({ + node: "node-1", + labels: { "5stack-id": "1", "5stack-network-limiter": "1" }, + nodeStats: { + cpuInfo: { sockets: 1, coresPerSocket: 8, threadsPerCore: 2 }, + disks: [{ mountpoint: "/", usedPercent: "95", available: "1" }], + }, + } as any); + + it("raises a disk alert for a node in service", async () => { + gameServerNodeService.updateStatus.mockResolvedValue({ inService: true }); + + await ping(); + + expect(notifications.send).toHaveBeenCalledWith( + "GameNodeStatus", + expect.objectContaining({ + title: "Game Server Node Disk Space Critical", + }), + undefined, + expect.any(Number), + false, + ); + }); + + it("raises no disk alert for a disabled node", async () => { + gameServerNodeService.updateStatus.mockResolvedValue({ inService: false }); + + await ping(); + + expect(notifications.send).not.toHaveBeenCalled(); + }); +}); diff --git a/src/game-server-node/game-server-node.controller.ts b/src/game-server-node/game-server-node.controller.ts index 879150d6c..b30782ec8 100644 --- a/src/game-server-node/game-server-node.controller.ts +++ b/src/game-server-node/game-server-node.controller.ts @@ -183,6 +183,9 @@ export class GameServerNodeController { const cooldownKey = (level: string) => `${payload.node}:${level}`; const shouldNotify = (level: string) => { + if (!result?.inService) { + return false; + } const last = this.diskWarningCooldowns.get(cooldownKey(level)); return ( !last || @@ -459,38 +462,11 @@ export class GameServerNodeController { game_server_node_id: string; enabled: boolean; }) { - const { game_server_nodes_by_pk } = await this.hasura.query({ - game_server_nodes_by_pk: { - __args: { - id: data.game_server_node_id, - }, - status: true, - }, - }); - - if (game_server_nodes_by_pk.status === "Setup") { - return { - success: false, - }; - } - - await this.hasura.mutation({ - update_game_server_nodes_by_pk: { - __args: { - pk_columns: { - id: data.game_server_node_id, - }, - _set: { - // we set it to offline, to allow it to come back online to accept new matches - status: data.enabled ? "Online" : "NotAcceptingNewMatches", - }, - }, - __typename: true, - }, - }); - return { - success: true, + success: await this.gameServerNodeService.setAcceptingNewMatches( + data.game_server_node_id, + data.enabled, + ), }; } @@ -1003,7 +979,7 @@ UNIT serverId, }, { - delay: 90 * 1000, + delay: MarkDedicatedServerOffline.delayFor(server), attempts: 1, removeOnFail: false, removeOnComplete: true, diff --git a/src/game-server-node/game-server-node.service.spec.ts b/src/game-server-node/game-server-node.service.spec.ts index 1c2015177..1690179a0 100644 --- a/src/game-server-node/game-server-node.service.spec.ts +++ b/src/game-server-node/game-server-node.service.spec.ts @@ -3,6 +3,7 @@ import { GameServerNodeService, GamedataValidationEntry, GamedataValidationResult, + NodeWorkloads, } from "./game-server-node.service"; const entry = ( @@ -256,3 +257,69 @@ describe("GameServerNodeService gamedata errors", () => { ).toBe("no pod was scheduled"); }); }); + +describe("GameServerNodeService.isInService", () => { + const node = (fields: Partial = {}): NodeWorkloads => ({ + enabled: true, + enabled_for_match_making: true, + gpu_streaming_enabled: true, + gpu_demos_enabled: true, + gpu_rendering_enabled: true, + servers: [], + ...fields, + }); + + const gpuMode = (fields: Partial = {}) => + node({ enabled_for_match_making: false, ...fields }); + + it("counts an enabled match node", () => { + expect(GameServerNodeService.isInService(node())).toBe(true); + }); + + it("drops a disabled node", () => { + expect(GameServerNodeService.isInService(node({ enabled: false }))).toBe( + false, + ); + }); + + it("keeps a disabled node that still hosts an enabled dedicated server", () => { + expect( + GameServerNodeService.isInService( + node({ enabled: false, servers: [{ id: "server-1" }] }), + ), + ).toBe(true); + }); + + it("counts a GPU-mode node while any GPU workload is on", () => { + expect( + GameServerNodeService.isInService( + gpuMode({ gpu_streaming_enabled: false, gpu_demos_enabled: false }), + ), + ).toBe(true); + }); + + it("drops a GPU-mode node with every workload off", () => { + expect( + GameServerNodeService.isInService( + gpuMode({ + gpu_streaming_enabled: false, + gpu_demos_enabled: false, + gpu_rendering_enabled: false, + }), + ), + ).toBe(false); + }); + + it("keeps a GPU-mode node with workloads off that hosts a dedicated server", () => { + expect( + GameServerNodeService.isInService( + gpuMode({ + gpu_streaming_enabled: false, + gpu_demos_enabled: false, + gpu_rendering_enabled: false, + servers: [{ id: "server-1" }], + }), + ), + ).toBe(true); + }); +}); diff --git a/src/game-server-node/game-server-node.service.ts b/src/game-server-node/game-server-node.service.ts index 76bf890c9..824ddcbc0 100644 --- a/src/game-server-node/game-server-node.service.ts +++ b/src/game-server-node/game-server-node.service.ts @@ -100,6 +100,15 @@ export type BuildNodeCandidate = { enabled_for_match_making: boolean | null; }; +export type NodeWorkloads = { + enabled: boolean | null; + enabled_for_match_making: boolean | null; + gpu_streaming_enabled: boolean | null; + gpu_demos_enabled: boolean | null; + gpu_rendering_enabled: boolean | null; + servers: Array; +}; + @Injectable() export class GameServerNodeService { private redis: Redis; @@ -279,6 +288,7 @@ export class GameServerNodeService { status: true, label: true, offline_at: true, + ...GameServerNodeService.inServiceSelection, lan_ip: true, node_ip: true, build_id: true, @@ -320,24 +330,20 @@ export class GameServerNodeService { status === "Online" && (storedStatus === "Offline" || storedStatus === "Setup") ) { - const { update_game_server_nodes } = await this.hasura.mutation({ - update_game_server_nodes: { - __args: { - where: { - id: { _eq: node }, - status: { _in: ["Offline", "Setup"] }, - }, - _set: { - status: "Online", - offline_at: null, - }, - }, - affected_rows: true, - }, - }); + const cameUp = await this.postgres.query>( + `UPDATE public.game_server_nodes + SET status = CASE + WHEN accepting_new_matches THEN 'Online' + ELSE 'NotAcceptingNewMatches' + END, + offline_at = NULL + WHERE id = $1 + AND status IN ('Offline', 'Setup') + RETURNING id`, + [node], + ); transitionedFromOffline = - storedStatus === "Offline" && - update_game_server_nodes.affected_rows === 1; + storedStatus === "Offline" && cameUp.length === 1; } if ( @@ -431,7 +437,37 @@ export class GameServerNodeService { const previousStatus = transitionedFromOffline ? "Offline" : storedStatus; - return { previousStatus, label, offlineAt, transitionedFromOffline }; + return { + previousStatus, + label, + offlineAt, + transitionedFromOffline, + inService: GameServerNodeService.isInService(game_server_nodes_by_pk), + }; + } + + // Kept apart from status, which reads Offline while a node is down: a node + // told to stop accepting matches has to come back that way, and toggling one + // that is down must not mark it Online. + public async setAcceptingNewMatches( + nodeId: string, + accepting: boolean, + ): Promise { + const updated = await this.postgres.query>( + `UPDATE public.game_server_nodes + SET accepting_new_matches = $2::boolean, + status = CASE + WHEN status NOT IN ('Online', 'NotAcceptingNewMatches') THEN status + WHEN $2::boolean THEN 'Online' + ELSE 'NotAcceptingNewMatches' + END + WHERE id = $1 + AND status <> 'Setup' + RETURNING id`, + [nodeId, accepting], + ); + + return updated.length === 1; } public async updateIdLabel(nodeId: string) { @@ -1607,6 +1643,45 @@ export class GameServerNodeService { return false; } + // Status alerts are only worth raising for a node something depends on. + // Disabling a node or switching it to GPU mode leaves its dedicated servers + // running, so hosting an enabled one keeps it in service regardless. `servers` + // is expected to hold only enabled dedicated servers. + public static isInService(node: NodeWorkloads): boolean { + if (node.servers.length > 0) { + return true; + } + + if (!node.enabled) { + return false; + } + + return Boolean( + node.enabled_for_match_making || + node.gpu_streaming_enabled || + node.gpu_demos_enabled || + node.gpu_rendering_enabled, + ); + } + + public static readonly inServiceSelection = { + enabled: true, + enabled_for_match_making: true, + gpu_streaming_enabled: true, + gpu_demos_enabled: true, + gpu_rendering_enabled: true, + servers: { + __args: { + where: { + is_dedicated: { _eq: true }, + enabled: { _eq: true }, + }, + limit: 1, + }, + id: true, + }, + } as const; + // Why a node cannot run a build job, or null when it can. Both jobs mount // the node's own install, so it has to be online and on the build itself. public static buildNodeIneligibility( diff --git a/src/game-server-node/jobs/CheckServerPluginVersions.spec.ts b/src/game-server-node/jobs/CheckServerPluginVersions.spec.ts new file mode 100644 index 000000000..78ecd83bf --- /dev/null +++ b/src/game-server-node/jobs/CheckServerPluginVersions.spec.ts @@ -0,0 +1,31 @@ +import { CheckServerPluginVersions } from "./CheckServerPluginVersions"; + +describe("CheckServerPluginVersions", () => { + it("only counts enabled servers as out of date", async () => { + const hasura = { + query: jest.fn(async (query: Record) => { + if (query.notifications_aggregate) { + return { notifications_aggregate: { aggregate: { count: 0 } } }; + } + if (query.plugin_versions) { + return { plugin_versions: [{ version: "2.0.0" }] }; + } + return { servers_aggregate: { aggregate: { count: 0 } } }; + }), + }; + const job = new CheckServerPluginVersions( + hasura as any, + { send: jest.fn() } as any, + { getPluginRuntime: jest.fn().mockResolvedValue("swiftlys2") } as any, + ); + + await job.process(); + + const [[servers]] = hasura.query.mock.calls.filter( + ([query]) => query.servers_aggregate, + ); + expect(servers.servers_aggregate.__args.where.enabled).toEqual({ + _eq: true, + }); + }); +}); diff --git a/src/game-server-node/jobs/CheckServerPluginVersions.ts b/src/game-server-node/jobs/CheckServerPluginVersions.ts index 6482877b5..cf8ea0703 100644 --- a/src/game-server-node/jobs/CheckServerPluginVersions.ts +++ b/src/game-server-node/jobs/CheckServerPluginVersions.ts @@ -82,6 +82,9 @@ export class CheckServerPluginVersions extends WorkerHost { connected: { _eq: true, }, + enabled: { + _eq: true, + }, // A server known to be on the other framework is waiting to be // recycled onto the selected runtime, not running an out of date // plugin. A server that has never reported one is assumed to be on diff --git a/src/game-server-node/jobs/MarkDedicatedServerOffline.spec.ts b/src/game-server-node/jobs/MarkDedicatedServerOffline.spec.ts new file mode 100644 index 000000000..12d83fba2 --- /dev/null +++ b/src/game-server-node/jobs/MarkDedicatedServerOffline.spec.ts @@ -0,0 +1,173 @@ +import { DelayedError } from "bullmq"; +import { MarkDedicatedServerOffline } from "./MarkDedicatedServerOffline"; + +type Server = { + label: string; + enabled: boolean; + is_dedicated: boolean; + game_server_node: { status: string } | null; +}; + +const server = (fields: Partial = {}): Server => ({ + label: "Retakes #1", + enabled: true, + is_dedicated: true, + game_server_node: null, + ...fields, +}); + +describe("MarkDedicatedServerOffline", () => { + let row: Server | null; + let graceRemaining: number; + let hasura: { mutation: jest.Mock }; + let notifications: { send: jest.Mock }; + let redis: { pttl: jest.Mock; set: jest.Mock }; + let job: MarkDedicatedServerOffline; + let queued: { moveToDelayed: jest.Mock }; + + beforeEach(() => { + graceRemaining = -2; + hasura = { + mutation: jest.fn(async () => ({ update_servers_by_pk: row })), + }; + notifications = { send: jest.fn().mockResolvedValue(undefined) }; + redis = { + pttl: jest.fn(async () => graceRemaining), + set: jest.fn().mockResolvedValue("OK"), + }; + queued = { moveToDelayed: jest.fn().mockResolvedValue(undefined) }; + job = new MarkDedicatedServerOffline( + hasura as any, + notifications as any, + { getConnection: () => redis } as any, + ); + }); + + const run = () => + job.process({ + data: { serverId: "server-1" }, + token: "token", + ...queued, + } as any); + + it("alerts when an enabled dedicated server stops heartbeating", async () => { + row = server(); + + await run(); + + expect(hasura.mutation).toHaveBeenCalledTimes(1); + expect(notifications.send).toHaveBeenCalledWith( + "DedicatedServerStatus", + expect.objectContaining({ title: "Dedicated Server Offline" }), + undefined, + expect.any(Number), + ); + }); + + it("marks a disabled server offline without alerting", async () => { + row = server({ enabled: false }); + + await run(); + + expect(hasura.mutation).toHaveBeenCalledTimes(1); + expect(notifications.send).not.toHaveBeenCalled(); + }); + + it("stays quiet for a match server", async () => { + row = server({ is_dedicated: false }); + + await run(); + + expect(notifications.send).not.toHaveBeenCalled(); + }); + + it("leaves a server on an offline node to the node's own alert", async () => { + row = server({ game_server_node: { status: "Offline" } }); + + await run(); + + expect(notifications.send).not.toHaveBeenCalled(); + }); + + it("alerts for a server that crashed on a node that is up", async () => { + row = server({ game_server_node: { status: "NotAcceptingNewMatches" } }); + + await run(); + + expect(notifications.send).toHaveBeenCalledTimes(1); + }); + + it("does not throw for a server deleted before the job ran", async () => { + row = null; + + await expect(run()).resolves.toBeUndefined(); + expect(notifications.send).not.toHaveBeenCalled(); + }); + + it("holds a server being restarted until the grace runs out", async () => { + row = server(); + graceRemaining = 3 * 60 * 1000; + + await expect(run()).rejects.toThrow(DelayedError); + + expect(queued.moveToDelayed).toHaveBeenCalledWith( + expect.any(Number), + "token", + ); + const [[until]] = queued.moveToDelayed.mock.calls; + expect(until - Date.now()).toBeGreaterThanOrEqual(3 * 60 * 1000); + expect(hasura.mutation).not.toHaveBeenCalled(); + expect(notifications.send).not.toHaveBeenCalled(); + }); + + it("reports a restarted server that never came back", async () => { + row = server(); + graceRemaining = -2; + + await run(); + + expect(notifications.send).toHaveBeenCalledTimes(1); + }); + + it("gives a restart five minutes", async () => { + await MarkDedicatedServerOffline.expectRestart(redis as any, "server-1"); + + expect(redis.set).toHaveBeenCalledWith( + "dedicated-servers:restarting:server-1", + "1", + "PX", + 5 * 60 * 1000, + ); + }); + + describe("delayFor", () => { + // When a node dies, its servers' last ping (every 15s) can come up to 15s + // before the node's own last ping, whose offline job then fires at +90s. + it("outlasts the node's offline timer for a node-hosted dedicated server", () => { + expect( + MarkDedicatedServerOffline.delayFor({ + is_dedicated: true, + game_server_node_id: "node-1", + }), + ).toBeGreaterThan(90 * 1000 + 15 * 1000); + }); + + it("keeps 90s for an external dedicated server", () => { + expect( + MarkDedicatedServerOffline.delayFor({ + is_dedicated: true, + game_server_node_id: null, + }), + ).toBe(90 * 1000); + }); + + it("keeps 90s for a match server", () => { + expect( + MarkDedicatedServerOffline.delayFor({ + is_dedicated: false, + game_server_node_id: "node-1", + }), + ).toBe(90 * 1000); + }); + }); +}); diff --git a/src/game-server-node/jobs/MarkDedicatedServerOffline.ts b/src/game-server-node/jobs/MarkDedicatedServerOffline.ts index b8d1259e5..7d63ab1c7 100644 --- a/src/game-server-node/jobs/MarkDedicatedServerOffline.ts +++ b/src/game-server-node/jobs/MarkDedicatedServerOffline.ts @@ -1,18 +1,36 @@ import { WorkerHost } from "@nestjs/bullmq"; import { GameServerQueues } from "../enums/GameServerQueues"; -import { Job } from "bullmq"; +import { DelayedError, Job } from "bullmq"; +import { Redis } from "ioredis"; import { HasuraService } from "../../hasura/hasura.service"; import { UseQueue } from "../../utilities/QueueProcessors"; import { NotificationsService } from "../../notifications/notifications.service"; import { DISCORD_COLORS } from "../../notifications/utilities/constants"; +import { RedisManagerService } from "../../redis/redis-manager/redis-manager.service"; + +type OfflineServer = { + enabled: boolean; + is_dedicated: boolean; + game_server_node?: { + status: string; + } | null; +}; @UseQueue("GameServerNode", GameServerQueues.NodeOffline) export class MarkDedicatedServerOffline extends WorkerHost { + // A server restarted on purpose is quiet for as long as it takes to boot, so + // its down alerts wait this out; one that never comes back is still reported. + public static readonly RESTART_GRACE_MS = 5 * 60 * 1000; + + private redis: Redis; + constructor( protected readonly hasura: HasuraService, protected readonly notifications: NotificationsService, + redisManager: RedisManagerService, ) { super(); + this.redis = redisManager.getConnection(); } async process( @@ -20,6 +38,16 @@ export class MarkDedicatedServerOffline extends WorkerHost { serverId: string; }>, ): Promise { + const grace = await MarkDedicatedServerOffline.restartGraceRemaining( + this.redis, + job.data.serverId, + ); + + if (grace > 0) { + await job.moveToDelayed(Date.now() + grace + 5 * 1000, job.token); + throw new DelayedError(); + } + const { update_servers_by_pk } = await this.hasura.mutation({ update_servers_by_pk: { __args: { @@ -32,11 +60,15 @@ export class MarkDedicatedServerOffline extends WorkerHost { }, }, label: true, + enabled: true, is_dedicated: true, + game_server_node: { + status: true, + }, }, }); - if (!update_servers_by_pk.is_dedicated) { + if (!MarkDedicatedServerOffline.shouldNotify(update_servers_by_pk)) { return; } @@ -52,4 +84,56 @@ export class MarkDedicatedServerOffline extends WorkerHost { DISCORD_COLORS.RED, ); } + + public static async expectRestart( + redis: Redis, + serverId: string, + ): Promise { + await redis.set( + MarkDedicatedServerOffline.restartGraceKey(serverId), + "1", + "PX", + MarkDedicatedServerOffline.RESTART_GRACE_MS, + ); + } + + public static async restartGraceRemaining( + redis: Redis, + serverId: string, + ): Promise { + const remaining = await redis.pttl( + MarkDedicatedServerOffline.restartGraceKey(serverId), + ); + + return Math.max(remaining, 0); + } + + private static restartGraceKey(serverId: string): string { + return `dedicated-servers:restarting:${serverId}`; + } + + // A node-hosted dedicated server waits out the node's own 90s timer (the node + // pings every 30s, plugins every 15s), so when the whole node dies it is + // already marked Offline and shouldNotify leaves the outage to the node. + public static delayFor(server: { + is_dedicated: boolean; + game_server_node_id: string | null; + }): number { + if (server.is_dedicated && server.game_server_node_id) { + return 120 * 1000; + } + + return 90 * 1000; + } + + // Disabling a server tears it down on purpose. A server on a node that is + // down is the node's outage, which the node reports: a node hosting an enabled + // dedicated server always counts as in service. + public static shouldNotify(server: OfflineServer | null): boolean { + if (!server?.is_dedicated || !server.enabled) { + return false; + } + + return server.game_server_node?.status !== "Offline"; + } } diff --git a/src/game-server-node/jobs/MarkGameServerNodeOffline.spec.ts b/src/game-server-node/jobs/MarkGameServerNodeOffline.spec.ts new file mode 100644 index 000000000..6fcfb9409 --- /dev/null +++ b/src/game-server-node/jobs/MarkGameServerNodeOffline.spec.ts @@ -0,0 +1,236 @@ +import { MarkGameServerNodeOffline } from "./MarkGameServerNodeOffline"; + +type Node = { + status: string; + enabled: boolean; + enabled_for_match_making: boolean; + gpu_streaming_enabled: boolean; + gpu_demos_enabled: boolean; + gpu_rendering_enabled: boolean; + servers: Array<{ id: string }>; + region: string | null; +}; + +const matchNode = (fields: Partial = {}): Node => ({ + status: "Online", + enabled: true, + enabled_for_match_making: true, + gpu_streaming_enabled: true, + gpu_demos_enabled: true, + gpu_rendering_enabled: true, + servers: [], + region: "us-east", + ...fields, +}); + +const gpuNode = (fields: Partial = {}): Node => + matchNode({ enabled_for_match_making: false, region: null, ...fields }); + +describe("MarkGameServerNodeOffline", () => { + let node: Node | null; + let regionBefore: string; + let regionAfter: string; + let lastRegionAlert: string | null; + let stuckMatches: Array>; + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let notifications: { send: jest.Mock; latestTitle: jest.Mock }; + let job: MarkGameServerNodeOffline; + + beforeEach(() => { + regionBefore = "Online"; + regionAfter = "Partial"; + lastRegionAlert = null; + stuckMatches = []; + + hasura = { + query: jest.fn(async (query: Record) => { + if (query.game_server_nodes_by_pk) { + return { + game_server_nodes_by_pk: node && { + ...node, + e_region: node.region ? { status: regionBefore } : null, + }, + }; + } + if (query.server_regions_by_pk) { + return { + server_regions_by_pk: { + value: "us-east", + description: "US East", + status: regionAfter, + }, + }; + } + if (query.matches) { + return { matches: stuckMatches }; + } + if (query.server_regions) { + return { + server_regions: [{ value: "us-east", status: regionAfter }], + }; + } + throw new Error(`unexpected query ${Object.keys(query)}`); + }), + mutation: jest.fn(async (mutation: Record) => { + if (mutation.update_game_server_nodes_by_pk) { + return { + update_game_server_nodes_by_pk: node && { + label: "node-1", + region: node.region, + }, + }; + } + return { update_notifications: { __typename: "x" } }; + }), + }; + notifications = { + send: jest.fn().mockResolvedValue(undefined), + latestTitle: jest.fn(async () => lastRegionAlert), + }; + job = new MarkGameServerNodeOffline(hasura as any, notifications as any); + }); + + const run = () => job.process({ data: { node: "node-1" } } as any); + + const markedOffline = () => + hasura.mutation.mock.calls.some( + ([mutation]) => + mutation.update_game_server_nodes_by_pk?.__args._set.status === + "Offline", + ); + + const sentTitles = () => + notifications.send.mock.calls.map(([, notification]) => notification.title); + + it("alerts when a node in service goes offline", async () => { + node = matchNode(); + + await run(); + + expect(markedOffline()).toBe(true); + expect(sentTitles()).toEqual(["Game Server Node Offline"]); + }); + + it("marks a disabled node offline without alerting", async () => { + node = matchNode({ enabled: false }); + + await run(); + + expect(markedOffline()).toBe(true); + expect(sentTitles()).toEqual([]); + }); + + it("alerts for a disabled node still hosting an enabled dedicated server", async () => { + node = matchNode({ enabled: false, servers: [{ id: "server-1" }] }); + + await run(); + + expect(sentTitles()).toEqual(["Game Server Node Offline"]); + }); + + it("asks only for enabled dedicated servers when deciding", async () => { + node = matchNode(); + + await run(); + + const [[query]] = hasura.query.mock.calls; + expect(query.game_server_nodes_by_pk.servers.__args.where).toEqual({ + is_dedicated: { _eq: true }, + enabled: { _eq: true }, + }); + }); + + it("stays quiet for a GPU node with every workload turned off", async () => { + node = gpuNode({ + gpu_streaming_enabled: false, + gpu_demos_enabled: false, + gpu_rendering_enabled: false, + }); + + await run(); + + expect(markedOffline()).toBe(true); + expect(sentTitles()).toEqual([]); + }); + + it("alerts for a GPU node still taking a workload", async () => { + node = gpuNode({ gpu_demos_enabled: false, gpu_rendering_enabled: false }); + + await run(); + + expect(sentTitles()).toEqual(["Game Server Node Offline"]); + }); + + it("does nothing for a node that is already offline", async () => { + node = matchNode({ status: "Offline" }); + + await run(); + + expect(markedOffline()).toBe(false); + expect(sentTitles()).toEqual([]); + }); + + it("does nothing for a node that no longer exists", async () => { + node = null; + + await run(); + + expect(sentTitles()).toEqual([]); + }); + + it("raises the region alert when this node takes the region down", async () => { + node = matchNode(); + regionAfter = "Offline"; + + await run(); + + expect(sentTitles()).toEqual([ + "Game Server Node Offline", + "Region Offline", + ]); + }); + + it("does not repeat a region alert already raised", async () => { + node = matchNode({ status: "NotAcceptingNewMatches" }); + regionBefore = "Offline"; + regionAfter = "Offline"; + lastRegionAlert = "Region Offline"; + + await run(); + + expect(notifications.latestTitle).toHaveBeenCalledWith( + "GameNodeStatus", + "us-east", + ["Region Offline", "Region Online"], + ); + expect(sentTitles()).toEqual(["Game Server Node Offline"]); + }); + + it("raises the region alert for a region that went offline unannounced", async () => { + node = matchNode({ status: "NotAcceptingNewMatches" }); + regionBefore = "Offline"; + regionAfter = "Offline"; + lastRegionAlert = "Region Online"; + stuckMatches = [ + { id: "match-1", region: "us-east", status: "Scheduled", options: null }, + ]; + + await run(); + + expect(sentTitles()).toEqual([ + "Game Server Node Offline", + "Region Offline", + "Match stuck: no regions available", + ]); + }); + + it("raises no region alert for a disabled node in an offline region", async () => { + node = matchNode({ enabled: false }); + regionBefore = "Offline"; + regionAfter = "Offline"; + + await run(); + + expect(sentTitles()).toEqual([]); + }); +}); diff --git a/src/game-server-node/jobs/MarkGameServerNodeOffline.ts b/src/game-server-node/jobs/MarkGameServerNodeOffline.ts index 6a409e6ad..66d2fd6de 100644 --- a/src/game-server-node/jobs/MarkGameServerNodeOffline.ts +++ b/src/game-server-node/jobs/MarkGameServerNodeOffline.ts @@ -5,6 +5,7 @@ import { HasuraService } from "../../hasura/hasura.service"; import { UseQueue } from "../../utilities/QueueProcessors"; import { NotificationsService } from "../../notifications/notifications.service"; import { DISCORD_COLORS } from "../../notifications/utilities/constants"; +import { GameServerNodeService } from "../game-server-node.service"; @UseQueue("GameServerNode", GameServerQueues.NodeOffline) export class MarkGameServerNodeOffline extends WorkerHost { @@ -20,6 +21,23 @@ export class MarkGameServerNodeOffline extends WorkerHost { node: string; }>, ): Promise { + const { game_server_nodes_by_pk: node } = await this.hasura.query({ + game_server_nodes_by_pk: { + __args: { + id: job.data.node, + }, + status: true, + ...GameServerNodeService.inServiceSelection, + e_region: { + status: true, + }, + }, + }); + + if (!node || node.status === "Offline") { + return; + } + const { update_game_server_nodes_by_pk } = await this.hasura.mutation({ update_game_server_nodes_by_pk: { __args: { @@ -57,7 +75,10 @@ export class MarkGameServerNodeOffline extends WorkerHost { }, }); - if (!update_game_server_nodes_by_pk) { + if ( + !update_game_server_nodes_by_pk || + !GameServerNodeService.isInService(node) + ) { return; } @@ -91,6 +112,18 @@ export class MarkGameServerNodeOffline extends WorkerHost { return; } + // A region can read Offline without anyone being told (its only node + // stopped accepting matches), so an unchanged status alone is no repeat. + if ( + node.e_region?.status === "Offline" && + (await this.notifications.latestTitle("GameNodeStatus", region, [ + "Region Offline", + "Region Online", + ])) === "Region Offline" + ) { + return; + } + await this.notifications.send( "GameNodeStatus", { diff --git a/src/game-server-node/jobs/MarkGameServerNodeOnline.spec.ts b/src/game-server-node/jobs/MarkGameServerNodeOnline.spec.ts new file mode 100644 index 000000000..c1fa1e881 --- /dev/null +++ b/src/game-server-node/jobs/MarkGameServerNodeOnline.spec.ts @@ -0,0 +1,119 @@ +import { MarkGameServerNodeOnline } from "./MarkGameServerNodeOnline"; + +describe("MarkGameServerNodeOnline", () => { + let region: string | null; + let regionStatus: string; + let lastAlerts: Record; + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let notifications: { send: jest.Mock; latestTitle: jest.Mock }; + let job: MarkGameServerNodeOnline; + + beforeEach(() => { + region = "us-east"; + regionStatus = "Online"; + lastAlerts = {}; + + hasura = { + query: jest.fn(async (query: Record) => { + if (query.game_server_nodes_by_pk) { + return { game_server_nodes_by_pk: { region } }; + } + if (query.server_regions_by_pk) { + return { + server_regions_by_pk: { + value: "us-east", + description: "US East", + status: regionStatus, + }, + }; + } + throw new Error(`unexpected query ${Object.keys(query)}`); + }), + mutation: jest + .fn() + .mockResolvedValue({ update_notifications: { __typename: "x" } }), + }; + notifications = { + send: jest.fn().mockResolvedValue(undefined), + latestTitle: jest.fn( + async (_type: string, entityId: string) => lastAlerts[entityId] ?? null, + ), + }; + job = new MarkGameServerNodeOnline(hasura as any, notifications as any); + }); + + const run = () => + job.process({ data: { node: "node-1", label: "node-1" } } as any); + + const sentTitles = () => + notifications.send.mock.calls.map(([, notification]) => notification.title); + + it("closes the node's offline alert", async () => { + lastAlerts["node-1"] = "Game Server Node Offline"; + + await run(); + + expect(notifications.latestTitle).toHaveBeenCalledWith( + "GameNodeStatus", + "node-1", + ["Game Server Node Offline", "Game Server Node Online"], + ); + expect(sentTitles()).toEqual(["Game Server Node Online"]); + }); + + it("stays quiet for a node whose outage was never announced", async () => { + await run(); + + expect(sentTitles()).toEqual([]); + }); + + it("does not announce a node twice", async () => { + lastAlerts["node-1"] = "Game Server Node Online"; + + await run(); + + expect(sentTitles()).toEqual([]); + }); + + it("closes the region's offline alert once the region is back", async () => { + lastAlerts["node-1"] = "Game Server Node Offline"; + lastAlerts["us-east"] = "Region Offline"; + + await run(); + + expect(notifications.latestTitle).toHaveBeenCalledWith( + "GameNodeStatus", + "us-east", + ["Region Offline", "Region Online"], + ); + expect(sentTitles()).toEqual(["Game Server Node Online", "Region Online"]); + }); + + it("does not announce a region that was never reported offline", async () => { + lastAlerts["node-1"] = "Game Server Node Offline"; + regionStatus = "Partial"; + + await run(); + + expect(sentTitles()).toEqual(["Game Server Node Online"]); + }); + + it("does not announce a region twice", async () => { + lastAlerts["node-1"] = "Game Server Node Offline"; + lastAlerts["us-east"] = "Region Online"; + + await run(); + + expect(sentTitles()).toEqual(["Game Server Node Online"]); + }); + + it("does not announce a region that is still offline", async () => { + lastAlerts["node-1"] = "Game Server Node Offline"; + lastAlerts["us-east"] = "Region Offline"; + regionStatus = "Offline"; + + await run(); + + expect(sentTitles()).toEqual(["Game Server Node Online"]); + }); +}); diff --git a/src/game-server-node/jobs/MarkGameServerNodeOnline.ts b/src/game-server-node/jobs/MarkGameServerNodeOnline.ts index 8e2702aad..04d4e8b93 100644 --- a/src/game-server-node/jobs/MarkGameServerNodeOnline.ts +++ b/src/game-server-node/jobs/MarkGameServerNodeOnline.ts @@ -15,6 +15,9 @@ export class MarkGameServerNodeOnline extends WorkerHost { super(); } + // "Back online" only ever closes an offline alert. A node that went down + // unannounced (disabled, out of service) comes back unannounced, and a node + // reconnecting in a healthy region says nothing about the region. async process( job: Job<{ node: string; @@ -22,40 +25,52 @@ export class MarkGameServerNodeOnline extends WorkerHost { offlineAt?: string; }>, ): Promise { - const nodeLabel = NotificationsService.escapeHtml( - job.data.label || job.data.node, - ); - let message = `Game Server Node (${nodeLabel}) is back Online.`; + const { game_server_nodes_by_pk: node } = await this.hasura.query({ + game_server_nodes_by_pk: { + __args: { id: job.data.node }, + region: true, + }, + }); - if (job.data.offlineAt) { - const offlineDuration = Math.round( - (Date.now() - new Date(job.data.offlineAt).getTime()) / 60000, - ); - if (offlineDuration > 0) { - message += ` Was offline for ${offlineDuration} minute${offlineDuration !== 1 ? "s" : ""}.`; - } + if (!node) { + return; } - await this.notifications.send( + const lastNodeAlert = await this.notifications.latestTitle( "GameNodeStatus", - { - message, - title: "Game Server Node Online", - role: "administrator", - entity_id: job.data.node, - }, - undefined, - DISCORD_COLORS.GREEN, + job.data.node, + ["Game Server Node Offline", "Game Server Node Online"], ); - const { game_server_nodes_by_pk } = await this.hasura.query({ - game_server_nodes_by_pk: { - __args: { id: job.data.node }, - region: true, - }, - }); + if (lastNodeAlert === "Game Server Node Offline") { + const nodeLabel = NotificationsService.escapeHtml( + job.data.label || job.data.node, + ); + let message = `Game Server Node (${nodeLabel}) is back Online.`; + + if (job.data.offlineAt) { + const offlineDuration = Math.round( + (Date.now() - new Date(job.data.offlineAt).getTime()) / 60000, + ); + if (offlineDuration > 0) { + message += ` Was offline for ${offlineDuration} minute${offlineDuration !== 1 ? "s" : ""}.`; + } + } + + await this.notifications.send( + "GameNodeStatus", + { + message, + title: "Game Server Node Online", + role: "administrator", + entity_id: job.data.node, + }, + undefined, + DISCORD_COLORS.GREEN, + ); + } - const region = game_server_nodes_by_pk?.region; + const region = node.region; if (!region) { return; } @@ -77,6 +92,16 @@ export class MarkGameServerNodeOnline extends WorkerHost { return; } + const lastRegionAlert = await this.notifications.latestTitle( + "GameNodeStatus", + region, + ["Region Offline", "Region Online"], + ); + + if (lastRegionAlert !== "Region Offline") { + return; + } + await this.hasura.mutation({ update_notifications: { __args: { diff --git a/src/notifications/notifications.service.spec.ts b/src/notifications/notifications.service.spec.ts index 2ab240cdc..c78caae0d 100644 --- a/src/notifications/notifications.service.spec.ts +++ b/src/notifications/notifications.service.spec.ts @@ -449,3 +449,59 @@ describe("NotificationsService", () => { }); }); }); + +describe("latestTitle", () => { + it("reads the newest matching alert for the entity, dismissed ones included", async () => { + const hasura = { + query: jest + .fn() + .mockResolvedValue({ notifications: [{ title: "Region Offline" }] }), + }; + const service = new NotificationsService( + hasura as any, + {} as any, + { log: jest.fn(), warn: jest.fn(), error: jest.fn() } as any, + { get: () => ({ webDomain: "https://5stack.gg" }) } as any, + {} as any, + {} as any, + {} as any, + {} as any, + ); + + await expect( + service.latestTitle("GameNodeStatus", "us-east", [ + "Region Offline", + "Region Online", + ]), + ).resolves.toBe("Region Offline"); + + expect(hasura.query.mock.calls[0][0].notifications.__args).toEqual({ + where: { + type: { _eq: "GameNodeStatus" }, + entity_id: { _eq: "us-east" }, + title: { _in: ["Region Offline", "Region Online"] }, + }, + order_by: [{ created_at: "desc" }], + limit: 1, + }); + }); + + it("is null when the entity has never alerted", async () => { + const service = new NotificationsService( + { query: jest.fn().mockResolvedValue({ notifications: [] }) } as any, + {} as any, + { log: jest.fn(), warn: jest.fn(), error: jest.fn() } as any, + { get: () => ({ webDomain: "https://5stack.gg" }) } as any, + {} as any, + {} as any, + {} as any, + {} as any, + ); + + await expect( + service.latestTitle("GameNodeStatus", "node-1", [ + "Game Server Node Offline", + ]), + ).resolves.toBeNull(); + }); +}); diff --git a/src/notifications/notifications.service.ts b/src/notifications/notifications.service.ts index 758d2add5..7211dec33 100644 --- a/src/notifications/notifications.service.ts +++ b/src/notifications/notifications.service.ts @@ -313,6 +313,31 @@ export class NotificationsService { this.logger.log(`notified warned player ${sanction.steamId}`); } + // The newest of an entity's paired alerts (offline/online). Dismissed rows + // count: deleting an alert does not undo the condition it reported. + async latestTitle( + type: e_notification_types_enum, + entityId: string, + titles: Array, + ): Promise { + const { notifications } = await this.hasura.query({ + notifications: { + __args: { + where: { + type: { _eq: type }, + entity_id: { _eq: entityId }, + title: { _in: titles }, + }, + order_by: [{ created_at: "desc" }], + limit: 1, + }, + title: true, + }, + }); + + return notifications.at(0)?.title ?? null; + } + async send( type: e_notification_types_enum, notification: { diff --git a/src/rcon/rcon.service.spec.ts b/src/rcon/rcon.service.spec.ts new file mode 100644 index 000000000..0587a834d --- /dev/null +++ b/src/rcon/rcon.service.spec.ts @@ -0,0 +1,89 @@ +import { RconService } from "./rcon.service"; + +jest.mock("rcon-client", () => ({ + Rcon: jest.fn().mockImplementation(() => { + const client: Record = { + authenticated: false, + connect: jest.fn().mockRejectedValue(new Error("ECONNREFUSED")), + end: jest.fn(), + }; + client.on = jest.fn(() => client); + client.off = jest.fn(() => client); + return client; + }), +})); + +describe("RconService connect failure", () => { + let hasura: { query: jest.Mock; mutation: jest.Mock }; + let notifications: { send: jest.Mock }; + let service: RconService; + + const dedicatedServer = (enabled: boolean, type = "Ranked") => ({ + host: "10.0.0.1", + port: 27015, + type, + label: "Retakes #1", + region: "us-east", + enabled, + is_dedicated: true, + rcon_status: true, + rcon_password: "secret", + game_server_node: null as null, + }); + + beforeEach(() => { + hasura = { + query: jest.fn(), + mutation: jest.fn().mockResolvedValue({}), + }; + notifications = { send: jest.fn().mockResolvedValue(undefined) }; + service = new RconService( + hasura as any, + { decrypt: jest.fn().mockResolvedValue("secret") } as any, + notifications as any, + { warn: jest.fn(), log: jest.fn(), error: jest.fn() } as any, + {} as any, + {} as any, + {} as any, + ); + }); + + it("alerts when an enabled Ranked server's RCON goes down", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: dedicatedServer(true) }); + + await service.connect("server-1"); + + expect(notifications.send).toHaveBeenCalledWith( + "DedicatedServerRconStatus", + expect.objectContaining({ title: "Dedicated Server RCON Error" }), + undefined, + expect.any(Number), + ); + }); + + it("records the failure without alerting for a disabled server", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: dedicatedServer(false) }); + + await service.connect("server-1"); + + expect(hasura.mutation).toHaveBeenCalledWith( + expect.objectContaining({ + update_servers_by_pk: expect.objectContaining({ + __args: expect.objectContaining({ _set: { rcon_status: false } }), + }), + }), + ); + expect(notifications.send).not.toHaveBeenCalled(); + }); + + it("leaves other dedicated servers to the minute ping", async () => { + hasura.query.mockResolvedValue({ + servers_by_pk: dedicatedServer(true, "Retake"), + }); + + await service.connect("server-1"); + + expect(hasura.mutation).toHaveBeenCalled(); + expect(notifications.send).not.toHaveBeenCalled(); + }); +}); diff --git a/src/rcon/rcon.service.ts b/src/rcon/rcon.service.ts index d3689b04a..cbf730260 100644 --- a/src/rcon/rcon.service.ts +++ b/src/rcon/rcon.service.ts @@ -93,8 +93,10 @@ export class RconService { }, host: true, port: true, + type: true, label: true, region: true, + enabled: true, is_dedicated: true, rcon_status: true, rcon_password: true, @@ -210,17 +212,21 @@ export class RconService { }, }); - void this.notifications.send( - "DedicatedServerRconStatus", - { - message: `Dedicated Server (${NotificationsService.escapeHtml(server.label || serverId)}) is not able to connect to the RCON.`, - title: "Dedicated Server RCON Error", - role: "administrator", - entity_id: serverId, - }, - undefined, - DISCORD_COLORS.RED, - ); + // Every other dedicated server is watched by PingDedicatedServers, which + // only reports one that stays unreachable. + if (server.enabled && server.type === "Ranked") { + void this.notifications.send( + "DedicatedServerRconStatus", + { + message: `Dedicated Server (${NotificationsService.escapeHtml(server.label || serverId)}) is not able to connect to the RCON.`, + title: "Dedicated Server RCON Error", + role: "administrator", + entity_id: serverId, + }, + undefined, + DISCORD_COLORS.RED, + ); + } } return; } diff --git a/test/node-scheduling.spec.ts b/test/node-scheduling.spec.ts new file mode 100644 index 000000000..a8953b62e --- /dev/null +++ b/test/node-scheduling.spec.ts @@ -0,0 +1,192 @@ +import { Logger } from "@nestjs/common"; +import { PostgresService } from "./../src/postgres/postgres.service"; +import { GameServerNodeService } from "./../src/game-server-node/game-server-node.service"; +import { GameServerNodeController } from "./../src/game-server-node/game-server-node.controller"; +import { bootMigratedDb, SqlTestDb } from "./utils/sql-test-db"; + +// "Accepting new matches" used to live only in status, which the offline job +// overwrites with Offline: a node told to stop taking matches came back from any +// outage taking them again, and toggling a node that was down marked it Online. +describe("node scheduling across an outage (SQL-driven)", () => { + let db: SqlTestDb; + let postgres: PostgresService; + let nodes: GameServerNodeService; + let controller: GameServerNodeController; + + beforeAll(async () => { + db = await bootMigratedDb("NodeSchedulingTest"); + postgres = db.postgres; + await postgres.query( + `INSERT INTO server_regions (value, description) VALUES ('SchedRegion', 'SchedRegion') + ON CONFLICT (value) DO NOTHING`, + ); + }, 600_000); + + afterAll(async () => { + await db?.stop(); + }); + + // No GraphQL engine runs here, so node reads come straight from the table and + // only the status columns of a node update are written back; that is all + // these paths change. + const hasura = { + query: async (query: Record) => { + const [row] = await postgres.query>>( + `SELECT * FROM game_server_nodes WHERE id = $1`, + [query.game_server_nodes_by_pk.__args.id], + ); + return { game_server_nodes_by_pk: row ? { ...row, servers: [] } : null }; + }, + mutation: async (mutation: Record) => { + const byPk = mutation.update_game_server_nodes_by_pk; + const bulk = mutation.update_game_server_nodes; + const args = byPk?.__args ?? bulk?.__args; + if (!args) { + return {}; + } + + const id = byPk ? args.pk_columns.id : args.where.id._eq; + const onlyFrom: Array | null = bulk + ? (args.where.status?._in ?? null) + : null; + const set = Object.entries(args._set).filter(([column]) => + ["status", "offline_at"].includes(column), + ); + if (set.length === 0) { + return { update_game_server_nodes: { affected_rows: 0 } }; + } + + const updated = await postgres.query>( + `UPDATE game_server_nodes + SET ${set.map(([column], index) => `${column} = $${index + 3}`).join(", ")} + WHERE id = $1 AND ($2::text[] IS NULL OR status = ANY($2::text[])) + RETURNING id`, + [id, onlyFrom, ...set.map(([, value]) => value)], + ); + return { update_game_server_nodes: { affected_rows: updated.length } }; + }, + }; + + beforeEach(async () => { + await postgres.query("DELETE FROM servers"); + await postgres.query("DELETE FROM game_server_nodes"); + await postgres.query( + `INSERT INTO game_server_nodes (id, region, status, enabled, label) + VALUES ('sched-node', 'SchedRegion', 'Online', true, 'sched-node')`, + ); + + nodes = new GameServerNodeService( + new Logger("NodeSchedulingTest"), + { get: () => ({ namespace: "5stack" }) } as never, + hasura as never, + { getConnection: () => ({}) } as never, + {} as never, + {} as never, + {} as never, + {} as never, + postgres, + {} as never, + ); + + controller = new GameServerNodeController( + new Logger("NodeSchedulingTest"), + {} as never, + { get: () => ({}) } as never, + hasura as never, + {} as never, + {} as never, + {} as never, + nodes, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + {} as never, + ); + }); + + const status = async () => { + const [row] = await postgres.query>( + `SELECT status FROM game_server_nodes WHERE id = 'sched-node'`, + ); + return row.status; + }; + + const goOffline = () => + postgres.query( + `UPDATE game_server_nodes SET status = 'Offline', offline_at = now() + WHERE id = 'sched-node'`, + ); + + const ping = () => + nodes.updateStatus( + "sched-node", + "10.0.0.2", + "10.0.0.2", + "203.0.113.2", + undefined, + undefined, + false, + false, + { sockets: 1, coresPerSocket: 4, threadsPerCore: 2 } as never, + { governor: "performance", cpus: {} }, + { frequency: 0, cpus: {} }, + { count: 0, devices: null }, + "Online", + ); + + const schedule = (enabled: boolean) => + controller.setGameNodeSchedulingState({ + game_server_node_id: "sched-node", + enabled, + }); + + it("stops and resumes taking matches on a node that is up", async () => { + await schedule(false); + expect(await status()).toBe("NotAcceptingNewMatches"); + + await schedule(true); + expect(await status()).toBe("Online"); + }); + + it("brings a node that was not accepting matches back the same way", async () => { + await schedule(false); + await goOffline(); + + const result = await ping(); + + expect(result?.transitionedFromOffline).toBe(true); + expect(await status()).toBe("NotAcceptingNewMatches"); + }); + + it("brings an accepting node back Online", async () => { + await goOffline(); + + await ping(); + + expect(await status()).toBe("Online"); + }); + + it("does not mark a node that is down Online when scheduling is turned on", async () => { + await schedule(false); + await goOffline(); + + await schedule(true); + + expect(await status()).toBe("Offline"); + await ping(); + expect(await status()).toBe("Online"); + }); + + it("leaves a node still in setup alone", async () => { + await postgres.query( + `UPDATE game_server_nodes SET status = 'Setup' WHERE id = 'sched-node'`, + ); + + expect(await schedule(false)).toEqual({ success: false }); + expect(await status()).toBe("Setup"); + }); +}); From b313b7e68605ed42ebcba45608ec45482a3666c1 Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Wed, 30 Sep 2026 05:43:48 -0400 Subject: [PATCH 2/3] bug: review fixes for server offline alerts - a server on a node that died is checked again once the node is back, so one that never returns is still reported - offline state is written right away; only the alert waits out a restart - Ranked RCON alerts respect the restart grace - unreachable streak restarts after a gap in pings, tolerates a missing entry, and only records an alert once it was sent --- .../dedicated-servers.service.spec.ts | 70 ++++++++++++++- .../dedicated-servers.service.ts | 42 ++++++--- .../jobs/MarkDedicatedServerOffline.spec.ts | 87 ++++++++++++------- .../jobs/MarkDedicatedServerOffline.ts | 80 +++++++++-------- src/rcon/rcon.service.spec.ts | 17 +++- src/rcon/rcon.service.ts | 13 ++- 6 files changed, 224 insertions(+), 85 deletions(-) diff --git a/src/dedicated-servers/dedicated-servers.service.spec.ts b/src/dedicated-servers/dedicated-servers.service.spec.ts index ae8c61da2..0ac3302f4 100644 --- a/src/dedicated-servers/dedicated-servers.service.spec.ts +++ b/src/dedicated-servers/dedicated-servers.service.spec.ts @@ -4,13 +4,14 @@ import { DedicatedServersService } from "./dedicated-servers.service"; // to interleave: both removed, one created, and the other's AlreadyExists // handler deleted the deployment the first had just created. describe("DedicatedServersService.rebuildDedicatedServer", () => { + const redis = { set: jest.fn() }; const service = new DedicatedServersService( { log: jest.fn(), error: jest.fn(), verbose: jest.fn() } as never, { get: () => ({ namespace: "5stack" }) } as never, null as never, null as never, null as never, - { getConnection: () => ({ set: jest.fn() }) } as never, + { getConnection: () => redis } as never, null as never, null as never, null as never, @@ -23,6 +24,7 @@ describe("DedicatedServersService.rebuildDedicatedServer", () => { beforeEach(() => { steps.length = 0; + redis.set.mockClear(); jest .spyOn(service, "removeDedicatedServer") @@ -71,6 +73,23 @@ describe("DedicatedServersService.rebuildDedicatedServer", () => { expect(steps).toEqual(["remove a"]); }); + it("gives a server that will start again time to boot", async () => { + await service.rebuildDedicatedServer("a"); + + expect(redis.set).toHaveBeenCalledWith( + "dedicated-servers:restarting:a", + "1", + "PX", + 5 * 60 * 1000, + ); + }); + + it("gives no boot time to a server being taken down", async () => { + await service.rebuildDedicatedServer("a", false); + + expect(redis.set).not.toHaveBeenCalled(); + }); + it("keeps going after a rebuild that failed", async () => { jest .spyOn(service, "setupDedicatedServer") @@ -288,13 +307,12 @@ describe("DedicatedServersService.pingDedicatedServer", () => { await service.restartDedicatedServer("server-1"); reachable = false; - for (let minute = 0; minute < 4; minute++) { + for (let minute = 0; minute < 5; minute++) { await service.pingDedicatedServer("server-1"); minutes(1); } expect(alerts()).toBe(0); - minutes(1); await service.pingDedicatedServer("server-1"); expect(alerts()).toBe(1); }); @@ -311,6 +329,52 @@ describe("DedicatedServersService.pingDedicatedServer", () => { expect(alerts()).toBe(0); }); + + it("starts the clock over after a gap in pings", async () => { + reachable = false; + + await service.pingDedicatedServer("server-1"); + minutes(5); + await service.pingDedicatedServer("server-1"); + minutes(1); + await service.pingDedicatedServer("server-1"); + expect(alerts()).toBe(0); + + minutes(1); + await service.pingDedicatedServer("server-1"); + expect(alerts()).toBe(1); + }); + + it("tries the alert again when sending it failed", async () => { + reachable = false; + notifications.send.mockRejectedValueOnce(new Error("discord down")); + + await service.pingDedicatedServer("server-1"); + minutes(1); + await service.pingDedicatedServer("server-1"); + minutes(1); + await expect(service.pingDedicatedServer("server-1")).rejects.toThrow( + "discord down", + ); + minutes(1); + await service.pingDedicatedServer("server-1"); + minutes(1); + await service.pingDedicatedServer("server-1"); + + expect(alerts()).toBe(2); + }); + + it("forgets the streak of a server that is removed", async () => { + (service as any).apps = { deleteNamespacedDeployment: jest.fn() }; + reachable = false; + + await service.pingDedicatedServer("server-1"); + await service.removeDedicatedServer("server-1"); + + expect( + await redis.hget("dedicated-servers:unreachable", "server-1"), + ).toBeNull(); + }); }); describe("DedicatedServersService.pluginInstallEnvironment", () => { diff --git a/src/dedicated-servers/dedicated-servers.service.ts b/src/dedicated-servers/dedicated-servers.service.ts index 021e95c34..a5240c99c 100644 --- a/src/dedicated-servers/dedicated-servers.service.ts +++ b/src/dedicated-servers/dedicated-servers.service.ts @@ -17,6 +17,12 @@ import { NotificationsService } from "src/notifications/notifications.service"; import { DISCORD_COLORS } from "src/notifications/utilities/constants"; import { MarkDedicatedServerOffline } from "src/game-server-node/jobs/MarkDedicatedServerOffline"; +type UnreachableStreak = { + since: number; + last: number; + reported: boolean; +}; + @Injectable() export class DedicatedServersService { private static readonly rebuilds = new Map>(); @@ -26,6 +32,8 @@ export class DedicatedServersService { // server is reported once it has stayed unreachable this long. public static readonly UNREACHABLE_ALERT_AFTER_MS = 2 * 60 * 1000; + private static readonly UNREACHABLE_GAP_MS = 90 * 1000; + private static readonly UNREACHABLE_KEY = "dedicated-servers:unreachable"; private appConfig: AppConfig; @@ -921,15 +929,25 @@ export class DedicatedServersService { server: { label: string; enabled: boolean; connected: boolean }, ): Promise { const now = Date.now(); + const previous: UnreachableStreak | null = JSON.parse( + (await this.redis.hget( + DedicatedServersService.UNREACHABLE_KEY, + serverId, + )) ?? "null", + ); + + // Pings run every minute. A longer gap means nothing was watching, not that + // the server stayed down the whole time. + const streak: UnreachableStreak = + previous && + now - previous.last <= DedicatedServersService.UNREACHABLE_GAP_MS + ? { ...previous, last: now } + : { since: now, last: now, reported: false }; - await this.redis.hsetnx( + await this.redis.hset( DedicatedServersService.UNREACHABLE_KEY, serverId, - JSON.stringify({ since: now, reported: false }), - ); - - const streak: { since: number; reported: boolean } = JSON.parse( - await this.redis.hget(DedicatedServersService.UNREACHABLE_KEY, serverId), + JSON.stringify(streak), ); if ( @@ -962,12 +980,6 @@ export class DedicatedServersService { return; } - await this.redis.hset( - DedicatedServersService.UNREACHABLE_KEY, - serverId, - JSON.stringify({ ...streak, reported: true }), - ); - await this.notifications.send( "DedicatedServerRconStatus", { @@ -979,6 +991,12 @@ export class DedicatedServersService { undefined, DISCORD_COLORS.RED, ); + + await this.redis.hset( + DedicatedServersService.UNREACHABLE_KEY, + serverId, + JSON.stringify({ ...streak, reported: true }), + ); } public async restartDedicatedServer(serverId: string): Promise { diff --git a/src/game-server-node/jobs/MarkDedicatedServerOffline.spec.ts b/src/game-server-node/jobs/MarkDedicatedServerOffline.spec.ts index 12d83fba2..675c4cc4c 100644 --- a/src/game-server-node/jobs/MarkDedicatedServerOffline.spec.ts +++ b/src/game-server-node/jobs/MarkDedicatedServerOffline.spec.ts @@ -5,6 +5,7 @@ type Server = { label: string; enabled: boolean; is_dedicated: boolean; + offline_at: string | null; game_server_node: { status: string } | null; }; @@ -12,28 +13,37 @@ const server = (fields: Partial = {}): Server => ({ label: "Retakes #1", enabled: true, is_dedicated: true, + offline_at: null, game_server_node: null, ...fields, }); describe("MarkDedicatedServerOffline", () => { let row: Server | null; - let graceRemaining: number; - let hasura: { mutation: jest.Mock }; + let now: number; + let graceUntil: number | null; + let hasura: { query: jest.Mock; mutation: jest.Mock }; let notifications: { send: jest.Mock }; let redis: { pttl: jest.Mock; set: jest.Mock }; - let job: MarkDedicatedServerOffline; let queued: { moveToDelayed: jest.Mock }; + let job: MarkDedicatedServerOffline; beforeEach(() => { - graceRemaining = -2; + now = Date.now(); + graceUntil = null; hasura = { - mutation: jest.fn(async () => ({ update_servers_by_pk: row })), + query: jest.fn(async () => ({ servers_by_pk: row && { ...row } })), + mutation: jest.fn().mockResolvedValue({}), }; notifications = { send: jest.fn().mockResolvedValue(undefined) }; redis = { - pttl: jest.fn(async () => graceRemaining), - set: jest.fn().mockResolvedValue("OK"), + pttl: jest.fn(async () => + graceUntil && graceUntil > now ? graceUntil - now : -2, + ), + set: jest.fn(async (_key: string, _value: string, _px: string, ms) => { + graceUntil = now + ms; + return "OK"; + }), }; queued = { moveToDelayed: jest.fn().mockResolvedValue(undefined) }; job = new MarkDedicatedServerOffline( @@ -50,12 +60,18 @@ describe("MarkDedicatedServerOffline", () => { ...queued, } as any); + const offlineWrite = () => + hasura.mutation.mock.calls[0]?.[0].update_servers_by_pk.__args._set; + it("alerts when an enabled dedicated server stops heartbeating", async () => { row = server(); await run(); - expect(hasura.mutation).toHaveBeenCalledTimes(1); + expect(offlineWrite()).toEqual({ + connected: false, + offline_at: expect.any(String), + }); expect(notifications.send).toHaveBeenCalledWith( "DedicatedServerStatus", expect.objectContaining({ title: "Dedicated Server Offline" }), @@ -69,7 +85,7 @@ describe("MarkDedicatedServerOffline", () => { await run(); - expect(hasura.mutation).toHaveBeenCalledTimes(1); + expect(offlineWrite().connected).toBe(false); expect(notifications.send).not.toHaveBeenCalled(); }); @@ -81,14 +97,6 @@ describe("MarkDedicatedServerOffline", () => { expect(notifications.send).not.toHaveBeenCalled(); }); - it("leaves a server on an offline node to the node's own alert", async () => { - row = server({ game_server_node: { status: "Offline" } }); - - await run(); - - expect(notifications.send).not.toHaveBeenCalled(); - }); - it("alerts for a server that crashed on a node that is up", async () => { row = server({ game_server_node: { status: "NotAcceptingNewMatches" } }); @@ -104,40 +112,59 @@ describe("MarkDedicatedServerOffline", () => { expect(notifications.send).not.toHaveBeenCalled(); }); - it("holds a server being restarted until the grace runs out", async () => { + it("keeps the time the server first went offline", async () => { + row = server({ offline_at: "2026-09-30T10:00:00.000Z" }); + + await run(); + + expect(offlineWrite().offline_at).toBe("2026-09-30T10:00:00.000Z"); + }); + + it("marks a restarting server offline but holds the alert until the grace runs out", async () => { row = server(); - graceRemaining = 3 * 60 * 1000; + graceUntil = now + 3 * 60 * 1000; await expect(run()).rejects.toThrow(DelayedError); - expect(queued.moveToDelayed).toHaveBeenCalledWith( - expect.any(Number), - "token", - ); - const [[until]] = queued.moveToDelayed.mock.calls; - expect(until - Date.now()).toBeGreaterThanOrEqual(3 * 60 * 1000); - expect(hasura.mutation).not.toHaveBeenCalled(); + expect(offlineWrite().connected).toBe(false); + const [[until, token]] = queued.moveToDelayed.mock.calls; + expect(token).toBe("token"); + expect(until - Date.now()).toBeGreaterThanOrEqual(3 * 60 * 1000 - 1000); expect(notifications.send).not.toHaveBeenCalled(); }); - it("reports a restarted server that never came back", async () => { + it("reports a restarted server that is still down once the grace has run out", async () => { row = server(); - graceRemaining = -2; + graceUntil = now + 3 * 60 * 1000; + await expect(run()).rejects.toThrow(DelayedError); + now += 3 * 60 * 1000 + 5 * 1000; await run(); expect(notifications.send).toHaveBeenCalledTimes(1); }); - it("gives a restart five minutes", async () => { - await MarkDedicatedServerOffline.expectRestart(redis as any, "server-1"); + it("leaves an outage to the node, then reports the server if it never came back", async () => { + row = server({ game_server_node: { status: "Offline" } }); + await expect(run()).rejects.toThrow(DelayedError); expect(redis.set).toHaveBeenCalledWith( "dedicated-servers:restarting:server-1", "1", "PX", 5 * 60 * 1000, ); + expect(notifications.send).not.toHaveBeenCalled(); + + now += 5 * 60 * 1000; + await expect(run()).rejects.toThrow(DelayedError); + expect(notifications.send).not.toHaveBeenCalled(); + + row.game_server_node = { status: "Online" }; + now += 5 * 60 * 1000 + 5 * 1000; + await run(); + + expect(notifications.send).toHaveBeenCalledTimes(1); }); describe("delayFor", () => { diff --git a/src/game-server-node/jobs/MarkDedicatedServerOffline.ts b/src/game-server-node/jobs/MarkDedicatedServerOffline.ts index 7d63ab1c7..b425b318e 100644 --- a/src/game-server-node/jobs/MarkDedicatedServerOffline.ts +++ b/src/game-server-node/jobs/MarkDedicatedServerOffline.ts @@ -8,14 +8,6 @@ import { NotificationsService } from "../../notifications/notifications.service" import { DISCORD_COLORS } from "../../notifications/utilities/constants"; import { RedisManagerService } from "../../redis/redis-manager/redis-manager.service"; -type OfflineServer = { - enabled: boolean; - is_dedicated: boolean; - game_server_node?: { - status: string; - } | null; -}; - @UseQueue("GameServerNode", GameServerQueues.NodeOffline) export class MarkDedicatedServerOffline extends WorkerHost { // A server restarted on purpose is quiet for as long as it takes to boot, so @@ -38,17 +30,26 @@ export class MarkDedicatedServerOffline extends WorkerHost { serverId: string; }>, ): Promise { - const grace = await MarkDedicatedServerOffline.restartGraceRemaining( - this.redis, - job.data.serverId, - ); + const { servers_by_pk: server } = await this.hasura.query({ + servers_by_pk: { + __args: { + id: job.data.serverId, + }, + label: true, + enabled: true, + is_dedicated: true, + offline_at: true, + game_server_node: { + status: true, + }, + }, + }); - if (grace > 0) { - await job.moveToDelayed(Date.now() + grace + 5 * 1000, job.token); - throw new DelayedError(); + if (!server) { + return; } - const { update_servers_by_pk } = await this.hasura.mutation({ + await this.hasura.mutation({ update_servers_by_pk: { __args: { pk_columns: { @@ -56,26 +57,42 @@ export class MarkDedicatedServerOffline extends WorkerHost { }, _set: { connected: false, - offline_at: new Date().toISOString(), + offline_at: server.offline_at ?? new Date().toISOString(), }, }, - label: true, - enabled: true, - is_dedicated: true, - game_server_node: { - status: true, - }, + __typename: true, }, }); - if (!MarkDedicatedServerOffline.shouldNotify(update_servers_by_pk)) { + // Disabling a server tears it down on purpose. + if (!server.is_dedicated || !server.enabled) { return; } + // A server on a node that is down is the node's outage, which the node + // reports. It is looked at again once the node is back and the server has + // had time to boot, so one that never returns is still reported. + if (server.game_server_node?.status === "Offline") { + await MarkDedicatedServerOffline.expectRestart( + this.redis, + job.data.serverId, + ); + } + + const grace = await MarkDedicatedServerOffline.restartGraceRemaining( + this.redis, + job.data.serverId, + ); + + if (grace > 0) { + await job.moveToDelayed(Date.now() + grace + 5 * 1000, job.token); + throw new DelayedError(); + } + await this.notifications.send( "DedicatedServerStatus", { - message: `Dedicated Server (${NotificationsService.escapeHtml(update_servers_by_pk.label || job.data.serverId)}) is Offline.`, + message: `Dedicated Server (${NotificationsService.escapeHtml(server.label || job.data.serverId)}) is Offline.`, title: "Dedicated Server Offline", role: "administrator", entity_id: job.data.serverId, @@ -114,7 +131,7 @@ export class MarkDedicatedServerOffline extends WorkerHost { // A node-hosted dedicated server waits out the node's own 90s timer (the node // pings every 30s, plugins every 15s), so when the whole node dies it is - // already marked Offline and shouldNotify leaves the outage to the node. + // already marked Offline by the time its servers are looked at. public static delayFor(server: { is_dedicated: boolean; game_server_node_id: string | null; @@ -125,15 +142,4 @@ export class MarkDedicatedServerOffline extends WorkerHost { return 90 * 1000; } - - // Disabling a server tears it down on purpose. A server on a node that is - // down is the node's outage, which the node reports: a node hosting an enabled - // dedicated server always counts as in service. - public static shouldNotify(server: OfflineServer | null): boolean { - if (!server?.is_dedicated || !server.enabled) { - return false; - } - - return server.game_server_node?.status !== "Offline"; - } } diff --git a/src/rcon/rcon.service.spec.ts b/src/rcon/rcon.service.spec.ts index 0587a834d..181fd881f 100644 --- a/src/rcon/rcon.service.spec.ts +++ b/src/rcon/rcon.service.spec.ts @@ -31,7 +31,10 @@ describe("RconService connect failure", () => { game_server_node: null as null, }); + let graceRemaining: number; + beforeEach(() => { + graceRemaining = -2; hasura = { query: jest.fn(), mutation: jest.fn().mockResolvedValue({}), @@ -43,7 +46,9 @@ describe("RconService connect failure", () => { notifications as any, { warn: jest.fn(), log: jest.fn(), error: jest.fn() } as any, {} as any, - {} as any, + { + getConnection: () => ({ pttl: jest.fn(async () => graceRemaining) }), + } as any, {} as any, ); }); @@ -86,4 +91,14 @@ describe("RconService connect failure", () => { expect(hasura.mutation).toHaveBeenCalled(); expect(notifications.send).not.toHaveBeenCalled(); }); + + it("stays quiet while a Ranked server is restarting", async () => { + hasura.query.mockResolvedValue({ servers_by_pk: dedicatedServer(true) }); + graceRemaining = 2 * 60 * 1000; + + await service.connect("server-1"); + + expect(hasura.mutation).toHaveBeenCalled(); + expect(notifications.send).not.toHaveBeenCalled(); + }); }); diff --git a/src/rcon/rcon.service.ts b/src/rcon/rcon.service.ts index cbf730260..73b2e3816 100644 --- a/src/rcon/rcon.service.ts +++ b/src/rcon/rcon.service.ts @@ -9,6 +9,7 @@ import { RedisManagerService } from "../redis/redis-manager/redis-manager.servic import { CacheService } from "../cache/cache.service"; import { User } from "../auth/types/User"; import { isRoleAbove } from "../utilities/isRoleAbove"; +import { MarkDedicatedServerOffline } from "../game-server-node/jobs/MarkDedicatedServerOffline"; @Injectable() export class RconService { @@ -213,8 +214,16 @@ export class RconService { }); // Every other dedicated server is watched by PingDedicatedServers, which - // only reports one that stays unreachable. - if (server.enabled && server.type === "Ranked") { + // only reports one that stays unreachable. A server restarted on purpose + // is still booting. + if ( + server.enabled && + server.type === "Ranked" && + (await MarkDedicatedServerOffline.restartGraceRemaining( + this.redisManager.getConnection(), + serverId, + )) === 0 + ) { void this.notifications.send( "DedicatedServerRconStatus", { From 3ab710e4fbf5a1b4b547a683ee4a981f44655aff Mon Sep 17 00:00:00 2001 From: Luke Policinski Date: Wed, 30 Sep 2026 05:45:54 -0400 Subject: [PATCH 3/3] bug: type node scheduling test stub, expose accepting_new_matches to admins --- .../databases/default/tables/public_game_server_nodes.yaml | 1 + test/node-scheduling.spec.ts | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) 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 ea967817a..f8c5995fb 100644 --- a/hasura/metadata/databases/default/tables/public_game_server_nodes.yaml +++ b/hasura/metadata/databases/default/tables/public_game_server_nodes.yaml @@ -61,6 +61,7 @@ select_permissions: - role: administrator permission: columns: + - accepting_new_matches - build_id - cpu_cores_per_socket - cpu_frequency_info diff --git a/test/node-scheduling.spec.ts b/test/node-scheduling.spec.ts index a8953b62e..d5d69aff2 100644 --- a/test/node-scheduling.spec.ts +++ b/test/node-scheduling.spec.ts @@ -35,7 +35,8 @@ describe("node scheduling across an outage (SQL-driven)", () => { `SELECT * FROM game_server_nodes WHERE id = $1`, [query.game_server_nodes_by_pk.__args.id], ); - return { game_server_nodes_by_pk: row ? { ...row, servers: [] } : null }; + const servers: Array<{ id: string }> = []; + return { game_server_nodes_by_pk: row ? { ...row, servers } : null }; }, mutation: async (mutation: Record) => { const byPk = mutation.update_game_server_nodes_by_pk;