From 46af13bfcb1a11cd740f93625d4196b8953c8bbb Mon Sep 17 00:00:00 2001 From: Salih Yilboga Date: Thu, 10 Sep 2026 17:05:11 +0300 Subject: [PATCH] fix: resolve AWS deploy crash-loop and CDK/deploy issues found via real deployment Root-caused against a real cdk deploy, not from reading code alone: - backend-service-stack.ts: ECS healthCheckGracePeriodSeconds silently defaulted to 60s once an ALB target was attached, but the backend takes ~85s to finish booting (JVM + ADOT javaagent + Spring context). ECS killed every task mid-boot before it could ever answer, crash-looping a perfectly healthy application forever. Raised to 240s and made the ALB target-group interval/thresholds explicit instead of relying on defaults. - backend-service-stack.ts: MaxMetaspaceSize=192m was sized before the ADOT javaagent was added; the extra instrumentation classes exhausted it under real traffic (OutOfMemoryError: Metaspace), which took down the Redis/ES health indicators and made /actuator/health report DOWN. Raised to 384m. - backend-service-stack.ts: the container health check used CMD-SHELL, which ECS runs through /bin/sh -- dash in eclipse-temurin:25-jre, which does not implement /dev/tcp. The probe could never pass no matter how healthy the app was. Verified directly against the real image (old form fails even against a healthy server; fixed form passes/fails correctly). Switched to CMD + explicit bash. - UserServiceImpl.java: a new user's first page load fires several authenticated requests in parallel (feed + /users/me at minimum); all find no row and try to provision, and the losers trip users_cognito_sub_key. provision() unconditionally blamed a username collision instead, 500ing a legitimate sign-in. Now re-reads and returns the winner's row. Reproduced live against the deployed stack, fixed, and covered by a new test. - database-stack.ts: t4g -> t3 instance class (Graviton RDS capacity was tighter than Intel in eu-central-1) plus a configurable multiAz flag. - search-stack.ts: ECS health check retries capped at the real AWS limit (10, was 20) with startPeriod raised to preserve intent; Elasticsearch moved off PRIVATE_ISOLATED to PRIVATE_WITH_EGRESS so the Fargate task can reach ECR/CloudWatch Logs before the container starts. - compute-stack.ts, cache-stack.ts, event-streaming-stack.ts, rtmp-service-stack.ts, ws-sfu-stack.ts: security-group description strings fixed to the AWS-documented allowed charset (no em-dash, no bare ">"). - infra/bin/infra.ts: threads the new dbMultiAz context flag through. - frontend/vite.config.ts: configurable VITE_CACHE_DIR so the dev server does not fight write permissions on a shared cache path. - .claude/launch.json: local dev-server launch config for the frontend. Verified: full infra CDK suite green (26 suites / 89 tests), backend UserServiceImplTest 4/4 including the new race test, and a real cdk deploy of all 29 stacks reached CREATE_COMPLETE with the backend ALB target group reporting healthy for the first time. Co-Authored-By: Claude Sonnet 5 --- .claude/launch.json | 11 +++ .../services/impl/UserServiceImpl.java | 15 +++- .../services/impl/UserServiceImplTest.java | 19 +++++ frontend/vite.config.ts | 6 ++ infra/bin/infra.ts | 7 +- infra/lib/backend-service-stack.ts | 72 +++++++++++++++++-- infra/lib/cache-stack.ts | 4 +- infra/lib/compute-stack.ts | 2 +- infra/lib/database-stack.ts | 24 +++++-- infra/lib/event-streaming-stack.ts | 4 +- infra/lib/rtmp-service-stack.ts | 4 +- infra/lib/search-stack.ts | 32 +++++++-- infra/lib/ws-sfu-stack.ts | 4 +- infra/test/backend-service-stack.test.ts | 29 ++++++++ infra/test/database-stack.test.ts | 2 +- 15 files changed, 204 insertions(+), 31 deletions(-) create mode 100644 .claude/launch.json diff --git a/.claude/launch.json b/.claude/launch.json new file mode 100644 index 0000000..6fd2e70 --- /dev/null +++ b/.claude/launch.json @@ -0,0 +1,11 @@ +{ + "version": "0.0.1", + "configurations": [ + { + "name": "frontend", + "runtimeExecutable": "npm", + "runtimeArgs": ["run", "dev", "--prefix", "frontend", "--", "--host", "127.0.0.1", "--port", "5173"], + "port": 5173 + } + ] +} diff --git a/backend/src/main/java/com/escld/backend/services/impl/UserServiceImpl.java b/backend/src/main/java/com/escld/backend/services/impl/UserServiceImpl.java index 86ed337..9f91ed5 100644 --- a/backend/src/main/java/com/escld/backend/services/impl/UserServiceImpl.java +++ b/backend/src/main/java/com/escld/backend/services/impl/UserServiceImpl.java @@ -116,7 +116,20 @@ private User provision(UUID cognitoSub, Jwt jwt) { log.info("Provisioned new user profile for Cognito identity {}", cognitoSub); return saved; } catch (DataIntegrityViolationException e) { - throw new UsernameAlreadyTakenException(preferredUsername); + // Which unique constraint actually tripped changes the answer, so + // don't assume it was the username one. A brand-new user's very + // first page load fires several authenticated requests in + // parallel (the feed and /users/me at minimum); each finds no row + // yet and tries to provision, and the losers trip + // users_cognito_sub_key. That is not a username collision — it is + // the same person, provisioned microseconds earlier by whichever + // request won — so re-read and return that row rather than + // failing a legitimate sign-in. Observed for real against the + // deployed stack, where the losing request's /api/v1/feed 500'd. + // A genuine username collision finds nothing here and still + // surfaces as the 409 the frontend already knows how to render. + return userRepository.findByCognitoSub(cognitoSub) + .orElseThrow(() -> new UsernameAlreadyTakenException(preferredUsername)); } } diff --git a/backend/src/test/java/com/escld/backend/services/impl/UserServiceImplTest.java b/backend/src/test/java/com/escld/backend/services/impl/UserServiceImplTest.java index 175e0f4..0d05080 100644 --- a/backend/src/test/java/com/escld/backend/services/impl/UserServiceImplTest.java +++ b/backend/src/test/java/com/escld/backend/services/impl/UserServiceImplTest.java @@ -98,6 +98,25 @@ void surfacesAUsernameCollisionDuringProvisioningAsUsernameAlreadyTakenRatherTha .isInstanceOf(UsernameAlreadyTakenException.class); } + @Test + void returnsTheWinnersRowWhenAConcurrentRequestProvisionedTheSameIdentityFirst() { + // A brand-new user's first page load fires several authenticated + // requests at once (the feed and /users/me at minimum). All of them + // find no row, all try to provision, and the losers trip + // users_cognito_sub_key rather than the username index. Seen for real + // against the deployed stack, where the loser's /api/v1/feed 500'd. + User winner = User.builder().id(UUID.randomUUID()).cognitoSub(cognitoSub).username("racer").build(); + when(userRepository.findByCognitoSub(cognitoSub)) + .thenReturn(Optional.empty()) // our own initial lookup + .thenReturn(Optional.of(winner)); // re-read after the constraint fires + when(userRepository.save(any(User.class))) + .thenThrow(new DataIntegrityViolationException("duplicate key value violates unique constraint \"users_cognito_sub_key\"")); + when(cognitoUserAttributesClient.fetchAttributes("test-token")) + .thenReturn(attributes("racer@example.com", "racer", null)); + + assertThat(userService.getOrProvisionByCognitoSub(jwtFor(cognitoSub))).isSameAs(winner); + } + private Map attributes(String email, String preferredUsername, String picture) { Map attrs = new HashMap<>(); attrs.put("email", email); diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 79a65e0..a27ccdd 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -5,6 +5,12 @@ import { defineConfig } from "vite" // https://vite.dev/config/ export default defineConfig({ + // Overridable so a poisoned cache dir can't block the dev server: a root- + // owned node_modules/.vite (left behind when a container wrote into the + // bind-mounted frontend/) makes Vite's re-optimize step fail with EACCES on + // unlink, and it can't be cleared without root. Set VITE_CACHE_DIR to route + // around it. + cacheDir: process.env.VITE_CACHE_DIR || "node_modules/.vite", plugins: [react(), tailwindcss()], resolve: { alias: { diff --git a/infra/bin/infra.ts b/infra/bin/infra.ts index 27cc226..cb4f08a 100644 --- a/infra/bin/infra.ts +++ b/infra/bin/infra.ts @@ -54,11 +54,16 @@ const computeStack = new ComputeStack(app, 'EscldComputeStack', { vpc: networkStack.vpc, }); +// `-c dbMultiAz=false` drops to single-AZ for a throwaway test deploy (half the +// RDS cost, and only needs capacity in one AZ). Anything other than the literal +// string 'false' keeps the production Multi-AZ default. +const dbMultiAz = app.node.tryGetContext('dbMultiAz') !== 'false'; const databaseStack = new DatabaseStack(app, 'EscldDatabaseStack', { env, - description: 'RDS Postgres (Multi-AZ) for users/posts/comments', + description: `RDS Postgres (${dbMultiAz ? 'Multi-AZ' : 'single-AZ'}) for users/posts/comments`, vpc: networkStack.vpc, appServiceSecurityGroup: computeStack.appServiceSecurityGroup, + multiAz: dbMultiAz, }); // Pulled forward from Phase 4 — the backend can't boot without it (see diff --git a/infra/lib/backend-service-stack.ts b/infra/lib/backend-service-stack.ts index 39f4391..abce288 100644 --- a/infra/lib/backend-service-stack.ts +++ b/infra/lib/backend-service-stack.ts @@ -189,7 +189,18 @@ export class BackendServiceStack extends cdk.Stack { // the filesystem/volume side, not this string. See otel-sidecar.ts // for why Java's ADOT wiring needs an init container at all, unlike // Node's NODE_OPTIONS-only approach in analytics-service-stack.ts. - JAVA_TOOL_OPTIONS: `-Xmx1536m -XX:MaxMetaspaceSize=192m ${JAVA_ADOT_AGENT_FLAG}`, + // MaxMetaspaceSize is 384m, not the 192m this ran with before Phase 1: + // 192m was sized for a plain Spring Boot app, and the ADOT javaagent + // added here instruments Spring/Tomcat/JDBC/Lettuce/Kafka/Elasticsearch/ + // the AWS SDK, generating enough extra loaded classes to exhaust it. + // A real deploy died with "OutOfMemoryError: Metaspace" once traffic + // hit it — and because a metaspace-starved JVM can't load new classes, + // the failure surfaced as the Redis/Elasticsearch health indicators + // throwing, so /actuator/health reported DOWN and every task was + // pulled from the load balancer while the app itself still served + // requests. Budget at 3072 task MiB: 1536 heap + 384 metaspace + + // ~350 JVM overhead + the sidecar's 256 reservation still leaves room. + JAVA_TOOL_OPTIONS: `-Xmx1536m -XX:MaxMetaspaceSize=384m ${JAVA_ADOT_AGENT_FLAG}`, ...otelEnvVars('escld-backend'), ...(props.mskClusterArn ? { KAFKA_ENABLED: 'true', KAFKA_CLUSTER_ARN: props.mskClusterArn } @@ -200,18 +211,34 @@ export class BackendServiceStack extends cdk.Stack { SPRING_DATASOURCE_PASSWORD: ecs.Secret.fromSecretsManager(props.dbSecret, 'password'), FEED_CURSOR_SIGNING_KEY: ecs.Secret.fromSecretsManager(feedCursorSigningKey), }, - // Same /dev/tcp raw-socket probe docker-compose uses (see - // docker-compose.yaml backend healthcheck comment) — the eclipse-temurin - // JRE base image ships neither curl nor wget. + // A /dev/tcp raw-socket probe, because the eclipse-temurin JRE base + // image ships neither curl nor wget. + // + // 'CMD' + an explicit 'bash', NOT 'CMD-SHELL': ECS runs a CMD-SHELL + // string through /bin/sh, and /bin/sh in eclipse-temurin:25-jre is a + // symlink to dash, which does not implement /dev/tcp at all (verified + // by running the real image: "sh: cannot create /dev/tcp/...: + // Directory nonexistent"). Under CMD-SHELL this probe could therefore + // never pass however healthy the app was, so ECS marked every task + // UNHEALTHY and replaced it forever. /dev/tcp is a bash builtin and + // bash is present at /usr/bin/bash, so invoking it explicitly is the + // fix. Do not "simplify" this back to CMD-SHELL. healthCheck: { command: [ - 'CMD-SHELL', + 'CMD', + 'bash', + '-c', 'exec 3<>/dev/tcp/127.0.0.1/9090 && printf \'GET /actuator/health HTTP/1.1\\r\\nHost: localhost\\r\\nConnection: close\\r\\n\\r\\n\' >&3 && grep -q \'"status":"UP"\' <&3', ], interval: cdk.Duration.seconds(10), timeout: cdk.Duration.seconds(5), retries: 10, - startPeriod: cdk.Duration.seconds(30), + // Measured against a real deploy: this container reaches "Started + // BackendApplication" ~85s after its first log line (JVM + the ADOT + // javaagent's class-loading + Spring context + Flyway). A 30s + // startPeriod meant the probe spent ~55s of that boot burning + // retries for no reason. + startPeriod: cdk.Duration.seconds(120), }, }); container.addPortMappings( @@ -242,6 +269,15 @@ export class BackendServiceStack extends cdk.Stack { // task. 100/200 keeps full capacity up throughout every deploy instead. minHealthyPercent: 100, maxHealthyPercent: 200, + // CDK defaults this to 60s the moment a load balancer target is + // attached, which is shorter than this app's own boot: a real deploy + // measured 83-85s from container start to a serving /actuator/health, + // while the ALB declares a fresh target unhealthy after 2 failed + // checks (2 x 30s = 60s). The grace period expired ~25s before the + // app could ever answer, so ECS killed every task mid-boot and the + // service crash-looped forever with a perfectly healthy application. + // 240s leaves real headroom over the measured 85s for a cold start. + healthCheckGracePeriod: cdk.Duration.seconds(240), }); const targetGroup = new elbv2.ApplicationTargetGroup(this, 'TargetGroup', { @@ -256,6 +292,21 @@ export class BackendServiceStack extends cdk.Stack { port: '9090', path: '/actuator/health', healthyHttpCodes: '200', + // Explicit, not the ALB defaults (30s interval / 5 healthy / 2 + // unhealthy), because those defaults interact badly with an ~85s JVM + // boot in both directions: + // - going healthy took 5 x 30s = 150s AFTER boot, i.e. ~235s from + // task start, which barely fits inside the grace period above and + // left no margin for a slow ECR pull or a busy host. + // - going unhealthy took only 2 x 30s = 60s, so one blip in a shared + // dependency could yank every task out of service at once. + // 15s/2 reaches healthy ~30s after boot (~115s total, comfortably + // inside the grace period) while 5 failures now need 75s, making a + // transient stumble less likely to cascade into a full outage. + interval: cdk.Duration.seconds(15), + timeout: cdk.Duration.seconds(5), + healthyThresholdCount: 2, + unhealthyThresholdCount: 5, }, }); @@ -313,7 +364,14 @@ export class BackendServiceStack extends cdk.Stack { notify(new cloudwatch.Alarm(this, 'HighErrorRateAlarm', { alarmDescription: 'Backend 5xx rate above 5% over 5 minutes', metric: new cloudwatch.MathExpression({ - expression: 'errors / MAX([requests, 1])', + // Guards divide-by-zero on idle periods. NOT `MAX([requests, 1])`: + // CloudWatch's MAX() reduces one time series to a scalar, it is not an + // element-wise max over two operands, so that form is rejected at + // alarm-creation time with "Unsupported operand type(s) for MAX: + // [Array[TimeSeries, Scalar]]". IF() is the construct that actually + // does per-datapoint branching — no requests in a period reads as a + // 0% error rate rather than a gap. + expression: 'IF(requests > 0, errors / requests, 0)', usingMetrics: { errors: errorCount, requests: requestCount }, period: cdk.Duration.minutes(5), }), diff --git a/infra/lib/cache-stack.ts b/infra/lib/cache-stack.ts index f043b9b..c3546a3 100644 --- a/infra/lib/cache-stack.ts +++ b/infra/lib/cache-stack.ts @@ -39,11 +39,11 @@ export class CacheStack extends cdk.Stack { this.securityGroup.addIngressRule( props.appServiceSecurityGroup, ec2.Port.tcp(6379), - 'Backend Fargate tasks -> Redis', + 'Backend Fargate tasks to Redis', ); const subnetGroup = new elasticache.CfnSubnetGroup(this, 'RedisSubnetGroup', { - description: 'Isolated subnets — Redis has no route to the internet and needs none', + description: 'Isolated subnets - Redis has no route to the internet and needs none', subnetIds: props.vpc.selectSubnets({ subnetType: ec2.SubnetType.PRIVATE_ISOLATED }).subnetIds, }); diff --git a/infra/lib/compute-stack.ts b/infra/lib/compute-stack.ts index 631d254..d3a2b4f 100644 --- a/infra/lib/compute-stack.ts +++ b/infra/lib/compute-stack.ts @@ -54,7 +54,7 @@ export class ComputeStack extends cdk.Stack { this.albSecurityGroup = new ec2.SecurityGroup(this, 'AlbSecurityGroup', { vpc: props.vpc, - description: 'escld ALB — inbound 80/443 from the internet, outbound to ECS tasks', + description: 'escld ALB - inbound 80/443 from the internet, outbound to ECS tasks', allowAllOutbound: true, }); this.albSecurityGroup.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(80), 'HTTP from the internet'); diff --git a/infra/lib/database-stack.ts b/infra/lib/database-stack.ts index 9b62b7f..8c9a79d 100644 --- a/infra/lib/database-stack.ts +++ b/infra/lib/database-stack.ts @@ -7,6 +7,12 @@ export interface DatabaseStackProps extends cdk.StackProps { vpc: ec2.IVpc; /** Backend Fargate tasks' security group (from ComputeStack) — granted inbound 5432 below. */ appServiceSecurityGroup: ec2.ISecurityGroup; + /** Defaults to true (the production posture: automatic failover, see the + * instance's own comment). Set false only for a throwaway test deploy — + * Multi-AZ needs RDS capacity in two AZs simultaneously and costs roughly + * double, and a real `insufficient-capacity` failure in eu-central-1 is + * what made this configurable. Wired to the `dbMultiAz` CDK context flag. */ + multiAz?: boolean; } export class DatabaseStack extends cdk.Stack { @@ -33,18 +39,18 @@ export class DatabaseStack extends cdk.Stack { this.dbSecurityGroup.addIngressRule( props.appServiceSecurityGroup, ec2.Port.tcp(5432), - 'Backend Fargate tasks -> Postgres', + 'Backend Fargate tasks to Postgres', ); this.authLambdaSecurityGroup = new ec2.SecurityGroup(this, 'AuthLambdaSecurityGroup', { vpc: props.vpc, - description: 'escld Amplify Cognito trigger Lambdas (postConfirmation, preSignUp) — VPC-attached so they can reach Postgres, a PRIVATE_ISOLATED-subnet instance no default (non-VPC) Lambda execution environment can route to at all', + description: 'escld Amplify Cognito trigger Lambdas (postConfirmation, preSignUp) - VPC-attached so they can reach Postgres, a PRIVATE_ISOLATED-subnet instance no default (non-VPC) Lambda execution environment can route to at all', allowAllOutbound: true, }); this.dbSecurityGroup.addIngressRule( this.authLambdaSecurityGroup, ec2.Port.tcp(5432), - 'Amplify Cognito trigger Lambdas -> Postgres', + 'Amplify Cognito trigger Lambdas to Postgres', ); // users/posts/comments — the relational, admin-queryable data the moderation @@ -59,14 +65,20 @@ export class DatabaseStack extends cdk.Stack { // Right-sized for the ~350-450 req/s peak / ~5-8 backend replicas estimated // for 100k DAU (see plan) — a connection ceiling of a few hundred comfortably // covers 4-8 replicas x a Hikari pool of 10. Bump before adding a read replica. - instanceType: ec2.InstanceType.of(ec2.InstanceClass.T4G, ec2.InstanceSize.MEDIUM), + // + // T3 (Intel), not T4G (Graviton): a real `insufficient-capacity` failure on + // db.t4g.medium in eu-central-1 blocked this stack's first deploy. Graviton + // RDS pools in this region run tight; T3 draws from a separate, broader pool + // for the same class size and near-identical price. Revisit T4G (~10-20% + // cheaper) once capacity there is reliable. + instanceType: ec2.InstanceType.of(ec2.InstanceClass.T3, ec2.InstanceSize.MEDIUM), allocatedStorage: 50, maxAllocatedStorage: 200, storageType: rds.StorageType.GP3, // Multi-AZ for automatic failover, not for read scaling — the load // estimate doesn't justify a read replica on day one (see plan's - // "what to revisit" section). - multiAz: true, + // "what to revisit" section). Defaults on; see props.multiAz. + multiAz: props.multiAz ?? true, databaseName: 'escld', credentials: rds.Credentials.fromGeneratedSecret('escld_app'), backupRetention: cdk.Duration.days(7), diff --git a/infra/lib/event-streaming-stack.ts b/infra/lib/event-streaming-stack.ts index abf751a..40c74f3 100644 --- a/infra/lib/event-streaming-stack.ts +++ b/infra/lib/event-streaming-stack.ts @@ -40,13 +40,13 @@ export class EventStreamingStack extends cdk.Stack { this.securityGroup = new ec2.SecurityGroup(this, 'MskSecurityGroup', { vpc: props.vpc, - description: 'escld MSK Serverless — Kafka (SASL/IAM, port 9098) from app services only', + description: 'escld MSK Serverless - Kafka (SASL/IAM, port 9098) from app services only', allowAllOutbound: false, }); this.securityGroup.addIngressRule( props.appServiceSecurityGroup, ec2.Port.tcp(9098), - 'Backend producer / bq-sink consumer -> MSK', + 'Backend producer / bq-sink consumer to MSK', ); this.cluster = new msk.CfnServerlessCluster(this, 'Cluster', { diff --git a/infra/lib/rtmp-service-stack.ts b/infra/lib/rtmp-service-stack.ts index a0c392f..ced9d5a 100644 --- a/infra/lib/rtmp-service-stack.ts +++ b/infra/lib/rtmp-service-stack.ts @@ -123,13 +123,13 @@ export class RtmpServiceStack extends cdk.Stack { const securityGroup = new ec2.SecurityGroup(this, 'RtmpSecurityGroup', { vpc: props.vpc, - description: 'escld rtmp — RTMP ingest (1935) public (any encoder, anywhere), HTTP (4001) from the ALB only', + description: 'escld rtmp - RTMP ingest (1935) public (any encoder, anywhere), HTTP (4001) from the ALB only', allowAllOutbound: true, }); securityGroup.addIngressRule( ec2.Peer.anyIpv4(), ec2.Port.tcp(1935), - 'RTMP publish — encoders (OBS, ffmpeg) connect directly, cannot go through the ALB', + 'RTMP publish - encoders (OBS, ffmpeg) connect directly, cannot go through the ALB', ); securityGroup.addIngressRule(props.albSecurityGroup, ec2.Port.tcp(4001), 'Health check + local HLS serving from the ALB'); diff --git a/infra/lib/search-stack.ts b/infra/lib/search-stack.ts index 86717a6..46a0fbf 100644 --- a/infra/lib/search-stack.ts +++ b/infra/lib/search-stack.ts @@ -51,13 +51,13 @@ export class SearchStack extends cdk.Stack { this.securityGroup = new ec2.SecurityGroup(this, 'SearchSecurityGroup', { vpc: props.vpc, - description: 'escld Elasticsearch — no auth (xpack.security disabled, matching dev), so this must stay app-tier-only, never public', + description: 'escld Elasticsearch - no auth (xpack.security disabled, matching dev), so this must stay app-tier-only, never public', allowAllOutbound: true, }); this.securityGroup.addIngressRule( props.appServiceSecurityGroup, ec2.Port.tcp(9200), - 'App-tier Fargate tasks -> Elasticsearch', + 'App-tier Fargate tasks to Elasticsearch', ); const fileSystem = new efs.FileSystem(this, 'SearchData', { @@ -66,7 +66,7 @@ export class SearchStack extends cdk.Stack { encrypted: true, removalPolicy: cdk.RemovalPolicy.DESTROY, }); - fileSystem.connections.allowDefaultPortFrom(this.securityGroup, 'Fargate task -> EFS mount target (NFS)'); + fileSystem.connections.allowDefaultPortFrom(this.securityGroup, 'Fargate task to EFS mount target (NFS)'); const accessPoint = fileSystem.addAccessPoint('SearchDataAccessPoint', { path: '/elasticsearch-data', @@ -107,8 +107,14 @@ export class SearchStack extends cdk.Stack { command: ['CMD-SHELL', 'curl -sf http://localhost:9200/_cluster/health || exit 1'], interval: cdk.Duration.seconds(10), timeout: cdk.Duration.seconds(5), - retries: 20, - startPeriod: cdk.Duration.seconds(60), + // ECS caps retries at 10 — a previous `retries: 20` here failed the + // CreateTaskDefinition API outright. The intent was a long grace + // period for Elasticsearch's slow first boot (JVM start + index + // recovery off the EFS volume), so that budget moves into + // startPeriod, which has no such cap: 180s before health checks + // count at all, then 10 x 10s of retries on top. + retries: 10, + startPeriod: cdk.Duration.seconds(180), }, }); container.addPortMappings({ containerPort: 9200 }); @@ -122,7 +128,21 @@ export class SearchStack extends cdk.Stack { cluster: props.cluster, taskDefinition, securityGroups: [this.securityGroup], - vpcSubnets: { subnetType: ec2.SubnetType.PRIVATE_ISOLATED }, + // PRIVATE_WITH_EGRESS, not PRIVATE_ISOLATED: a Fargate task needs to + // reach ECR (image pull) and CloudWatch Logs (the awslogs driver) before + // the container ever starts. From an isolated subnet with no NAT and no + // ECR/Logs VPC endpoints, that fails at task start with + // "ResourceInitializationError: ... cannot find the Amazon CloudWatch log + // group ... connection issue between the task and Amazon CloudWatch" — + // which is exactly what blocked this stack's first deploy. + // + // This does not weaken the "app-tier-only, never public" rule this + // stack's security group enforces (see its description): these subnets + // still have no inbound route from the internet, and nothing on the ALB + // routes here. The alternative — interface endpoints for ecr.api, + // ecr.dkr and logs, plus an S3 gateway endpoint — costs real money per + // AZ to buy back outbound access the NAT already provides. + vpcSubnets: { subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS }, // Single node by design (see class doc) — do not raise this without // first giving Elasticsearch real multi-node cluster configuration. desiredCount: 1, diff --git a/infra/lib/ws-sfu-stack.ts b/infra/lib/ws-sfu-stack.ts index 5ef95a5..24cd0ed 100644 --- a/infra/lib/ws-sfu-stack.ts +++ b/infra/lib/ws-sfu-stack.ts @@ -97,14 +97,14 @@ export class WsSfuStack extends cdk.Stack { const securityGroup = new ec2.SecurityGroup(this, 'WsSfuSecurityGroup', { vpc: props.vpc, - description: 'escld ws-sfu — signaling from the ALB only, WebRTC media UDP range public (no way around this for a self-hosted SFU)', + description: 'escld ws-sfu - signaling from the ALB only, WebRTC media UDP range public (no way around this for a self-hosted SFU)', allowAllOutbound: true, }); securityGroup.addIngressRule(props.albSecurityGroup, ec2.Port.tcp(4000), 'Socket.IO signaling from the ALB'); securityGroup.addIngressRule( ec2.Peer.anyIpv4(), ec2.Port.udpRange(40000, 40999), - 'WebRTC media (RTP/RTCP) — clients connect directly, cannot go through the ALB', + 'WebRTC media (RTP/RTCP) - clients connect directly, cannot go through the ALB', ); const role = new iam.Role(this, 'InstanceRole', { diff --git a/infra/test/backend-service-stack.test.ts b/infra/test/backend-service-stack.test.ts index fa98592..e847f7a 100644 --- a/infra/test/backend-service-stack.test.ts +++ b/infra/test/backend-service-stack.test.ts @@ -109,6 +109,35 @@ test('health-checks the actuator port, not the JWT-protected public API port', ( }); }); +test('gives the JVM longer to boot than the ALB takes to declare a target unhealthy', () => { + const app = new cdk.App(); + const { backend } = buildStack(app, '3a'); + const template = Template.fromStack(backend); + + // A real deploy measured 83-85s from container start to a serving + // /actuator/health. CDK's own default grace period (60s, applied silently + // once an ALB target is attached) is shorter than that, and the ALB gives + // up after UnhealthyThresholdCount x Interval — so every task was killed + // mid-boot and the service crash-looped with a healthy application. + // Guard the invariant, not just the literal: grace must outlast the ALB. + const service = Object.values( + template.findResources('AWS::ECS::Service'), + )[0] as { Properties: { HealthCheckGracePeriodSeconds: number } }; + const targetGroup = Object.values( + template.findResources('AWS::ElasticLoadBalancingV2::TargetGroup'), + )[0] as { Properties: { HealthCheckIntervalSeconds?: number; UnhealthyThresholdCount?: number } }; + + const grace = service.Properties.HealthCheckGracePeriodSeconds; + // Fall back to the ALB's own documented defaults when the template leaves + // them implicit, which is exactly the case that bit us. + const albGivesUpAfter = + (targetGroup.Properties.HealthCheckIntervalSeconds ?? 30) * + (targetGroup.Properties.UnhealthyThresholdCount ?? 2); + + expect(grace).toBeGreaterThan(albGivesUpAfter); + expect(grace).toBeGreaterThan(85 * 2); +}); + test('grants the task role read/write on every DynamoDB table and send on both SQS queues', () => { const app = new cdk.App(); const { backend } = buildStack(app, '4'); diff --git a/infra/test/database-stack.test.ts b/infra/test/database-stack.test.ts index 04faf5b..7ee2597 100644 --- a/infra/test/database-stack.test.ts +++ b/infra/test/database-stack.test.ts @@ -90,7 +90,7 @@ test('grants a dedicated security group (for the out-of-app Amplify Cognito trig IpProtocol: 'tcp', FromPort: 5432, ToPort: 5432, - Description: 'Amplify Cognito trigger Lambdas -> Postgres', + Description: 'Amplify Cognito trigger Lambdas to Postgres', }); const outputs = template.findOutputs('*');