diff --git a/src/api.ts b/src/api.ts index 3b2f179..a0f92d9 100644 --- a/src/api.ts +++ b/src/api.ts @@ -6,11 +6,13 @@ import _ from 'lodash'; import getPort from 'get-port'; import fs from 'fs-extra'; import path from 'path'; +import fetch from 'node-fetch'; import { ContainerInfo } from 'dockerode'; import { config, envContainerConfig } from './config'; import { l } from './logger'; import { pendingHashes } from './builder'; +import { pollUntilHealthy } from './health'; import { ensureRepoCloned, fetchRemoteBranches, @@ -19,13 +21,15 @@ import { } from './git'; import { CONTAINER_EXPIRY_TIME } from './constants'; -import { timing } from './stats'; +import { increment, timing } from './stats'; import { ContainerError, ImageError } from './error'; type APIState = { accesses: Map< ContainerName, number >; branchHashes: Map< CommitHash, BranchName >; containers: Map< string, Docker.ContainerInfo >; + healthyContainers: Set< string >; + probingContainers: Set< string >; localImages: Map< ImageName, Docker.ImageInfo >; pullingImages: Map< ImageName, Promise< DockerodeStream > >; remoteBranches: Map< BranchName, CommitHash >; @@ -36,6 +40,8 @@ export const state: APIState = { accesses: new Map(), branchHashes: new Map(), containers: new Map(), + healthyContainers: new Set(), + probingContainers: new Set(), localImages: new Map(), pullingImages: new Map(), remoteBranches: new Map(), @@ -303,6 +309,80 @@ export async function deleteImage( hash: CommitHash ) { l.error( { err, commitHash: hash }, 'failed to remove image' ); } } +function firePollLoop( + containerId: string, + commitHash: CommitHash, + port: number +): Promise< void > { + state.probingContainers.add( containerId ); + increment( 'health.probe.started' ); + + const cleanup = () => { + state.probingContainers.delete( containerId ); + }; + + return pollUntilHealthy( { + port, + fetchImpl: fetch as any, + healthPath: config.build.healthPath, + intervalMs: config.build.healthProbeIntervalMs, + ceilingMs: config.build.healthProbeCeilingMs, + probeTimeoutMs: 1000, + shouldAbort: () => ! state.containers.has( containerId ), + } ) + .then( + outcome => { + cleanup(); + if ( outcome === 'healthy' ) { + increment( 'health.probe.healthy' ); + l.info( { containerId, commitHash, freePort: port }, 'Container reported healthy' ); + markContainerHealthy( containerId ); + } else if ( outcome === 'ceiling-exceeded' ) { + increment( 'health.probe.fail_open' ); + l.warn( + { containerId, commitHash, freePort: port }, + 'Container /health did not return 200 before ceiling; failing open' + ); + markContainerHealthy( containerId ); + } else { + increment( 'health.probe.aborted' ); + l.info( + { containerId, commitHash, freePort: port }, + 'Health probe aborted (container disappeared)' + ); + } + }, + err => { + cleanup(); + increment( 'health.probe.error' ); + l.error( + { err, containerId, commitHash, freePort: port }, + 'Unexpected error in health probe loop' + ); + } + ) + // Belt-and-suspenders: the .then handlers above catch both branches, so + // this chain is always fulfilled today. The terminal .catch guards against + // a future edit that reintroduces a throw inside a handler — otherwise + // every fire-and-forget caller would produce an unhandled rejection. + .catch( () => {} ); +} + +export function ensureHealthProbeFor( container: ContainerInfo ): Promise< void > { + const containerId = container.Id; + + if ( state.healthyContainers.has( containerId ) ) return Promise.resolve(); + if ( state.probingContainers.has( containerId ) ) return Promise.resolve(); + + const commitHash = extractCommitFromImage( container.Image ); + if ( ! commitHash ) return Promise.resolve(); + + const port = container.Ports && container.Ports[ 0 ] && container.Ports[ 0 ].PublicPort; + if ( ! port ) return Promise.resolve(); + + return firePollLoop( containerId, commitHash, port ); +} + export async function startContainer( commitHash: CommitHash, env: RunEnv ) { //l.info( { commitHash }, `Request to start a container for ${ commitHash }` ); const image = getImageName( commitHash ); @@ -386,7 +466,13 @@ export async function startContainer( commitHash: CommitHash, env: RunEnv ) { { image, freePort, commitHash }, `Successfully started container for ${ image } on ${ freePort }` ); - return refreshContainers().then( () => getRunningContainerForHash( commitHash ) ); + return refreshContainers().then( () => { + const container = getRunningContainerForHash( commitHash, env ); + if ( container ) { + ensureHealthProbeFor( container ); + } + return container; + } ); }, ( { error, freePort } ) => { l.error( @@ -419,6 +505,8 @@ export async function refreshContainers() { state.containers = new Map( containers.map( container => [ container.Id, container ] as [ string, ContainerInfo ] ) ); + reconcileHealthyContainers(); + ensureHealthProbesForRunningContainers(); } export function getRunningContainerForHash( hash: CommitHash, env?: RunEnv ): ContainerInfo | null { @@ -435,6 +523,62 @@ export function isContainerRunning( hash: CommitHash, env?: RunEnv ): boolean { return !! getRunningContainerForHash( hash, env ); } +export function markContainerHealthy( containerId: string ): void { + state.healthyContainers.add( containerId ); +} + +export function forgetContainerHealth( containerId: string ): void { + state.healthyContainers.delete( containerId ); +} + +export function isContainerHealthy( hash: CommitHash, env?: RunEnv ): boolean { + const container = getRunningContainerForHash( hash, env ); + if ( ! container ) { + return false; + } + return state.healthyContainers.has( container.Id ); +} + +// Policy layer on top of isContainerHealthy: honors the healthGateEnabled kill +// switch. When the gate is disabled, any running container is considered ready +// to serve (pre-QAO-358 behavior). +export function isContainerReadyToServe( hash: CommitHash, env?: RunEnv ): boolean { + if ( ! config.build.healthGateEnabled ) { + return isContainerRunning( hash, env ); + } + return isContainerHealthy( hash, env ); +} + +export function reconcileHealthyContainers(): void { + const aliveIds = new Set< string >(); + for ( const container of state.containers.values() ) { + aliveIds.add( container.Id ); + } + for ( const id of Array.from( state.healthyContainers ) ) { + if ( ! aliveIds.has( id ) ) { + state.healthyContainers.delete( id ); + } + } + for ( const id of Array.from( state.probingContainers ) ) { + if ( ! aliveIds.has( id ) ) { + state.probingContainers.delete( id ); + } + } +} + +export function ensureHealthProbesForRunningContainers(): Promise< void > { + const promises: Promise< void >[] = []; + for ( const container of state.containers.values() ) { + if ( container.State !== 'running' ) { + continue; + } + promises.push( ensureHealthProbeFor( container ) ); + } + // allSettled (not all) so a future change that breaks the "probe promise + // never rejects" invariant doesn't orphan sibling probes via fail-fast. + return Promise.allSettled( promises ).then( (): void => undefined ); +} + export function getPortForContainer( hash: CommitHash, env: RunEnv ): number | boolean { const container = getRunningContainerForHash( hash, env ); diff --git a/src/config.ts b/src/config.ts index ce58a0e..72f042c 100644 --- a/src/config.ts +++ b/src/config.ts @@ -16,6 +16,10 @@ type BuildConfig = Readonly< { exposedPort: number; logFilename: string; tagPrefix: string; + healthPath: string; + healthProbeIntervalMs: number; + healthProbeCeilingMs: number; + healthGateEnabled: boolean; } >; type RepoConfig = Readonly< { @@ -34,6 +38,13 @@ export const config: AppConfig = { exposedPort: 3000, logFilename: 'dserve-build-log.txt', tagPrefix: 'dserve-wpcalypso', + healthPath: '/health', + healthProbeIntervalMs: 500, + healthProbeCeilingMs: 30000, + // Kill switch for the container-readiness gate. Set the env var + // DSERVE_HEALTH_GATE_ENABLED=false to fall back to pre-QAO-358 behavior + // (proxy to any running container). Requires a dserve restart to take effect. + healthGateEnabled: process.env.DSERVE_HEALTH_GATE_ENABLED !== 'false', }, repo: { diff --git a/src/health.ts b/src/health.ts new file mode 100644 index 0000000..8404f7f --- /dev/null +++ b/src/health.ts @@ -0,0 +1,68 @@ +export type HealthFetchImpl = ( + url: string, + init?: { timeout?: number } +) => Promise< { status: number } >; + +export type ProbeOptions = { + fetchImpl: HealthFetchImpl; + healthPath: string; + timeoutMs: number; +}; + +export async function probeContainerHealth( + port: number, + options: ProbeOptions +): Promise< boolean > { + const url = `http://127.0.0.1:${ port }${ options.healthPath }`; + try { + const response = await options.fetchImpl( url, { timeout: options.timeoutMs } ); + return response.status === 200; + } catch ( err ) { + return false; + } +} + +export type PollOptions = { + port: number; + fetchImpl: HealthFetchImpl; + healthPath: string; + intervalMs: number; + ceilingMs: number; + probeTimeoutMs: number; + shouldAbort?: () => boolean; + now?: () => number; +}; + +export type PollOutcome = 'healthy' | 'ceiling-exceeded' | 'aborted'; + +export async function pollUntilHealthy( options: PollOptions ): Promise< PollOutcome > { + const now = options.now || ( () => Date.now() ); + const startedAt = now(); + + const sleep = ( ms: number ) => + new Promise< void >( resolve => { + setTimeout( resolve, ms ); + } ); + + while ( true ) { + if ( options.shouldAbort && options.shouldAbort() ) { + return 'aborted'; + } + + const healthy = await probeContainerHealth( options.port, { + fetchImpl: options.fetchImpl, + healthPath: options.healthPath, + timeoutMs: options.probeTimeoutMs, + } ); + + if ( healthy ) { + return 'healthy'; + } + + if ( now() - startedAt >= options.ceilingMs ) { + return 'ceiling-exceeded'; + } + + await sleep( options.intervalMs ); + } +} diff --git a/src/index.ts b/src/index.ts index 0458d5d..a86ab47 100644 --- a/src/index.ts +++ b/src/index.ts @@ -15,11 +15,13 @@ import { getPortForContainer, startContainer, isContainerRunning, + isContainerReadyToServe, proxyRequestToHash as proxy, deleteImage, getLocalImages, getBranchHashes, } from './api'; +import { decideRouteAction } from './route-actions'; import { ONE_MINUTE, ONE_SECOND, TEN_MINUTES } from './constants'; @@ -167,8 +169,10 @@ calypsoServer.use( determineEnvironment ); calypsoServer.get( '/status', async ( req: express.Request, res: express.Response ) => { const commitHash = req.session.commitHash; let status; - if ( isContainerRunning( commitHash ) ) { + if ( isContainerReadyToServe( commitHash ) ) { status = 'Ready'; + } else if ( isContainerRunning( commitHash ) ) { + status = 'Starting'; } else if ( didBuildFail( commitHash ) ) { status = 'FAIL'; } else if ( await hasHashLocally( commitHash ) ) { @@ -186,49 +190,72 @@ calypsoServer.get( '*', async ( req: express.Request, res: express.Response ) => const { commitHash, runEnv } = req.session; const hasLocally = await hasHashLocally( commitHash ); const isCurrentlyBuilding = ! hasLocally && ( await isBuildInProgress( commitHash ) ); - const needsToBuild = ! isCurrentlyBuilding && ! hasLocally; - const shouldStartContainer = hasLocally && ! isContainerRunning( commitHash, runEnv ); - const shouldReset = req.query.reset; - - if ( shouldReset ) { - l.info( { commitHash }, `Hard reset for ${ commitHash }` ); - increment( 'hash_reset' ); - await deleteImage( commitHash ); - await cleanupBuildDir( commitHash ); - const response = `hard reset hash: ${ commitHash } and loading it now...`; - res.set( 'Refresh', `5;url=${ req.path }` ); - res.send( striptags( response ) ); - return; - } - - if ( isContainerRunning( commitHash, runEnv ) ) { - proxy( req, res ); - return; - } - - let buildLog; - let message; - if ( isCurrentlyBuilding ) { - buildLog = await readBuildLog( commitHash ); - } else if ( needsToBuild ) { - message = 'Starting build now'; - addToBuildQueue( commitHash ); - } else if ( shouldStartContainer ) { - //message = 'Just started your hash, this page will restart automatically'; - // TODO: fix race condition where multiple containers may be spun up - // within the same subsecond time period. - try { - await startContainer( commitHash, runEnv ); - res.set( 'Refresh', `1;url=${ req.path }` ); - res.send( striptags( 'build complete, loading now...' ) ); + const isRunning = isContainerRunning( commitHash, runEnv ); + const isHealthy = isContainerReadyToServe( commitHash, runEnv ); + const shouldReset = Boolean( req.query.reset ); + const acceptsHtml = ( req.header( 'accept' ) || '' ).includes( 'text/html' ); + + const action = decideRouteAction( { + commitHash, + runEnv: runEnv || '', + hasLocally, + isCurrentlyBuilding, + isRunning, + isHealthy, + didFail: didBuildFail( commitHash ), + shouldReset, + acceptsHtml, + } ); + + switch ( action.kind ) { + case 'reset': { + l.info( { commitHash }, `Hard reset for ${ commitHash }` ); + increment( 'hash_reset' ); + await deleteImage( commitHash ); + await cleanupBuildDir( commitHash ); + res.set( 'Refresh', `5;url=${ req.path }` ); + res.send( striptags( `hard reset hash: ${ commitHash } and loading it now...` ) ); + return; + } + case 'proxy': { + proxy( req, res ); + return; + } + case 'loading': { + res.set( 'Refresh', `2;url=${ req.path }` ); + renderApp( { message: action.message, startedServerAt } ).pipe( res ); + return; + } + case 'not-ready': { + res.set( 'Retry-After', '2' ); + res.status( 503 ).send( 'Container not ready' ); + return; + } + case 'show-build-log': { + const buildLog = await readBuildLog( commitHash ); + renderApp( { buildLog, startedServerAt } ).pipe( res ); + return; + } + case 'enqueue-build': { + addToBuildQueue( commitHash ); + renderApp( { message: 'Starting build now', startedServerAt } ).pipe( res ); + return; + } + case 'start-container': { + try { + await startContainer( commitHash, runEnv ); + res.set( 'Refresh', `1;url=${ req.path }` ); + res.send( striptags( 'build complete, loading now...' ) ); + } catch ( err ) { + l.error( { err }, 'Error starting commit' ); + renderApp( { + message: 'Error starting that commit...', + startedServerAt, + } ).pipe( res ); + } return; - } catch ( err ) { - message = 'Error starting that commit...'; - l.error( { err }, 'Error starting commit' ); } } - - renderApp( { message, buildLog, startedServerAt } ).pipe( res ); } ); // log errors diff --git a/src/route-actions.ts b/src/route-actions.ts new file mode 100644 index 0000000..1011419 --- /dev/null +++ b/src/route-actions.ts @@ -0,0 +1,49 @@ +export type RouteFacts = { + commitHash: string; + runEnv: string; + hasLocally: boolean; + isCurrentlyBuilding: boolean; + isRunning: boolean; + isHealthy: boolean; + didFail: boolean; + shouldReset: boolean; + acceptsHtml: boolean; +}; + +export type RouteAction = + | { kind: 'reset' } + | { kind: 'proxy' } + | { kind: 'loading'; message: string } + | { kind: 'not-ready' } + | { kind: 'show-build-log' } + | { kind: 'enqueue-build' } + | { kind: 'start-container' }; + +export function decideRouteAction( facts: RouteFacts ): RouteAction { + if ( facts.shouldReset ) { + return { kind: 'reset' }; + } + + if ( facts.isRunning ) { + if ( facts.isHealthy ) { + return { kind: 'proxy' }; + } + if ( facts.acceptsHtml ) { + return { + kind: 'loading', + message: 'Starting container, this page will refresh shortly', + }; + } + return { kind: 'not-ready' }; + } + + if ( facts.isCurrentlyBuilding ) { + return { kind: 'show-build-log' }; + } + + if ( ! facts.hasLocally ) { + return { kind: 'enqueue-build' }; + } + + return { kind: 'start-container' }; +} diff --git a/test/api.test.ts b/test/api.test.ts index 3775404..31a60d2 100644 --- a/test/api.test.ts +++ b/test/api.test.ts @@ -7,6 +7,25 @@ jest.mock( 'hot-shots', () => ( { } ) ), } ) ); +jest.mock( '../src/health', () => ( { + pollUntilHealthy: jest.fn().mockResolvedValue( 'healthy' ), + probeContainerHealth: jest.fn(), +} ) ); + +jest.mock( '../src/logger', () => ( { + l: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + child: jest.fn().mockReturnValue( { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + } ), + }, + ringbuffer: { records: [] as any[] }, +} ) ); + import { CONTAINER_EXPIRY_TIME } from '../src/constants'; const { @@ -84,4 +103,109 @@ describe( 'api', () => { expect( getExpiredContainers() ).toEqual( [ state.containers.get( getImageName( '2' ) ) ] ); } ); } ); + + describe( 'refreshContainers healthyContainers cleanup', () => { + test( 'drops healthy ids that are no longer present in state.containers', () => { + const { state, reconcileHealthyContainers } = require( '../src/api' ); + state.containers = new Map( [ + [ 'alive', { Id: 'alive', Image: 'dserve-wpcalypso:a' } ], + ] ); + state.healthyContainers = new Set( [ 'alive', 'ghost' ] ); + + reconcileHealthyContainers(); + + expect( Array.from( state.healthyContainers ) ).toEqual( [ 'alive' ] ); + } ); + + test( 'is a no-op when every healthy id is still present', () => { + const { state, reconcileHealthyContainers } = require( '../src/api' ); + state.containers = new Map( [ + [ 'a', { Id: 'a' } ], + [ 'b', { Id: 'b' } ], + ] ); + state.healthyContainers = new Set( [ 'a', 'b' ] ); + + reconcileHealthyContainers(); + + expect( Array.from( state.healthyContainers ).sort() ).toEqual( [ 'a', 'b' ] ); + } ); + + test( 'drops probing ids that are no longer present in state.containers', () => { + const { state, reconcileHealthyContainers } = require( '../src/api' ); + state.containers = new Map( [ + [ 'alive', { Id: 'alive', Image: 'dserve-wpcalypso:a' } ], + ] ); + state.healthyContainers = new Set(); + state.probingContainers = new Set( [ 'alive', 'ghost' ] ); + + reconcileHealthyContainers(); + + expect( Array.from( state.probingContainers ) ).toEqual( [ 'alive' ] ); + } ); + } ); + + describe( 'ensureHealthProbesForRunningContainers', () => { + test( 'starts a probe for each running dserve container that is not yet healthy', () => { + const { + ensureHealthProbesForRunningContainers, + state, + } = require( '../src/api' ); + + state.containers = new Map( [ + [ + 'cid-1', + { + Id: 'cid-1', + Image: 'dserve-wpcalypso:aaa', + State: 'running', + Ports: [ { PublicPort: 11111 } ], + Labels: { calypsoEnvironment: 'calypso' }, + }, + ], + [ + 'cid-2', + { + Id: 'cid-2', + Image: 'dserve-wpcalypso:bbb', + State: 'running', + Ports: [ { PublicPort: 22222 } ], + Labels: { calypsoEnvironment: 'jetpack' }, + }, + ], + ] ); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + ensureHealthProbesForRunningContainers(); + + expect( state.probingContainers.has( 'cid-1' ) ).toBe( true ); + expect( state.probingContainers.has( 'cid-2' ) ).toBe( true ); + } ); + + test( 'skips containers that are not in running state', () => { + const { + ensureHealthProbesForRunningContainers, + state, + } = require( '../src/api' ); + + state.containers = new Map( [ + [ + 'cid-1', + { + Id: 'cid-1', + Image: 'dserve-wpcalypso:aaa', + State: 'exited', + Ports: [ { PublicPort: 11111 } ], + Labels: { calypsoEnvironment: 'calypso' }, + }, + ], + ] ); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + ensureHealthProbesForRunningContainers(); + + expect( state.probingContainers.size ).toBe( 0 ); + } ); + } ); } ); diff --git a/test/ensure-health-probe.test.ts b/test/ensure-health-probe.test.ts new file mode 100644 index 0000000..b0b6983 --- /dev/null +++ b/test/ensure-health-probe.test.ts @@ -0,0 +1,204 @@ +jest.mock( 'hot-shots', () => ( { + StatsD: jest.fn( () => ( { + increment: jest.fn(), + decrement: jest.fn(), + gauge: jest.fn(), + timing: jest.fn(), + } ) ), +} ) ); + +describe( 'ensureHealthProbeFor', () => { + const pollUntilHealthy = jest.fn(); + const increment = jest.fn(); + + beforeEach( () => { + jest.resetModules(); + jest.clearAllMocks(); + + jest.doMock( '../src/health', () => ( { + pollUntilHealthy: pollUntilHealthy.mockResolvedValue( 'healthy' ), + probeContainerHealth: jest.fn(), + } ) ); + jest.doMock( '../src/stats', () => ( { + increment, + decrement: jest.fn(), + gauge: jest.fn(), + timing: jest.fn(), + } ) ); + jest.doMock( '../src/builder', () => ( { + pendingHashes: new Set(), + } ) ); + jest.doMock( '../src/logger', () => ( { + l: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, + } ) ); + } ); + + const runningDserveContainer = ( id: string, hash: string, port: number ) => ( { + Id: id, + Image: `dserve-wpcalypso:${ hash }`, + State: 'running', + Ports: [ { PublicPort: port } ], + Labels: { calypsoEnvironment: 'calypso' }, + } ); + + test( 'starts a probe when the container is not healthy and not already being probed', () => { + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + const container = runningDserveContainer( 'cid-1', 'abcdef', 12345 ); + state.containers.set( container.Id, container ); + + ensureHealthProbeFor( container ); + + expect( pollUntilHealthy ).toHaveBeenCalledTimes( 1 ); + expect( pollUntilHealthy.mock.calls[ 0 ][ 0 ].port ).toBe( 12345 ); + expect( state.probingContainers.has( 'cid-1' ) ).toBe( true ); + expect( increment ).toHaveBeenCalledWith( 'health.probe.started' ); + } ); + + test( 'does not start a probe for a container already marked healthy', () => { + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set( [ 'cid-1' ] ); + state.probingContainers = new Set(); + + const container = runningDserveContainer( 'cid-1', 'abcdef', 12345 ); + state.containers.set( container.Id, container ); + + ensureHealthProbeFor( container ); + + expect( pollUntilHealthy ).not.toHaveBeenCalled(); + } ); + + test( 'does not start a duplicate probe when one is already in flight', () => { + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set( [ 'cid-1' ] ); + + const container = runningDserveContainer( 'cid-1', 'abcdef', 12345 ); + state.containers.set( container.Id, container ); + + ensureHealthProbeFor( container ); + + expect( pollUntilHealthy ).not.toHaveBeenCalled(); + } ); + + test( 'skips containers whose image is not a dserve image', () => { + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + const container = { + Id: 'cid-1', + Image: 'nginx:latest', + State: 'running', + Ports: [ { PublicPort: 80 } ], + Labels: {}, + }; + state.containers.set( container.Id, container ); + + ensureHealthProbeFor( container ); + + expect( pollUntilHealthy ).not.toHaveBeenCalled(); + } ); + + test( 'skips containers that have no published port yet', () => { + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + const container = { + Id: 'cid-1', + Image: 'dserve-wpcalypso:abcdef', + State: 'running', + Ports: [] as Array< { PublicPort: number } >, + Labels: { calypsoEnvironment: 'calypso' }, + }; + state.containers.set( container.Id, container ); + + ensureHealthProbeFor( container ); + + expect( pollUntilHealthy ).not.toHaveBeenCalled(); + expect( state.probingContainers.has( 'cid-1' ) ).toBe( false ); + } ); + + test( 'removes the id from probingContainers once the probe resolves and marks healthy', async () => { + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + const container = runningDserveContainer( 'cid-1', 'abcdef', 12345 ); + state.containers.set( container.Id, container ); + + await ensureHealthProbeFor( container ); + + expect( state.probingContainers.has( 'cid-1' ) ).toBe( false ); + expect( state.healthyContainers.has( 'cid-1' ) ).toBe( true ); + expect( increment ).toHaveBeenCalledWith( 'health.probe.started' ); + expect( increment ).toHaveBeenCalledWith( 'health.probe.healthy' ); + } ); + + test( 'emits health.probe.fail_open and marks healthy on ceiling-exceeded', async () => { + pollUntilHealthy.mockResolvedValueOnce( 'ceiling-exceeded' ); + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + const container = runningDserveContainer( 'cid-1', 'abcdef', 12345 ); + state.containers.set( container.Id, container ); + + await ensureHealthProbeFor( container ); + + expect( state.probingContainers.has( 'cid-1' ) ).toBe( false ); + expect( state.healthyContainers.has( 'cid-1' ) ).toBe( true ); + expect( increment ).toHaveBeenCalledWith( 'health.probe.fail_open' ); + } ); + + test( 'emits health.probe.aborted when the probe loop is aborted', async () => { + pollUntilHealthy.mockResolvedValueOnce( 'aborted' ); + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + const container = runningDserveContainer( 'cid-1', 'abcdef', 12345 ); + state.containers.set( container.Id, container ); + + await ensureHealthProbeFor( container ); + + expect( state.probingContainers.has( 'cid-1' ) ).toBe( false ); + // Aborted path explicitly does NOT mark healthy. + expect( state.healthyContainers.has( 'cid-1' ) ).toBe( false ); + expect( increment ).toHaveBeenCalledWith( 'health.probe.aborted' ); + } ); + + test( 'emits health.probe.error when pollUntilHealthy rejects unexpectedly', async () => { + pollUntilHealthy.mockRejectedValueOnce( new Error( 'boom' ) ); + const { ensureHealthProbeFor, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + const container = runningDserveContainer( 'cid-1', 'abcdef', 12345 ); + state.containers.set( container.Id, container ); + + await ensureHealthProbeFor( container ); + + // Cleanup still runs on the error branch. + expect( state.probingContainers.has( 'cid-1' ) ).toBe( false ); + // Error path explicitly does NOT mark healthy. + expect( state.healthyContainers.has( 'cid-1' ) ).toBe( false ); + expect( increment ).toHaveBeenCalledWith( 'health.probe.error' ); + } ); +} ); diff --git a/test/health.test.ts b/test/health.test.ts new file mode 100644 index 0000000..c139137 --- /dev/null +++ b/test/health.test.ts @@ -0,0 +1,146 @@ +describe( 'probeContainerHealth', () => { + beforeEach( () => { + jest.resetModules(); + } ); + + test( 'returns true when fetch resolves with 200', async () => { + const fetchImpl = jest.fn().mockResolvedValue( { status: 200 } ); + const { probeContainerHealth } = require( '../src/health' ); + + const result = await probeContainerHealth( 12345, { + fetchImpl, + healthPath: '/health', + timeoutMs: 1000, + } ); + + expect( result ).toBe( true ); + expect( fetchImpl ).toHaveBeenCalledTimes( 1 ); + const [ url ] = fetchImpl.mock.calls[ 0 ]; + expect( url ).toBe( 'http://127.0.0.1:12345/health' ); + } ); + + test( 'returns false when fetch resolves with non-200', async () => { + const fetchImpl = jest.fn().mockResolvedValue( { status: 503 } ); + const { probeContainerHealth } = require( '../src/health' ); + + const result = await probeContainerHealth( 12345, { + fetchImpl, + healthPath: '/health', + timeoutMs: 1000, + } ); + + expect( result ).toBe( false ); + } ); + + test( 'returns false when fetch rejects (connection refused)', async () => { + const fetchImpl = jest.fn().mockRejectedValue( new Error( 'ECONNREFUSED' ) ); + const { probeContainerHealth } = require( '../src/health' ); + + const result = await probeContainerHealth( 12345, { + fetchImpl, + healthPath: '/health', + timeoutMs: 1000, + } ); + + expect( result ).toBe( false ); + } ); +} ); + +describe( 'pollUntilHealthy', () => { + beforeEach( () => { + jest.resetModules(); + jest.useFakeTimers(); + } ); + + afterEach( () => { + jest.useRealTimers(); + } ); + + test( 'resolves with "healthy" on first successful probe', async () => { + const fetchImpl = jest.fn().mockResolvedValue( { status: 200 } ); + const { pollUntilHealthy } = require( '../src/health' ); + + const outcome = await pollUntilHealthy( { + port: 12345, + fetchImpl, + healthPath: '/health', + intervalMs: 500, + ceilingMs: 30000, + probeTimeoutMs: 1000, + } ); + + expect( outcome ).toBe( 'healthy' ); + expect( fetchImpl ).toHaveBeenCalledTimes( 1 ); + } ); + + test( 'retries until success', async () => { + const fetchImpl = jest + .fn() + .mockResolvedValueOnce( { status: 503 } ) + .mockResolvedValueOnce( { status: 503 } ) + .mockResolvedValueOnce( { status: 200 } ); + const { pollUntilHealthy } = require( '../src/health' ); + + const promise = pollUntilHealthy( { + port: 12345, + fetchImpl, + healthPath: '/health', + intervalMs: 500, + ceilingMs: 30000, + probeTimeoutMs: 1000, + } ); + + // Drain all pending microtasks + timers until the loop completes. + await jest.runAllTimersAsync(); + const outcome = await promise; + + expect( outcome ).toBe( 'healthy' ); + expect( fetchImpl ).toHaveBeenCalledTimes( 3 ); + } ); + + test( 'resolves with "ceiling-exceeded" when ceilingMs elapses without success', async () => { + const fetchImpl = jest.fn().mockResolvedValue( { status: 503 } ); + const { pollUntilHealthy } = require( '../src/health' ); + + const promise = pollUntilHealthy( { + port: 12345, + fetchImpl, + healthPath: '/health', + intervalMs: 500, + ceilingMs: 2000, + probeTimeoutMs: 1000, + } ); + + await jest.runAllTimersAsync(); + const outcome = await promise; + + expect( outcome ).toBe( 'ceiling-exceeded' ); + // ceilingMs=2000 / intervalMs=500 => at least 4 probes, at most 5 including the final attempt + expect( fetchImpl.mock.calls.length ).toBeGreaterThanOrEqual( 4 ); + expect( fetchImpl.mock.calls.length ).toBeLessThanOrEqual( 5 ); + } ); + + test( 'stops polling when aborted', async () => { + const fetchImpl = jest.fn().mockResolvedValue( { status: 503 } ); + const { pollUntilHealthy } = require( '../src/health' ); + + const abort = { aborted: false }; + const promise = pollUntilHealthy( { + port: 12345, + fetchImpl, + healthPath: '/health', + intervalMs: 500, + ceilingMs: 30000, + probeTimeoutMs: 1000, + shouldAbort: () => abort.aborted, + } ); + + // Let one probe fire, then abort. + await jest.advanceTimersByTimeAsync( 500 ); + abort.aborted = true; + await jest.runAllTimersAsync(); + const outcome = await promise; + + expect( outcome ).toBe( 'aborted' ); + } ); +} ); diff --git a/test/is-container-healthy.test.ts b/test/is-container-healthy.test.ts new file mode 100644 index 0000000..56e8aaa --- /dev/null +++ b/test/is-container-healthy.test.ts @@ -0,0 +1,61 @@ +jest.mock( 'hot-shots', () => ( { + StatsD: jest.fn( () => ( { + increment: jest.fn(), + decrement: jest.fn(), + gauge: jest.fn(), + timing: jest.fn(), + } ) ), +} ) ); + +const { + getImageName, + isContainerHealthy, + markContainerHealthy, + forgetContainerHealth, + state, +} = require( '../src/api' ); + +describe( 'isContainerHealthy', () => { + beforeEach( () => { + state.containers = new Map(); + state.healthyContainers = new Set(); + } ); + + test( 'returns false when the container is not running', () => { + expect( isContainerHealthy( 'feedface', 'calypso' ) ).toBe( false ); + } ); + + test( 'returns false when the container is running but not marked healthy', () => { + const container = { + Id: 'container-id', + Image: getImageName( 'feedface' ), + State: 'running', + Ports: [ { PublicPort: 12345 } ], + Labels: { calypsoEnvironment: 'calypso' }, + }; + state.containers.set( container.Id, container ); + + expect( isContainerHealthy( 'feedface', 'calypso' ) ).toBe( false ); + } ); + + test( 'returns true once the container id has been marked healthy', () => { + const container = { + Id: 'container-id', + Image: getImageName( 'feedface' ), + State: 'running', + Ports: [ { PublicPort: 12345 } ], + Labels: { calypsoEnvironment: 'calypso' }, + }; + state.containers.set( container.Id, container ); + + markContainerHealthy( 'container-id' ); + + expect( isContainerHealthy( 'feedface', 'calypso' ) ).toBe( true ); + } ); + + test( 'forgetContainerHealth removes the id', () => { + markContainerHealthy( 'container-id' ); + forgetContainerHealth( 'container-id' ); + expect( state.healthyContainers.has( 'container-id' ) ).toBe( false ); + } ); +} ); diff --git a/test/is-container-ready-to-serve.test.ts b/test/is-container-ready-to-serve.test.ts new file mode 100644 index 0000000..7aa7de9 --- /dev/null +++ b/test/is-container-ready-to-serve.test.ts @@ -0,0 +1,128 @@ +jest.mock( 'hot-shots', () => ( { + StatsD: jest.fn( () => ( { + increment: jest.fn(), + decrement: jest.fn(), + gauge: jest.fn(), + timing: jest.fn(), + } ) ), +} ) ); + +jest.mock( '../src/health', () => ( { + pollUntilHealthy: jest.fn().mockResolvedValue( 'healthy' ), + probeContainerHealth: jest.fn(), +} ) ); + +describe( 'isContainerReadyToServe', () => { + const runningContainer = { + Id: 'container-id', + Image: 'dserve-wpcalypso:feedface', + State: 'running', + Ports: [ { PublicPort: 12345 } ], + Labels: { calypsoEnvironment: 'calypso' }, + }; + + beforeEach( () => { + jest.resetModules(); + + jest.doMock( '../src/logger', () => ( { + l: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, + } ) ); + } ); + + test( 'when the health gate is enabled, delegates to isContainerHealthy (false if not yet healthy)', () => { + jest.doMock( '../src/config', () => { + const actual = jest.requireActual( '../src/config' ); + return { + ...actual, + config: { + ...actual.config, + build: { + ...actual.config.build, + healthGateEnabled: true, + }, + }, + }; + } ); + + const { isContainerReadyToServe, state } = require( '../src/api' ); + state.containers = new Map( [ [ runningContainer.Id, runningContainer ] ] ); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + expect( isContainerReadyToServe( 'feedface', 'calypso' ) ).toBe( false ); + } ); + + test( 'when the health gate is enabled, returns true once the container is marked healthy', () => { + jest.doMock( '../src/config', () => { + const actual = jest.requireActual( '../src/config' ); + return { + ...actual, + config: { + ...actual.config, + build: { + ...actual.config.build, + healthGateEnabled: true, + }, + }, + }; + } ); + + const { isContainerReadyToServe, markContainerHealthy, state } = require( '../src/api' ); + state.containers = new Map( [ [ runningContainer.Id, runningContainer ] ] ); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + markContainerHealthy( runningContainer.Id ); + + expect( isContainerReadyToServe( 'feedface', 'calypso' ) ).toBe( true ); + } ); + + test( 'when the health gate is disabled, returns true for any running container regardless of healthy state', () => { + jest.doMock( '../src/config', () => { + const actual = jest.requireActual( '../src/config' ); + return { + ...actual, + config: { + ...actual.config, + build: { + ...actual.config.build, + healthGateEnabled: false, + }, + }, + }; + } ); + + const { isContainerReadyToServe, state } = require( '../src/api' ); + state.containers = new Map( [ [ runningContainer.Id, runningContainer ] ] ); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + expect( isContainerReadyToServe( 'feedface', 'calypso' ) ).toBe( true ); + } ); + + test( 'when the health gate is disabled, still returns false when the container is not running', () => { + jest.doMock( '../src/config', () => { + const actual = jest.requireActual( '../src/config' ); + return { + ...actual, + config: { + ...actual.config, + build: { + ...actual.config.build, + healthGateEnabled: false, + }, + }, + }; + } ); + + const { isContainerReadyToServe, state } = require( '../src/api' ); + state.containers = new Map(); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + expect( isContainerReadyToServe( 'feedface', 'calypso' ) ).toBe( false ); + } ); +} ); diff --git a/test/refresh-fire-and-forget.test.ts b/test/refresh-fire-and-forget.test.ts new file mode 100644 index 0000000..d938c4d --- /dev/null +++ b/test/refresh-fire-and-forget.test.ts @@ -0,0 +1,68 @@ +jest.mock( 'hot-shots', () => ( { + StatsD: jest.fn( () => ( { + increment: jest.fn(), + decrement: jest.fn(), + gauge: jest.fn(), + timing: jest.fn(), + } ) ), +} ) ); + +describe( 'refreshContainers fire-and-forget invariant', () => { + const listContainers = jest.fn(); + const pollUntilHealthy = jest.fn(); + + beforeEach( () => { + jest.resetModules(); + jest.clearAllMocks(); + + listContainers.mockResolvedValue( [ + { + Id: 'cid-fire-and-forget', + Image: 'dserve-wpcalypso:facef00d', + State: 'running', + Ports: [ { PublicPort: 30000 } ], + Labels: { calypsoEnvironment: 'calypso' }, + }, + ] ); + + jest.doMock( 'dockerode', () => + jest.fn().mockImplementation( () => ( { + listContainers, + } ) ) + ); + jest.doMock( '../src/builder', () => ( { + pendingHashes: new Set(), + } ) ); + jest.doMock( '../src/logger', () => ( { + l: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, + } ) ); + jest.doMock( '../src/health', () => ( { + pollUntilHealthy, + probeContainerHealth: jest.fn(), + } ) ); + } ); + + test( 'refreshContainers resolves without waiting for probes to complete', async () => { + // Probe promise that never resolves. If refreshContainers were to await + // the fire-and-forget probe chain, this test would hang until jest's + // default 5s timeout and fail. + pollUntilHealthy.mockReturnValueOnce( new Promise( () => {} ) ); + + const { refreshContainers, state } = require( '../src/api' ); + + const start = Date.now(); + await refreshContainers(); + const elapsed = Date.now() - start; + + expect( elapsed ).toBeLessThan( 500 ); + + // The probe did start — sanity check that the scan actually kicked it off + // rather than skipping the container on some predicate. + expect( state.probingContainers.has( 'cid-fire-and-forget' ) ).toBe( true ); + expect( pollUntilHealthy ).toHaveBeenCalledTimes( 1 ); + } ); +} ); diff --git a/test/restart-recovery.test.ts b/test/restart-recovery.test.ts new file mode 100644 index 0000000..230a011 --- /dev/null +++ b/test/restart-recovery.test.ts @@ -0,0 +1,76 @@ +jest.mock( 'hot-shots', () => ( { + StatsD: jest.fn( () => ( { + increment: jest.fn(), + decrement: jest.fn(), + gauge: jest.fn(), + timing: jest.fn(), + } ) ), +} ) ); + +describe( 'dserve restart recovery', () => { + let fetchImpl: jest.Mock; + + beforeEach( () => { + jest.resetModules(); + + fetchImpl = jest.fn().mockResolvedValue( { status: 200 } ); + + // node-fetch is called by ensureHealthProbeFor → pollUntilHealthy. Stub it so + // tests can control probe responses without real network calls. + jest.doMock( 'node-fetch', () => ( { + __esModule: true, + default: fetchImpl, + } ) ); + + jest.doMock( '../src/builder', () => ( { + pendingHashes: new Set(), + } ) ); + + jest.doMock( '../src/logger', () => ( { + l: { + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, + } ) ); + } ); + + test( 'a running container discovered via refresh (not startContainer) eventually becomes healthy', async () => { + const { + ensureHealthProbesForRunningContainers, + state, + } = require( '../src/api' ); + + // Simulate the state dserve would be in right after a restart: containers + // known from the Docker daemon (via refreshContainers) but healthyContainers + // empty because the in-memory set was lost on restart. + state.containers = new Map( [ + [ + 'post-restart-id', + { + Id: 'post-restart-id', + Image: 'dserve-wpcalypso:abc123', + State: 'running', + Ports: [ { PublicPort: 54321 } ], + Labels: { calypsoEnvironment: 'calypso' }, + }, + ], + ] ); + state.healthyContainers = new Set(); + state.probingContainers = new Set(); + + // In production this is called fire-and-forget from refreshContainers, but + // the function returns a Promise explicitly so tests can await the + // full chain deterministically without microtask bookkeeping. + await ensureHealthProbesForRunningContainers(); + + expect( state.healthyContainers.has( 'post-restart-id' ) ).toBe( true ); + expect( state.probingContainers.has( 'post-restart-id' ) ).toBe( false ); + + // Proves the real pollUntilHealthy → probeContainerHealth → fetch path ran, + // not a mock shortcut. + expect( fetchImpl ).toHaveBeenCalled(); + const url = fetchImpl.mock.calls[ 0 ][ 0 ]; + expect( url ).toBe( 'http://127.0.0.1:54321/health' ); + } ); +} ); diff --git a/test/routes-health-gating.test.ts b/test/routes-health-gating.test.ts new file mode 100644 index 0000000..a41f268 --- /dev/null +++ b/test/routes-health-gating.test.ts @@ -0,0 +1,71 @@ +import { decideRouteAction } from '../src/route-actions'; + +describe( 'decideRouteAction', () => { + const baseFacts = { + commitHash: 'feedface', + runEnv: 'calypso', + hasLocally: true, + isCurrentlyBuilding: false, + isRunning: true, + isHealthy: true, + didFail: false, + shouldReset: false, + acceptsHtml: true, + }; + + test( 'proxies when container is running and healthy', () => { + expect( decideRouteAction( baseFacts ) ).toEqual( { kind: 'proxy' } ); + } ); + + test( 'shows loading page when running but not healthy and client wants HTML', () => { + expect( + decideRouteAction( { ...baseFacts, isHealthy: false, acceptsHtml: true } ) + ).toEqual( { kind: 'loading', message: 'Starting container, this page will refresh shortly' } ); + } ); + + test( 'returns 503 when running but not healthy and client does not want HTML', () => { + expect( + decideRouteAction( { ...baseFacts, isHealthy: false, acceptsHtml: false } ) + ).toEqual( { kind: 'not-ready' } ); + } ); + + test( 'reports "build in progress" when building', () => { + expect( + decideRouteAction( { + ...baseFacts, + hasLocally: false, + isCurrentlyBuilding: true, + isRunning: false, + isHealthy: false, + } ) + ).toEqual( { kind: 'show-build-log' } ); + } ); + + test( 'enqueues build when nothing is built and nothing is in progress', () => { + expect( + decideRouteAction( { + ...baseFacts, + hasLocally: false, + isCurrentlyBuilding: false, + isRunning: false, + isHealthy: false, + } ) + ).toEqual( { kind: 'enqueue-build' } ); + } ); + + test( 'starts the container when image exists but no container is running', () => { + expect( + decideRouteAction( { + ...baseFacts, + isRunning: false, + isHealthy: false, + } ) + ).toEqual( { kind: 'start-container' } ); + } ); + + test( 'honors reset intent regardless of other state', () => { + expect( + decideRouteAction( { ...baseFacts, shouldReset: true } ) + ).toEqual( { kind: 'reset' } ); + } ); +} ); diff --git a/test/start-container.test.ts b/test/start-container.test.ts index ad5d561..001bdbb 100644 --- a/test/start-container.test.ts +++ b/test/start-container.test.ts @@ -1,6 +1,7 @@ describe( 'startContainer', () => { const listContainers = jest.fn(); const run = jest.fn(); + const pollUntilHealthy = jest.fn(); beforeEach( () => { jest.resetModules(); @@ -36,8 +37,15 @@ describe( 'startContainer', () => { }, } ) ); jest.doMock( '../src/stats', () => ( { + increment: jest.fn(), + decrement: jest.fn(), + gauge: jest.fn(), timing: jest.fn(), } ) ); + jest.doMock( '../src/health', () => ( { + pollUntilHealthy: pollUntilHealthy.mockResolvedValue( 'healthy' ), + probeContainerHealth: jest.fn(), + } ) ); } ); test( 'publishes the exposed port when starting an existing image', async () => { @@ -72,4 +80,69 @@ describe( 'startContainer', () => { global.setTimeout = realSetTimeout; } } ); + + test( 'starts health polling for the freshly started container', async () => { + const realSetTimeout = global.setTimeout; + try { + ( global as any ).setTimeout = ( fn: Function ) => { + fn(); + return 0; + }; + + const { startContainer } = require( '../src/api' ); + + await startContainer( 'feedface', 'calypso' ); + + expect( pollUntilHealthy ).toHaveBeenCalledTimes( 1 ); + const call = pollUntilHealthy.mock.calls[ 0 ][ 0 ]; + expect( call.port ).toBe( 12345 ); + expect( call.healthPath ).toBe( '/health' ); + expect( call.intervalMs ).toBe( 500 ); + expect( call.ceilingMs ).toBe( 30000 ); + } finally { + global.setTimeout = realSetTimeout; + } + } ); + + test( 'marks the container healthy when pollUntilHealthy resolves to healthy', async () => { + const realSetTimeout = global.setTimeout; + try { + ( global as any ).setTimeout = ( fn: Function ) => { + fn(); + return 0; + }; + + pollUntilHealthy.mockResolvedValueOnce( 'healthy' ); + const { startContainer, state } = require( '../src/api' ); + + await startContainer( 'feedface', 'calypso' ); + + // Let the fire-and-forget promise settle. + await new Promise( resolve => setImmediate( resolve ) ); + + expect( state.healthyContainers.has( 'container-id' ) ).toBe( true ); + } finally { + global.setTimeout = realSetTimeout; + } + } ); + + test( 'fails open and marks healthy on ceiling-exceeded', async () => { + const realSetTimeout = global.setTimeout; + try { + ( global as any ).setTimeout = ( fn: Function ) => { + fn(); + return 0; + }; + + pollUntilHealthy.mockResolvedValueOnce( 'ceiling-exceeded' ); + const { startContainer, state } = require( '../src/api' ); + + await startContainer( 'feedface', 'calypso' ); + await new Promise( resolve => setImmediate( resolve ) ); + + expect( state.healthyContainers.has( 'container-id' ) ).toBe( true ); + } finally { + global.setTimeout = realSetTimeout; + } + } ); } );