From 20612532b4466648d00acab3a6361f5f248372c9 Mon Sep 17 00:00:00 2001 From: utkarsh patrikar Date: Sat, 23 May 2026 19:35:38 +0530 Subject: [PATCH 1/3] feat: add live watch mode for kdm health --- src/__tests__/health.test.ts | 52 ++++++++++++++++ src/commands/health.ts | 115 ++++++++++++++++++++++++++++------- 2 files changed, 146 insertions(+), 21 deletions(-) diff --git a/src/__tests__/health.test.ts b/src/__tests__/health.test.ts index 38f80c6..5db42cb 100644 --- a/src/__tests__/health.test.ts +++ b/src/__tests__/health.test.ts @@ -192,4 +192,56 @@ describe('health command', () => { }), ); }); + + it('should register watch and interval options', () => { + const healthCmd = program.commands.find((c) => c.name() === 'health'); + const watchOption = healthCmd?.options.find((o) => o.short === '-w'); + const intervalOption = healthCmd?.options.find((o) => o.short === '-i'); + expect(watchOption).toBeDefined(); + expect(intervalOption).toBeDefined(); + }); + + it('should log error on invalid interval', async () => { + await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '-1']); + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining('Invalid interval'), + ); + }); + + it('should poll health status periodically in watch mode', async () => { + vi.useFakeTimers(); + const writeSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + + vi.mocked(getRunningContainers).mockResolvedValue([ + { id: '1', name: 'web', image: 'nginx', state: 'running', status: 'Up 2 hours' }, + ]); + vi.mocked(getRunningPods).mockResolvedValue([]); + + // Parse to run the watch mode command + await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '3']); + + expect(tableUtils.renderTable).toHaveBeenCalledTimes(1); + expect(writeSpy).toHaveBeenCalledWith('\x1Bc'); + + // Change mock status to verify the next iteration + vi.mocked(getRunningContainers).mockResolvedValue([ + { id: '1', name: 'web', image: 'nginx', state: 'exited', status: 'Exited' }, + ]); + + // Advance timer by 3 seconds (3000ms) + await vi.advanceTimersByTimeAsync(3000); + + expect(tableUtils.renderTable).toHaveBeenCalledTimes(2); + expect(tableUtils.renderTable).toHaveBeenLastCalledWith( + expect.objectContaining({ + rows: expect.arrayContaining([ + expect.arrayContaining(['container', 'web', expect.stringContaining('exited')]), + ]), + }), + ); + + // Clean up + writeSpy.mockRestore(); + vi.useRealTimers(); + }); }); \ No newline at end of file diff --git a/src/commands/health.ts b/src/commands/health.ts index 49695c9..827ce64 100644 --- a/src/commands/health.ts +++ b/src/commands/health.ts @@ -32,6 +32,11 @@ import { logger } from '../utils/logger'; import { createSpinner } from '../ui/spinner'; import { renderTable } from '../ui/table'; +export interface HealthOptions { + watch?: boolean; + interval?: string; +} + const healthColor = (status: string): string => { if (status === 'healthy' || status === 'running' || status === 'Running') { return chalk.green(status); @@ -42,19 +47,7 @@ const healthColor = (status: string): string => { return chalk.yellow(status); }; -export const showHealth = async (target: string): Promise => { - logger.info?.(`Showing health for ${target}...`); - - const validTargets = ['all', 'containers', 'pods']; - if (!validTargets.includes(target)) { - logger.error?.( - `Unknown target: ${target}. Valid targets are: ${validTargets.join(', ')}.`, - ); - process.exitCode = 1; - return; - } - - const spinner = createSpinner(`Checking ${target} health...`).start(); +const fetchHealthRows = async (target: string): Promise<(string | number)[][]> => { const rows: (string | number)[][] = []; if (target === 'all' || target === 'containers') { @@ -91,17 +84,93 @@ export const showHealth = async (target: string): Promise => { } } - spinner.stop(); + return rows; +}; + +export const showHealth = async (target: string, options: HealthOptions = {}): Promise => { + logger.info?.(`Showing health for ${target}...`); - if (rows.length === 0) { - logger.warn?.(`No ${target === 'all' ? 'workloads' : target} found.`); + const validTargets = ['all', 'containers', 'pods']; + if (!validTargets.includes(target)) { + logger.error?.( + `Unknown target: ${target}. Valid targets are: ${validTargets.join(', ')}.`, + ); + process.exitCode = 1; return; } - renderTable({ - head: ['TYPE', 'NAME', 'HEALTH', 'DETAILS'], - rows, - }); + if (options.watch) { + const intervalSeconds = parseInt(options.interval || '5', 10); + if (isNaN(intervalSeconds) || intervalSeconds <= 0) { + logger.error?.('Invalid interval. Please provide a positive number of seconds.'); + process.exitCode = 1; + return; + } + + let isRunning = true; + let timer: NodeJS.Timeout | undefined; + + const cleanup = () => { + isRunning = false; + if (timer) { + clearTimeout(timer); + } + }; + + const sigintHandler = () => { + cleanup(); + process.exit(0); + }; + + if (process.env.NODE_ENV !== 'test') { + process.on('SIGINT', sigintHandler); + process.on('SIGTERM', sigintHandler); + } + + const poll = async () => { + if (!isRunning) return; + + const rows = await fetchHealthRows(target); + + // Clear terminal screen + process.stdout.write('\x1Bc'); + + const timestamp = new Date().toLocaleTimeString(); + console.log( + chalk.bold.cyan(`[KDM Health] Target: ${target} | Last updated: ${timestamp} (Interval: ${intervalSeconds}s)`) + ); + console.log(chalk.dim('Press Ctrl+C to exit\n')); + + if (rows.length === 0) { + logger.warn?.(`No ${target === 'all' ? 'workloads' : target} found.`); + } else { + renderTable({ + head: ['TYPE', 'NAME', 'HEALTH', 'DETAILS'], + rows, + }); + } + + if (isRunning) { + timer = setTimeout(poll, intervalSeconds * 1000); + } + }; + + await poll(); + } else { + const spinner = createSpinner(`Checking ${target} health...`).start(); + const rows = await fetchHealthRows(target); + spinner.stop(); + + if (rows.length === 0) { + logger.warn?.(`No ${target === 'all' ? 'workloads' : target} found.`); + return; + } + + renderTable({ + head: ['TYPE', 'NAME', 'HEALTH', 'DETAILS'], + rows, + }); + } }; export const registerHealthCommand = (program: Command): void => { @@ -111,5 +180,9 @@ export const registerHealthCommand = (program: Command): void => { 'Show health status for pods, containers, or all workloads.\n' + 'Valid targets: all | containers | pods', ) - .action(showHealth); + .option('-w, --watch', 'Watch mode: continuously refresh health output') + .option('-i, --interval ', 'Refresh interval in seconds', '5') + .action(async (target, options) => { + await showHealth(target, options); + }); }; \ No newline at end of file From d778e615ef142ca45e9a174ddf73733c6b406fa4 Mon Sep 17 00:00:00 2001 From: Utkarsh patrikar <137105846+utkarsh232005@users.noreply.github.com> Date: Sat, 23 May 2026 19:39:20 +0530 Subject: [PATCH 2/3] Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- src/commands/health.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/commands/health.ts b/src/commands/health.ts index 827ce64..85ced52 100644 --- a/src/commands/health.ts +++ b/src/commands/health.ts @@ -136,10 +136,10 @@ export const showHealth = async (target: string, options: HealthOptions = {}): P process.stdout.write('\x1Bc'); const timestamp = new Date().toLocaleTimeString(); - console.log( + logger.info?.( chalk.bold.cyan(`[KDM Health] Target: ${target} | Last updated: ${timestamp} (Interval: ${intervalSeconds}s)`) ); - console.log(chalk.dim('Press Ctrl+C to exit\n')); + logger.info?.(chalk.dim('Press Ctrl+C to exit\n')); if (rows.length === 0) { logger.warn?.(`No ${target === 'all' ? 'workloads' : target} found.`); From 9e4975e9a3b55d65e89559a403e3f84324563ee9 Mon Sep 17 00:00:00 2001 From: utkarsh patrikar Date: Sat, 23 May 2026 19:47:14 +0530 Subject: [PATCH 3/3] refactor: address review feedback - parallel fetching, AbortSignal cancellation, strict validation --- src/__tests__/health.test.ts | 46 ++++++++++++++++-- src/commands/health.ts | 93 +++++++++++++++++++++++++----------- 2 files changed, 109 insertions(+), 30 deletions(-) diff --git a/src/__tests__/health.test.ts b/src/__tests__/health.test.ts index 5db42cb..5ee784b 100644 --- a/src/__tests__/health.test.ts +++ b/src/__tests__/health.test.ts @@ -42,7 +42,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; import { Command } from 'commander'; -import { registerHealthCommand } from '../commands/health'; +import { registerHealthCommand, showHealth } from '../commands/health'; import { getRunningContainers } from '../docker/containers'; import { getRunningPods } from '../kubernetes/pods'; import { logger } from '../utils/logger'; @@ -201,9 +201,19 @@ describe('health command', () => { expect(intervalOption).toBeDefined(); }); - it('should log error on invalid interval', async () => { + it('should reject non-integer and malformed intervals strictly', async () => { await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '-1']); - expect(logger.error).toHaveBeenCalledWith( + expect(logger.error).toHaveBeenLastCalledWith( + expect.stringContaining('Invalid interval'), + ); + + await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '3.5']); + expect(logger.error).toHaveBeenLastCalledWith( + expect.stringContaining('Invalid interval'), + ); + + await program.parseAsync(['node', 'test', 'health', 'all', '--watch', '--interval', '3abc']); + expect(logger.error).toHaveBeenLastCalledWith( expect.stringContaining('Invalid interval'), ); }); @@ -244,4 +254,34 @@ describe('health command', () => { writeSpy.mockRestore(); vi.useRealTimers(); }); + + it('should stop watch loop when AbortSignal is aborted', async () => { + vi.useFakeTimers(); + const writeSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + + vi.mocked(getRunningContainers).mockResolvedValue([ + { id: '1', name: 'web', image: 'nginx', state: 'running', status: 'Up 2 hours' }, + ]); + vi.mocked(getRunningPods).mockResolvedValue([]); + + const controller = new AbortController(); + + // Call showHealth directly with the signal and await the first poll tick + await showHealth('all', { watch: true, interval: '3', signal: controller.signal }); + + expect(tableUtils.renderTable).toHaveBeenCalledTimes(1); + + // Now abort the controller + controller.abort(); + + // Advance timers by 3 seconds + await vi.advanceTimersByTimeAsync(3000); + + // It should NOT call renderTable again because it was aborted + expect(tableUtils.renderTable).toHaveBeenCalledTimes(1); + + // Clean up + writeSpy.mockRestore(); + vi.useRealTimers(); + }); }); \ No newline at end of file diff --git a/src/commands/health.ts b/src/commands/health.ts index 85ced52..c49b871 100644 --- a/src/commands/health.ts +++ b/src/commands/health.ts @@ -35,6 +35,7 @@ import { renderTable } from '../ui/table'; export interface HealthOptions { watch?: boolean; interval?: string; + signal?: AbortSignal; } const healthColor = (status: string): string => { @@ -49,37 +50,46 @@ const healthColor = (status: string): string => { const fetchHealthRows = async (target: string): Promise<(string | number)[][]> => { const rows: (string | number)[][] = []; + const shouldFetchContainers = target === 'all' || target === 'containers'; + const shouldFetchPods = target === 'all' || target === 'pods'; - if (target === 'all' || target === 'containers') { - try { - const containers = await getRunningContainers(); + const [containerResult, podResult] = await Promise.allSettled([ + shouldFetchContainers ? getRunningContainers() : Promise.resolve([]), + shouldFetchPods ? getRunningPods() : Promise.resolve([]), + ]); + + if (shouldFetchContainers) { + if (containerResult.status === 'fulfilled') { rows.push( - ...containers.map((container) => [ + ...containerResult.value.map((container) => [ 'container', container.name, healthColor(container.state), container.status, ]), ); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); + } else { + const message = containerResult.reason instanceof Error + ? containerResult.reason.message + : String(containerResult.reason); logger.warn?.(`Docker unavailable: ${message}`); } } - if (target === 'all' || target === 'pods') { - try { - const pods = await getRunningPods(); + if (shouldFetchPods) { + if (podResult.status === 'fulfilled') { rows.push( - ...pods.map((pod) => [ + ...podResult.value.map((pod) => [ 'pod', pod.name, healthColor(pod.status), `namespace: ${pod.namespace}, restarts: ${pod.restarts}`, ]), ); - } catch (error) { - const message = error instanceof Error ? error.message : String(error); + } else { + const message = podResult.reason instanceof Error + ? podResult.reason.message + : String(podResult.reason); logger.warn?.(`Kubernetes unavailable: ${message}`); } } @@ -100,9 +110,15 @@ export const showHealth = async (target: string, options: HealthOptions = {}): P } if (options.watch) { - const intervalSeconds = parseInt(options.interval || '5', 10); - if (isNaN(intervalSeconds) || intervalSeconds <= 0) { - logger.error?.('Invalid interval. Please provide a positive number of seconds.'); + const intervalStr = options.interval || '5'; + if (!/^\d+$/.test(intervalStr)) { + logger.error?.('Invalid interval. Please provide a positive integer number of seconds.'); + process.exitCode = 1; + return; + } + const intervalSeconds = parseInt(intervalStr, 10); + if (intervalSeconds <= 0) { + logger.error?.('Invalid interval. Please provide a positive integer number of seconds.'); process.exitCode = 1; return; } @@ -115,23 +131,27 @@ export const showHealth = async (target: string, options: HealthOptions = {}): P if (timer) { clearTimeout(timer); } + if (options.signal) { + options.signal.removeEventListener('abort', cleanup); + } }; - const sigintHandler = () => { - cleanup(); - process.exit(0); - }; - - if (process.env.NODE_ENV !== 'test') { - process.on('SIGINT', sigintHandler); - process.on('SIGTERM', sigintHandler); + if (options.signal) { + if (options.signal.aborted) { + cleanup(); + return; + } + options.signal.addEventListener('abort', cleanup); } const poll = async () => { - if (!isRunning) return; + if (!isRunning || (options.signal && options.signal.aborted)) { + cleanup(); + return; + } const rows = await fetchHealthRows(target); - + // Clear terminal screen process.stdout.write('\x1Bc'); @@ -150,8 +170,10 @@ export const showHealth = async (target: string, options: HealthOptions = {}): P }); } - if (isRunning) { + if (isRunning && (!options.signal || !options.signal.aborted)) { timer = setTimeout(poll, intervalSeconds * 1000); + } else { + cleanup(); } }; @@ -183,6 +205,23 @@ export const registerHealthCommand = (program: Command): void => { .option('-w, --watch', 'Watch mode: continuously refresh health output') .option('-i, --interval ', 'Refresh interval in seconds', '5') .action(async (target, options) => { - await showHealth(target, options); + if (options.watch) { + const controller = new AbortController(); + const sigintHandler = () => { + controller.abort(); + process.exit(0); + }; + process.once('SIGINT', sigintHandler); + process.once('SIGTERM', sigintHandler); + + try { + await showHealth(target, { ...options, signal: controller.signal }); + } finally { + process.off('SIGINT', sigintHandler); + process.off('SIGTERM', sigintHandler); + } + } else { + await showHealth(target, options); + } }); }; \ No newline at end of file