Commit c174566cba3a1ba475c3d6ae05dc238cfb0ee165
graphlemon: merge e66b3bef-f6c6-47e0-8cfe-3c9bb7fd988d
Showing
2 changed files
with
207 additions
and
9 deletions
supervisor/deploy_stage.py
| ... | ... | @@ -28,6 +28,7 @@ staging and cutover would otherwise lose track of it entirely. |
| 28 | 28 | import os |
| 29 | 29 | import shutil |
| 30 | 30 | import socket |
| 31 | +import sqlite3 | |
| 31 | 32 | import subprocess |
| 32 | 33 | import sys |
| 33 | 34 | import uuid |
| ... | ... | @@ -42,6 +43,34 @@ class CheckoutFailedError(Exception): |
| 42 | 43 | time this is raised.""" |
| 43 | 44 | |
| 44 | 45 | |
| 46 | +class ConcurrentDeploymentError(Exception): | |
| 47 | + """CONCURRENT_DEPLOYMENT -- ux_deployment_active unique index detected an | |
| 48 | + existing deployment row not yet in a terminal state (finalized/ | |
| 49 | + rolled_back/blocked_pending_human) when attempting to INSERT a new one. | |
| 50 | + Unlike CheckoutFailedError, the INSERT never committed, so there is no | |
| 51 | + half-finished deployment row to mark blocked_pending_human -- callers | |
| 52 | + (the Deploy Supervisor polling loop) should treat this as 'skip this | |
| 53 | + round, retry next poll', not as a failure needing human intervention. | |
| 54 | + Diagnostic fields are real attributes, not just baked into the message: | |
| 55 | + - git_commit_hash: the commit hash this rejected call attempted to stage. | |
| 56 | + - blocking_deployment_id / blocking_status: the deployment row currently | |
| 57 | + holding the slot (if it could be located via a best-effort read-only | |
| 58 | + query); both None if it could not be located -- the exception is | |
| 59 | + still raised in that case. | |
| 60 | + """ | |
| 61 | + | |
| 62 | + def __init__(self, git_commit_hash, blocking_deployment_id=None, blocking_status=None): | |
| 63 | + self.git_commit_hash = git_commit_hash | |
| 64 | + self.blocking_deployment_id = blocking_deployment_id | |
| 65 | + self.blocking_status = blocking_status | |
| 66 | + message = "cannot stage commit {}: a deployment is already in progress".format(git_commit_hash) | |
| 67 | + if blocking_deployment_id is not None: | |
| 68 | + message += " (blocking deployment_id={}, status={})".format( | |
| 69 | + blocking_deployment_id, blocking_status | |
| 70 | + ) | |
| 71 | + super().__init__(message) | |
| 72 | + | |
| 73 | + | |
| 45 | 74 | def _last_finalized_deployment(conn): |
| 46 | 75 | """Returns (git_commit_hash, candidate_instance_dir) of the most recent |
| 47 | 76 | `finalized` deployment, or None if there isn't one yet (the bootstrap |
| ... | ... | @@ -145,6 +174,37 @@ def _launch_candidate(candidate_dir, port, db_path, python_executable=None): |
| 145 | 174 | ) |
| 146 | 175 | |
| 147 | 176 | |
| 177 | +def _is_concurrent_deployment_conflict(error): | |
| 178 | + """Distinguish a ux_deployment_active unique-index conflict from any | |
| 179 | + other IntegrityError. sqlite reports named UNIQUE INDEX conflicts with | |
| 180 | + the index name embedded in the message, e.g. "UNIQUE constraint | |
| 181 | + failed: index 'ux_deployment_active'". Match only this exact index | |
| 182 | + name string -- do not broadly catch all IntegrityErrors, or unrelated | |
| 183 | + integrity errors would be misclassified and masked as concurrency | |
| 184 | + conflicts. | |
| 185 | + """ | |
| 186 | + return "ux_deployment_active" in str(error) | |
| 187 | + | |
| 188 | + | |
| 189 | +def _find_blocking_deployment(conn): | |
| 190 | + """Read-only diagnostic query: find the non-terminal deployment row | |
| 191 | + currently occupying the ux_deployment_active slot. Only called after a | |
| 192 | + conflict has already been detected, purely to enrich the exception. If | |
| 193 | + the query itself fails (e.g. a broken connection), swallow the | |
| 194 | + sqlite3.Error and return None -- the caller still raises | |
| 195 | + ConcurrentDeploymentError, just without blocking_deployment_id/status. | |
| 196 | + """ | |
| 197 | + try: | |
| 198 | + row = conn.execute( | |
| 199 | + "SELECT deployment_id, status FROM deployment " | |
| 200 | + "WHERE status NOT IN ('finalized', 'rolled_back', 'blocked_pending_human') " | |
| 201 | + "ORDER BY rowid DESC LIMIT 1" | |
| 202 | + ).fetchone() | |
| 203 | + return tuple(row) if row else None | |
| 204 | + except sqlite3.Error: | |
| 205 | + return None | |
| 206 | + | |
| 207 | + | |
| 148 | 208 | def stage_deployment( |
| 149 | 209 | conn, git_commit_hash, repo_root=None, prod_db_path=None, |
| 150 | 210 | checkout_parent_dir=None, port=None, python_executable=None, |
| ... | ... | @@ -167,13 +227,25 @@ def stage_deployment( |
| 167 | 227 | previous_commit_hash = _current_commit(repo_root) |
| 168 | 228 | previous_instance_dir = repo_root |
| 169 | 229 | |
| 170 | - conn.execute( | |
| 171 | - "INSERT INTO deployment " | |
| 172 | - "(deployment_id, git_commit_hash, previous_commit_hash, status, previous_instance_dir, started_at) " | |
| 173 | - "VALUES (?, ?, ?, 'staged', ?, CURRENT_TIMESTAMP)", | |
| 174 | - (deployment_id, git_commit_hash, previous_commit_hash, previous_instance_dir), | |
| 175 | - ) | |
| 176 | - conn.commit() | |
| 230 | + try: | |
| 231 | + conn.execute( | |
| 232 | + "INSERT INTO deployment " | |
| 233 | + "(deployment_id, git_commit_hash, previous_commit_hash, status, previous_instance_dir, started_at) " | |
| 234 | + "VALUES (?, ?, ?, 'staged', ?, CURRENT_TIMESTAMP)", | |
| 235 | + (deployment_id, git_commit_hash, previous_commit_hash, previous_instance_dir), | |
| 236 | + ) | |
| 237 | + conn.commit() | |
| 238 | + except sqlite3.IntegrityError as e: | |
| 239 | + conn.rollback() | |
| 240 | + if not _is_concurrent_deployment_conflict(e): | |
| 241 | + raise | |
| 242 | + blocking = _find_blocking_deployment(conn) | |
| 243 | + blocking_deployment_id, blocking_status = blocking if blocking else (None, None) | |
| 244 | + raise ConcurrentDeploymentError( | |
| 245 | + git_commit_hash=git_commit_hash, | |
| 246 | + blocking_deployment_id=blocking_deployment_id, | |
| 247 | + blocking_status=blocking_status, | |
| 248 | + ) from e | |
| 177 | 249 | |
| 178 | 250 | try: |
| 179 | 251 | candidate_dir = _checkout_candidate(repo_root, git_commit_hash, deployment_id, checkout_parent_dir) | ... | ... |
supervisor/test_deploy_stage.py
| ... | ... | @@ -9,6 +9,7 @@ Run: python3 -m unittest discover -s supervisor -p "test_*.py" -v |
| 9 | 9 | |
| 10 | 10 | import os |
| 11 | 11 | import shutil |
| 12 | +import sqlite3 | |
| 12 | 13 | import subprocess |
| 13 | 14 | import sys |
| 14 | 15 | import tempfile |
| ... | ... | @@ -171,13 +172,19 @@ class StageDeploymentHappyPathTest(DeployStageTestBase): |
| 171 | 172 | def test_only_one_non_terminal_deployment_allowed(self): |
| 172 | 173 | with patch.object(deploy_stage, "_launch_candidate") as mock_launch: |
| 173 | 174 | mock_launch.return_value.pid = 1 |
| 174 | - deploy_stage.stage_deployment( | |
| 175 | + first = deploy_stage.stage_deployment( | |
| 175 | 176 | self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9001, |
| 176 | 177 | ) |
| 177 | - with self.assertRaises(Exception): # schema's ux_deployment_active partial unique index | |
| 178 | + with self.assertRaises(deploy_stage.ConcurrentDeploymentError) as ctx: | |
| 178 | 179 | deploy_stage.stage_deployment( |
| 179 | 180 | self.conn, self.commit_v1, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9002, |
| 180 | 181 | ) |
| 182 | + err = ctx.exception | |
| 183 | + self.assertEqual(err.git_commit_hash, self.commit_v1) | |
| 184 | + self.assertEqual(err.blocking_deployment_id, first["deployment_id"]) | |
| 185 | + self.assertEqual(err.blocking_status, "candidate_starting") | |
| 186 | + count = self.conn.execute("SELECT COUNT(*) FROM deployment").fetchone()[0] | |
| 187 | + self.assertEqual(count, 1) | |
| 181 | 188 | |
| 182 | 189 | |
| 183 | 190 | class CheckoutFailedTest(DeployStageTestBase): |
| ... | ... | @@ -198,6 +205,125 @@ class CheckoutFailedTest(DeployStageTestBase): |
| 198 | 205 | mock_launch.assert_not_called() |
| 199 | 206 | |
| 200 | 207 | |
| 208 | +class ConcurrentDeploymentTest(DeployStageTestBase): | |
| 209 | + """Tests for `ConcurrentDeploymentError` -- the diagnosable replacement | |
| 210 | + for the raw `sqlite3.IntegrityError` `ux_deployment_active` used to let | |
| 211 | + escape uncaught.""" | |
| 212 | + | |
| 213 | + # Every deployment.status value from src/schema.py's CHECK constraint | |
| 214 | + # that is NOT in ux_deployment_active's exclusion set (finalized/ | |
| 215 | + # rolled_back/blocked_pending_human) -- each of these must block a new | |
| 216 | + # stage_deployment() call, including the less-obvious terminal-looking | |
| 217 | + # ones (health_check_failed, rollback_failed) that are NOT actually | |
| 218 | + # excluded by the index. | |
| 219 | + _NON_TERMINAL_STATUSES = [ | |
| 220 | + "staged", "candidate_starting", "health_check_running", "health_check_passed", | |
| 221 | + "health_check_failed", "cutover_in_progress", "live", "monitoring", | |
| 222 | + "anomaly_detected", "rollback_in_progress", "rollback_failed", "monitoring_window_clean", | |
| 223 | + ] | |
| 224 | + | |
| 225 | + def _seed_deployment_row(self, deployment_id, status): | |
| 226 | + self.conn.execute( | |
| 227 | + "INSERT INTO deployment (deployment_id, git_commit_hash, previous_commit_hash, status, " | |
| 228 | + "candidate_instance_dir, previous_instance_dir) " | |
| 229 | + "VALUES (?, 'seed-commit', 'seed-previous-commit', ?, '/seed/candidate/dir', '/seed/previous/dir')", | |
| 230 | + (deployment_id, status), | |
| 231 | + ) | |
| 232 | + self.conn.commit() | |
| 233 | + | |
| 234 | + def test_rolled_back_prior_deployment_allows_new_staging(self): | |
| 235 | + self._seed_deployment_row("dep-rolled-back", "rolled_back") | |
| 236 | + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: | |
| 237 | + mock_launch.return_value.pid = 1 | |
| 238 | + result = deploy_stage.stage_deployment( | |
| 239 | + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9010, | |
| 240 | + ) | |
| 241 | + self.assertEqual(result["status"], "candidate_starting") | |
| 242 | + row = self.conn.execute( | |
| 243 | + "SELECT status FROM deployment WHERE deployment_id = ?", (result["deployment_id"],) | |
| 244 | + ).fetchone() | |
| 245 | + self.assertEqual(row[0], "candidate_starting") | |
| 246 | + | |
| 247 | + def test_blocked_pending_human_prior_deployment_allows_new_staging(self): | |
| 248 | + self._seed_deployment_row("dep-blocked", "blocked_pending_human") | |
| 249 | + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: | |
| 250 | + mock_launch.return_value.pid = 1 | |
| 251 | + result = deploy_stage.stage_deployment( | |
| 252 | + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9011, | |
| 253 | + ) | |
| 254 | + self.assertEqual(result["status"], "candidate_starting") | |
| 255 | + row = self.conn.execute( | |
| 256 | + "SELECT status FROM deployment WHERE deployment_id = ?", (result["deployment_id"],) | |
| 257 | + ).fetchone() | |
| 258 | + self.assertEqual(row[0], "candidate_starting") | |
| 259 | + | |
| 260 | + def test_is_concurrent_deployment_conflict_matches_only_the_exact_index_name(self): | |
| 261 | + self.assertTrue( | |
| 262 | + deploy_stage._is_concurrent_deployment_conflict( | |
| 263 | + sqlite3.IntegrityError("UNIQUE constraint failed: index 'ux_deployment_active'") | |
| 264 | + ) | |
| 265 | + ) | |
| 266 | + self.assertFalse( | |
| 267 | + deploy_stage._is_concurrent_deployment_conflict( | |
| 268 | + sqlite3.IntegrityError("NOT NULL constraint failed: deployment.git_commit_hash") | |
| 269 | + ) | |
| 270 | + ) | |
| 271 | + self.assertFalse( | |
| 272 | + deploy_stage._is_concurrent_deployment_conflict( | |
| 273 | + sqlite3.IntegrityError("UNIQUE constraint failed: index 'ux_some_other_index'") | |
| 274 | + ) | |
| 275 | + ) | |
| 276 | + | |
| 277 | + def test_find_blocking_deployment_returns_most_recently_inserted_by_rowid(self): | |
| 278 | + # ux_deployment_active itself forbids two non-terminal rows from | |
| 279 | + # coexisting (that's the whole point of this fix) -- drop it just | |
| 280 | + # for this test so two can be seeded directly, to isolate and prove | |
| 281 | + # _find_blocking_deployment's own "most recent by rowid" ordering | |
| 282 | + # logic independent of that constraint. | |
| 283 | + self.conn.execute("DROP INDEX ux_deployment_active") | |
| 284 | + self._seed_deployment_row("dep-older", "staged") | |
| 285 | + self._seed_deployment_row("dep-newer", "candidate_starting") | |
| 286 | + blocking = deploy_stage._find_blocking_deployment(self.conn) | |
| 287 | + self.assertEqual(blocking, ("dep-newer", "candidate_starting")) | |
| 288 | + | |
| 289 | + def test_find_blocking_deployment_returns_none_on_broken_connection(self): | |
| 290 | + broken_conn = schema.connect(":memory:") | |
| 291 | + broken_conn.close() | |
| 292 | + self.assertIsNone(deploy_stage._find_blocking_deployment(broken_conn)) | |
| 293 | + | |
| 294 | + def test_conn_still_usable_after_conflict_is_caught(self): | |
| 295 | + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: | |
| 296 | + mock_launch.return_value.pid = 1 | |
| 297 | + deploy_stage.stage_deployment( | |
| 298 | + self.conn, self.commit_v2, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9012, | |
| 299 | + ) | |
| 300 | + with self.assertRaises(deploy_stage.ConcurrentDeploymentError): | |
| 301 | + deploy_stage.stage_deployment( | |
| 302 | + self.conn, self.commit_v1, repo_root=self.repo_dir, prod_db_path=self.prod_db_path, port=9013, | |
| 303 | + ) | |
| 304 | + # conn.rollback() in the except branch must clear the dangling | |
| 305 | + # implicit transaction -- a subsequent trivial query must not raise. | |
| 306 | + self.assertEqual(self.conn.execute("SELECT 1").fetchone()[0], 1) | |
| 307 | + | |
| 308 | + def test_every_non_excluded_status_blocks_staging_with_concurrent_deployment_error(self): | |
| 309 | + with patch.object(deploy_stage, "_launch_candidate") as mock_launch: | |
| 310 | + for status in self._NON_TERMINAL_STATUSES: | |
| 311 | + deployment_id = "dep-status-{}".format(status) | |
| 312 | + self._seed_deployment_row(deployment_id, status) | |
| 313 | + with self.assertRaises(deploy_stage.ConcurrentDeploymentError, msg=status) as ctx: | |
| 314 | + deploy_stage.stage_deployment( | |
| 315 | + self.conn, self.commit_v2, repo_root=self.repo_dir, | |
| 316 | + prod_db_path=self.prod_db_path, port=9020, | |
| 317 | + ) | |
| 318 | + self.assertEqual(ctx.exception.blocking_deployment_id, deployment_id, status) | |
| 319 | + self.assertEqual(ctx.exception.blocking_status, status, status) | |
| 320 | + mock_launch.assert_not_called() | |
| 321 | + count = self.conn.execute("SELECT COUNT(*) FROM deployment").fetchone()[0] | |
| 322 | + self.assertEqual(count, 1, status) | |
| 323 | + self.conn.execute("DELETE FROM deployment WHERE deployment_id = ?", (deployment_id,)) | |
| 324 | + self.conn.commit() | |
| 325 | + | |
| 326 | + | |
| 201 | 327 | class LaunchCandidateArgsTest(unittest.TestCase): |
| 202 | 328 | """2026-08-06 (task_reconciliation): `_launch_candidate`'s `db_path` is |
| 203 | 329 | always a COPY of the real production db -- without `--disable- | ... | ... |