fix(SEC-ORCH-16): Implement real health and readiness checks
- Add ping() method to ValkeyClient and ValkeyService for health checks - Update HealthService to check Valkey connectivity before reporting ready - /health/ready now returns 503 if dependencies are unhealthy - Add detailed checks object showing individual dependency status - Update tests with ValkeyService mock Refs #339 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -1,13 +1,21 @@
|
|||||||
import { describe, it, expect, beforeEach } from "vitest";
|
import { describe, it, expect, beforeEach, vi } from "vitest";
|
||||||
|
import { HttpException, HttpStatus } from "@nestjs/common";
|
||||||
import { HealthController } from "./health.controller";
|
import { HealthController } from "./health.controller";
|
||||||
import { HealthService } from "./health.service";
|
import { HealthService } from "./health.service";
|
||||||
|
import { ValkeyService } from "../../valkey/valkey.service";
|
||||||
|
|
||||||
|
// Mock ValkeyService
|
||||||
|
const mockValkeyService = {
|
||||||
|
ping: vi.fn(),
|
||||||
|
} as unknown as ValkeyService;
|
||||||
|
|
||||||
describe("HealthController", () => {
|
describe("HealthController", () => {
|
||||||
let controller: HealthController;
|
let controller: HealthController;
|
||||||
let service: HealthService;
|
let service: HealthService;
|
||||||
|
|
||||||
beforeEach(() => {
|
beforeEach(() => {
|
||||||
service = new HealthService();
|
vi.clearAllMocks();
|
||||||
|
service = new HealthService(mockValkeyService);
|
||||||
controller = new HealthController(service);
|
controller = new HealthController(service);
|
||||||
});
|
});
|
||||||
|
|
||||||
@@ -83,17 +91,46 @@ describe("HealthController", () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
describe("GET /health/ready", () => {
|
describe("GET /health/ready", () => {
|
||||||
it("should return ready status", () => {
|
it("should return ready status with checks when all dependencies are healthy", async () => {
|
||||||
const result = controller.ready();
|
vi.mocked(mockValkeyService.ping).mockResolvedValue(true);
|
||||||
|
|
||||||
|
const result = await controller.ready();
|
||||||
|
|
||||||
expect(result).toBeDefined();
|
expect(result).toBeDefined();
|
||||||
expect(result).toHaveProperty("ready");
|
expect(result).toHaveProperty("ready");
|
||||||
|
expect(result).toHaveProperty("checks");
|
||||||
|
expect(result.ready).toBe(true);
|
||||||
|
expect(result.checks.valkey).toBe(true);
|
||||||
});
|
});
|
||||||
|
|
||||||
it("should return ready as true", () => {
|
it("should throw 503 when Valkey is unhealthy", async () => {
|
||||||
const result = controller.ready();
|
vi.mocked(mockValkeyService.ping).mockResolvedValue(false);
|
||||||
|
|
||||||
expect(result.ready).toBe(true);
|
await expect(controller.ready()).rejects.toThrow(HttpException);
|
||||||
|
|
||||||
|
try {
|
||||||
|
await controller.ready();
|
||||||
|
} catch (error) {
|
||||||
|
expect(error).toBeInstanceOf(HttpException);
|
||||||
|
expect((error as HttpException).getStatus()).toBe(HttpStatus.SERVICE_UNAVAILABLE);
|
||||||
|
const response = (error as HttpException).getResponse() as { ready: boolean };
|
||||||
|
expect(response.ready).toBe(false);
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should return checks object with individual dependency status", async () => {
|
||||||
|
vi.mocked(mockValkeyService.ping).mockResolvedValue(true);
|
||||||
|
|
||||||
|
const result = await controller.ready();
|
||||||
|
|
||||||
|
expect(result.checks).toBeDefined();
|
||||||
|
expect(typeof result.checks.valkey).toBe("boolean");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("should handle Valkey ping errors gracefully", async () => {
|
||||||
|
vi.mocked(mockValkeyService.ping).mockRejectedValue(new Error("Connection refused"));
|
||||||
|
|
||||||
|
await expect(controller.ready()).rejects.toThrow(HttpException);
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -1,6 +1,6 @@
|
|||||||
import { Controller, Get, UseGuards } from "@nestjs/common";
|
import { Controller, Get, UseGuards, HttpStatus, HttpException } from "@nestjs/common";
|
||||||
import { Throttle } from "@nestjs/throttler";
|
import { Throttle } from "@nestjs/throttler";
|
||||||
import { HealthService } from "./health.service";
|
import { HealthService, ReadinessResult } from "./health.service";
|
||||||
import { OrchestratorThrottlerGuard } from "../../common/guards/throttler.guard";
|
import { OrchestratorThrottlerGuard } from "../../common/guards/throttler.guard";
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -26,8 +26,13 @@ export class HealthController {
|
|||||||
|
|
||||||
@Get("ready")
|
@Get("ready")
|
||||||
@Throttle({ status: { limit: 200, ttl: 60000 } })
|
@Throttle({ status: { limit: 200, ttl: 60000 } })
|
||||||
ready(): { ready: boolean } {
|
async ready(): Promise<ReadinessResult> {
|
||||||
// NOTE: Check Valkey connection, Docker daemon (see issue #TBD)
|
const result = await this.healthService.isReady();
|
||||||
return { ready: true };
|
|
||||||
|
if (!result.ready) {
|
||||||
|
throw new HttpException(result, HttpStatus.SERVICE_UNAVAILABLE);
|
||||||
|
}
|
||||||
|
|
||||||
|
return result;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -1,8 +1,10 @@
|
|||||||
import { Module } from "@nestjs/common";
|
import { Module } from "@nestjs/common";
|
||||||
import { HealthController } from "./health.controller";
|
import { HealthController } from "./health.controller";
|
||||||
import { HealthService } from "./health.service";
|
import { HealthService } from "./health.service";
|
||||||
|
import { ValkeyModule } from "../../valkey/valkey.module";
|
||||||
|
|
||||||
@Module({
|
@Module({
|
||||||
|
imports: [ValkeyModule],
|
||||||
controllers: [HealthController],
|
controllers: [HealthController],
|
||||||
providers: [HealthService],
|
providers: [HealthService],
|
||||||
})
|
})
|
||||||
|
|||||||
@@ -1,14 +1,56 @@
|
|||||||
import { Injectable } from "@nestjs/common";
|
import { Injectable, Logger } from "@nestjs/common";
|
||||||
|
import { ValkeyService } from "../../valkey/valkey.service";
|
||||||
|
|
||||||
|
export interface ReadinessResult {
|
||||||
|
ready: boolean;
|
||||||
|
checks: {
|
||||||
|
valkey: boolean;
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
export class HealthService {
|
export class HealthService {
|
||||||
private readonly startTime: number;
|
private readonly startTime: number;
|
||||||
|
private readonly logger = new Logger(HealthService.name);
|
||||||
|
|
||||||
constructor() {
|
constructor(private readonly valkeyService: ValkeyService) {
|
||||||
this.startTime = Date.now();
|
this.startTime = Date.now();
|
||||||
}
|
}
|
||||||
|
|
||||||
getUptime(): number {
|
getUptime(): number {
|
||||||
return Math.floor((Date.now() - this.startTime) / 1000);
|
return Math.floor((Date.now() - this.startTime) / 1000);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check if the service is ready to accept requests
|
||||||
|
* Validates connectivity to required dependencies
|
||||||
|
*/
|
||||||
|
async isReady(): Promise<ReadinessResult> {
|
||||||
|
const valkeyReady = await this.checkValkey();
|
||||||
|
|
||||||
|
const ready = valkeyReady;
|
||||||
|
|
||||||
|
if (!ready) {
|
||||||
|
this.logger.warn(`Readiness check failed: valkey=${String(valkeyReady)}`);
|
||||||
|
}
|
||||||
|
|
||||||
|
return {
|
||||||
|
ready,
|
||||||
|
checks: {
|
||||||
|
valkey: valkeyReady,
|
||||||
|
},
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
private async checkValkey(): Promise<boolean> {
|
||||||
|
try {
|
||||||
|
return await this.valkeyService.ping();
|
||||||
|
} catch (error) {
|
||||||
|
this.logger.error(
|
||||||
|
"Valkey health check failed",
|
||||||
|
error instanceof Error ? error.message : String(error)
|
||||||
|
);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -71,6 +71,19 @@ export class ValkeyClient {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check Valkey connectivity
|
||||||
|
* @returns true if connection is healthy, false otherwise
|
||||||
|
*/
|
||||||
|
async ping(): Promise<boolean> {
|
||||||
|
try {
|
||||||
|
await this.client.ping();
|
||||||
|
return true;
|
||||||
|
} catch {
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* Task State Management
|
* Task State Management
|
||||||
*/
|
*/
|
||||||
|
|||||||
@@ -152,4 +152,12 @@ export class ValkeyService implements OnModuleDestroy {
|
|||||||
};
|
};
|
||||||
await this.setAgentState(state);
|
await this.setAgentState(state);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Check Valkey connectivity
|
||||||
|
* @returns true if connection is healthy, false otherwise
|
||||||
|
*/
|
||||||
|
async ping(): Promise<boolean> {
|
||||||
|
return this.client.ping();
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user