docs: SEV1 post-mortem for model-engine 500s incident (MLI-6574)

[lilyz-ai]

Summary

• Adds post-mortem one-pager for the Apr 24–25 SEV1 where a kubectl set image deployed a model-engine image that referenced endpoints.temporal_task_queue before the DB migration was applied, causing ~47 min of 500s on all endpoint operations
• Document lives in docs/internal/ alongside other internal ops docs
• Covers: timeline, root cause + contributing factors, impact, 5 action items, lessons learned

Test plan

• Review doc for accuracy against incident timeline
• Confirm action item owners

Closes MLI-6574

🤖 Generated with Claude Code

Greptile Summary

This PR adds a post-mortem for the Apr 24–25 SEV1 where a kubectl set image bypassed Helm migration hooks and caused ~58 min of 500s on all model-engine endpoint operations. It also silently promotes a new production image tag in values_sample.yaml.

Previous review rounds have identified several significant issues in the action-item code samples that need resolution before the document is referenced by engineers: the _check_master_branch justfile guard targets origin/master (the repo default branch is main), the ClusterRoleBinding snippet grants cluster-wide admin rather than the intended namespace-scoped access, the scale_developer RBAC role over-provisions create/delete on pods/configmaps/services, and the "chosen" Option A is not reflected in any of the implementation snippets which all implement Option B.

Confidence Score: 3/5

Not safe to merge until action-item code samples are corrected and the image tag bump is validated against applied migrations

Multiple P1 findings flagged in prior review rounds: the justfile guard references the wrong branch (origin/master vs origin/main), the ClusterRoleBinding grants cluster-wide admin defeating the stated security goal, the scale_developer RBAC role over-provisions destructive verbs, the chosen Option A is entirely absent from all implementation snippets, and the values_sample.yaml image tag change lacks confirmation that the DB migration is in place.

docs/internal/post-mortem-mli-6574-model-engine-500s.md requires corrections to the action-item code samples before it can serve as a reliable runbook; charts/model-engine/values_sample.yaml needs migration-safety confirmation before the image tag lands in production

Important Files Changed

Filename	Overview
docs/internal/post-mortem-mli-6574-model-engine-500s.md	New post-mortem document; multiple factual inconsistencies (duration values) and implementation errors in action-item code samples (wrong git branch ref, over-privileged RBAC, Option A/B mismatch, ClusterRoleBinding scope) flagged in prior review rounds
charts/model-engine/values_sample.yaml	Bumps the production image tag without explicit documentation that the accompanying Alembic migration has been applied, repeating the exact failure pattern documented in the post-mortem

Sequence Diagram

sequenceDiagram
    participant Eng as Engineer
    participant K8s as kubectl / K8s API
    participant App as model-engine Pod
    participant DB as PostgreSQL
    participant PD as PagerDuty

    Note over Eng,DB: Apr 24, 23:50Z — Incident begins
    Eng->>K8s: kubectl set image (04729cef…)
    Note over K8s: Helm pre-upgrade hook SKIPPED
    K8s->>App: New pod starts (ORM declares temporal_task_queue)
    App->>DB: SELECT … temporal_task_queue … FROM endpoints
    DB-->>App: ERROR: column does not exist
    App-->>Eng: HTTP 500 on every list/get/create/update/delete

    loop 5 patch attempts over ~34 min
        Eng->>K8s: kubectl set image (patch N)
        K8s->>App: New pod (no migration applied)
        App->>DB: Same failing SELECT
        DB-->>App: ERROR: column does not exist
    end

    Note over PD: 00:37Z — PagerDuty fires (47 min late)
    Note over Eng,DB: 00:48Z — Rollback to f395ffa6…
    Eng->>K8s: kubectl rollout (f395ffa6…)
    K8s->>App: Old pod (no temporal_task_queue in ORM)
    App->>DB: SELECT without missing column
    DB-->>App: OK
    App-->>Eng: HTTP 200 — errors clear

Loading

_{Reviews (22): Last reviewed commit: "docs(post-mortem): drop genai from audit..." | Re-trigger Greptile}

[greptile-apps]

Undocumented production image tag change

The PR description mentions only the post-mortem doc addition, but values_sample.yaml is quietly updated to a new image tag (2e9d00786419ef44ec5c9d3305d8d6451d6aabfb). The post-mortem itself notes that values_sample.yaml targets production (no separate staging config exists). Deploying a new image tag here without an explicit mention in the PR description or test plan risks repeating the exact pattern documented in the incident — an image promotion that bypasses scrutiny of whether the accompanying Alembic migration is in place. The test plan should confirm that the migration for this new image has been applied to production before this config lands.

Prompt To Fix With AI

This is a comment left during a code review.
Path: charts/model-engine/values_sample.yaml
Line: 27

Comment:
**Undocumented production image tag change**

The PR description mentions only the post-mortem doc addition, but `values_sample.yaml` is quietly updated to a new image tag (`2e9d00786419ef44ec5c9d3305d8d6451d6aabfb`). The post-mortem itself notes that `values_sample.yaml` targets production (no separate staging config exists). Deploying a new image tag here without an explicit mention in the PR description or test plan risks repeating the exact pattern documented in the incident — an image promotion that bypasses scrutiny of whether the accompanying Alembic migration is in place. The test plan should confirm that the migration for this new image has been applied to production before this config lands.

How can I resolve this? If you propose a fix, please make it concise.

[billyang-scale]

for option b, which service assume this new role?

[billyang-scale]

in what scenario, other team need to deploy to scale-deploywithml_infra_admin`?

[billyang-scale]

ideal world, no one should have mutate permission in prodution environment unless there is additional approval process.

regurlar or on demand release/deploymeny should be done by ci/cd pipeline / automation

[billyang-scale]

+1, need canary environment to test production changes before actually release to production

[yixu34]

I don't disagree but it seems like once we're running kubectl commands directly, that's already incorrect, regardless of where the command is pointed.

[billyang-scale]

do we have alert ready to capture if similar things happen in the future?

[lilyz-ai]

added a new section for alerting on failure over 50% in data plane Istio mesh metrics.

[lilyz-ai]

thanks @billyang-scale for reviewing! took another look, ml-infra role should only be used by our team in ml-serving-new. so scope down the role permission + a new deployer role is a better design. also added alerting.

[billyang-scale]

are this related to the scope of this doc? ml-training-new and circelci seems irrelavant

[sandeshghanta]

A quick chat told me it's possible to have a guardrail which ensures alembic migrations is in sync with ORM model

#!/usr/bin/env bash
set -euo pipefail

# use a clean test DB
alembic upgrade head

# Make sure alembic/env.py points at your ORM metadata:
# fail if models contain schema changes not represented by migrations
alembic check

[mayavkrishnan25]

could you add some context on who the entity was who ran the set-image - human user/some automated process

[yixu34]

Going forward let's please use the postmortem template. This is AI generated and only covers the technical aspects but doesn't go into the 5 why's.

[yixu34]

This can work, but if we're going to do this, then we'd also want to make sure that people can force merge things in the models repo. Otherwise, there's no available way to do emergency deploys.

[yixu34]

Think another issue is: while this is good in the sense that it would force a review of the PR, it assumes that everyone follows the same release checklist. Would something like greptile caught the fact that we were trying to push something without a corresponding migration?

[yixu34]

Do other teams do this?

[yixu34]

I don't disagree but it seems like once we're running kubectl commands directly, that's already incorrect, regardless of where the command is pointed.

[billyang-scale]

we should propose a release process to make sure release is ready for prod release. e.g

• no auto deployment in production.
• make cutline to release candidate in staging env, and let it back for certain time e.g 24h
• have monitor/test running make sure no regression
• release to prod expicitly with release candidate version

[sandeshghanta]

Confirming - can we do this today without being spammy?
Ex: Does model engine throw 5xx if user request is overwhelming a model?

[lilyz-ai]

Action items:

1. improve devx of model engine, to be on par with newer service, e.g. arp. @lilyz-ai to create a new design doc for model engine devx improvements.
2. prod deploy process documentation.
3. scope down ml-infra role permissions to avoid deploy.
4. have a separate deployer role to do prod deploy. allow force merge to allow emergency hot fix.
5. have separate staging env for model engine, where ml infra team can test things.

Discussions:

1. staging needs to follow prod deployment process, before prod deployment.
2. hot fixes, post release verification.
3. model engine testing locally, https://github.com/scaleapi/models/tree/master/model-engine-internal#running-the-server-locally.
4. sandbox local testing, @sandeshghanta will share a process doc.
5. developer experience improvement should be put in place first, so the guardrail is less relevant or will be needed less.