feature: clean up Kubernetes leftovers of removed game server nodes

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!
 }

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

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

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<CleanupRemovedNodesOutput> = {}) => ({
+    nodes: 0,
+    jobs: 0,
+    volume_claims: 0,
+    volumes: 0,
+    failed: 0,
+    node_delete_forbidden: false,
+    recently_ready: 0,
+    ...counts,
+  });
+
+  const removed = (old: Record<string, unknown>) =>
+    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();
+  });
+});

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<AppConfig>("app");
   }
@@ -813,6 +816,46 @@ UNIT
     );
   }
 
+  @HasuraAction()
+  public async cleanupRemovedNodes() {
+    return await this.nodeCleanup.cleanupRemovedNodes();
+  }
+
+  @HasuraEvent()
+  public async game_server_node_removed(
+    data: HasuraEventData<game_server_nodes_set_input>,
+  ) {
+    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;

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(),
   ],

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";
     }