also looking at this
fix(deploy): allocate site ports per box, so a second attach cannot collide
#170
4 files
+643
-1
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -395,8 +395,15 @@ Ports do not compose. Every `server-app` route resolves to `from: | ||
| 395 | 395 | 'localhost:<port>'` on one shared loopback namespace, and apps generated from the |
| 396 | 396 | same template declare the same ports. `validateDeploymentConfig` only ever sees |
| 397 | 397 | one project's `sites`, so a second project attaching to an occupied box passes |
| 398 | validation and then fails at `systemctl start` with a bind error naming neither | |
| 399 | side. | |
| 398 | validation. | |
| 399 | ||
| 400 | What happens next is worse than a failure. ts-cloud's units do not set exclusive | |
| 401 | binding, so both listeners bind and the kernel load-balances between them: nothing | |
| 402 | errors, both services look healthy, and each domain answers with the other | |
| 403 | project's site for roughly half its requests. `buddy deploy` catches this late via | |
| 404 | `assertPortsAreFree`, which compares wanted ports against the box's live listeners | |
| 405 | and stops the deploy, but its only remedy is to tell you to pick free ports by | |
| 406 | hand. | |
| 400 | 407 | |
| 401 | 408 | The fragments already answer this, since every one of them records its upstream |
| 402 | 409 | port. `site-ports.ts` reads them: |
| @@ -9,8 +9,22 @@ import { resolveSiteKind } from './site-target' | ||
| 9 | 9 | * That composes cleanly for ROUTES, because routes are keyed by host. It does |
| 10 | 10 | * not compose for PORTS, because every app generated from the same template |
| 11 | 11 | * declares the same loopback ports, and `validateDeploymentConfig` only ever |
| 12 | * sees one project's `sites`. The second attach therefore passes validation and | |
| 13 | * then fails at `systemctl start` with a bind error, naming neither culprit. | |
| 12 | * sees one project's `sites`. | |
| 13 | * | |
| 14 | * The second attach therefore passes validation, and then does something worse | |
| 15 | * than failing: ts-cloud's units do not set exclusive binding, so both listeners | |
| 16 | * bind and the kernel load-balances between them. Nothing errors, both services | |
| 17 | * look healthy, and each domain answers with the other project's site for about | |
| 18 | * half its requests. That is not hypothetical - it is what happened to | |
| 19 | * predicthq.org when a storefront picked a port by reading other projects' | |
| 20 | * config files rather than the box (see `assertPortsAreFree` in stacks). | |
| 21 | * | |
| 22 | * `assertPortsAreFree` catches this late, from the deploying box over SSH, by | |
| 23 | * comparing wanted ports against live listeners. That is the better evidence and | |
| 24 | * it stays the last line of defence. What it cannot do is avoid the clash: it | |
| 25 | * exits and tells the operator to go pick free ports by hand. This module is the | |
| 26 | * other half - deciding the ports before anything is shipped, from data a plan | |
| 27 | * already has. | |
| 14 | 28 | * |
| 15 | 29 | * The fix does not need new bookkeeping on the box. The fragments already record |
| 16 | 30 | * every upstream a project serves, so the host is its own port registry - it was |
| @@ -131,9 +131,11 @@ export interface ValidateDeploymentOptions { | ||
| 131 | 131 | * |
| 132 | 132 | * Validation is otherwise blind to co-tenants: it only ever sees one project's |
| 133 | 133 | * `sites`, so two apps attached to the same server can both declare the |
| 134 | * template's default port, both pass, and the second one fail at | |
| 135 | * `systemctl start` naming neither owner. Supplying this turns that into a | |
| 136 | * plan-time error that names the project holding the port. | |
| 134 | * template's default port and both pass. The box does not reject the second | |
| 135 | * one either - ts-cloud's units do not set exclusive binding, so both bind and | |
| 136 | * the kernel load-balances, leaving each domain serving the other project's | |
| 137 | * site about half the time with nothing logged as an error. Supplying this | |
| 138 | * turns that into a plan-time error naming the project holding the port. | |
| 137 | 139 | * |
| 138 | 140 | * Build it with `occupiedHostPorts()` from `./site-ports`, passing the |
| 139 | 141 | * deploying project's own slug as `ignoreSlug` - its fragment is already on the |