Conversation
…y' into oonimeasurements-limits
…anage_quotas https://github.com/idealista/clickhouse_role/blob/main/molecule/default/group_vars/clickhouse_group.yml actually use sha256 password type clickhouse role disregards password_type and only looks at key password_sha256_hex ... fix quotas keys
the difference between the _xml and sql managed user settings is poorly documented and fails open.
…word if type is sha256_hash
note: why was oonifindings missing CLICKHOUSE_URL ?
…sers fix conflict on clickhouse_url for fastpath2 (use data1, keep new user)
…sers merge upstream changes to use data1 by fastpath
these queries can be slow; poll every 5 minutes
add private api targets as blackbox job with interval
Drop countly host from monitoring
main replaced the shared clickhouse_write_url, clickhouse_readonly_url and clickhouse_readonly_test_url parameters with one per service (b7baff6). The blue/green service secrets are built from the same parameters as each service's ECS task_secrets, so they now point at the per-service ones again: ooniprobe, oonirun and oonimeasurements in dev, oonirun in prod. dev oonifindings no longer gets a CLICKHOUSE_URL, like its ECS task. Without this, terraform init fails on references to undeclared data sources after merging main.
Review: "We should add a log.info on this" (on run()). Every ssh, scp and aws command now goes through the logger with a timestamp, so the CodeBuild log shows what ran against which host and when, not just the start and end of each host's deploy. Secret values are only ever written to files, never put on a command line, so they don't reach the log.
Review: "We should make sure 20s is enough". It wasn't comparable to what the same images get on ECS: the target groups (ooniapi_service) check /health every 30 s with a 5 s timeout and want 2 passes in a row, so a new task has about a minute to come up. The blue/green check gave 20 s, a single pass sufficed, and curl had no timeout of its own. Now each check has curl --max-time 5, the slot must pass twice in a row, and it has health_check_timeout seconds (default 120, per service) to do so. The time it took is logged, so the deadline can be tuned per service from real deploys; at 2 hosts per deploy it stays well within the deploy job's 20 minute build timeout.
Review: "what happens if nginx -t fails". The new upstream conf was moved into conf.d before nginx -t, and left there when it failed. nginx kept serving its old config, but the next reload on that host (another service's deploy, a certificate renewal) would fail, and a restart would take every service on the host down. Now a failing nginx -t puts back the upstream of the slot that is still serving and checks it again, then aborts the deploy without reloading nginx or moving the active_slot marker. If nginx -t fails even then, the config is broken independently of this deploy, and the error says so. The restore uses the same mv the deploy user's sudoers already allows.
Review: "We should make sure there is some kind of timeout on connections while draining". After a flip, nginx workers from before the reload keep serving their in-flight requests to the old slot, with nothing bounding them but proxy_read_timeout (900 s), and long lived HTTP/2 connections can keep them longer. On ECS the ALB bounds the same thing with its deregistration delay, the AWS default of 300 s here. - ooniapi_gateway sets worker_shutdown_timeout to ooniapi_gateway_drain_timeout (300 s), through modules-enabled/, the only place included in nginx's main context. - The next deploy of a service replaces the slot that is draining, so deploy.py now waits until drain_timeout (300 s, per service) has passed since the previous flip, from the active_slot marker's mtime, before restarting it. A first deploy has no marker and does not wait.
The routes of the API are going to be served by two frontends, the ALB for the services on ECS and the nginx gateway on the Hetzner hosts for those deployed blue/green, and their copies have already drifted: the gateway's hand written locations predate main's X-Protocol-Version rules. routes.yaml becomes the one place they are declared; this change has Terraform read it, the gateway follows. The 18 hand written listener rules become one aws_lb_listener_rule with for_each over routes.yaml, with moved blocks from their old addresses. Planned with fake ARNs (no state), the old and the new code produce the same 18 rules: same priorities, targets, conditions and tags. Applying should only move them in the state. The "hotfix" that made the oonimeasurements and testlists rules optional (count = arn != null ? 1 : 0) is removed rather than carried over. It had stopped working anyway: oonimeasurements_rule_3 lacked the count, so the frontend couldn't be planned without oonimeasurements. Both environments pass every target group, so those two inputs are now required and nullable = false. A missing service fails the plan with "Required variable not set" instead of silently losing its routes. The planned rules are unchanged.
The gateway's locations were a hand written copy of the ALB rules, and had already fallen behind them (main's X-Protocol-Version rules). They are now generated from tf/modules/ooniapi_frontend/routes.yaml, which the ALB rules are generated from too, as are the per service direct-host vhosts. nginx can't follow ALB priorities: it picks an exact match, then the longest prefix. filter_plugins/ooniapi_routes.py turns each ALB pattern into an exact or ^~ prefix location and lists the routes nginx couldn't serve the same way (a wildcard other than a trailing *, the same path twice, a prefix of one service covering a path of another); the role fails on any. A plugin rather than inline Jinja, so the result doesn't depend on how a given ansible-core converts templated strings. Routes marked legacy (ooniprobe_legacy, being retired) are left to the ALB: the gateway doesn't run it, so those paths go to ooniprobe. Rendered with ansible-core 2.21 for all six services plus testlists proxied cross-cloud, the generated config has the same 46 locations, with the same match types and upstreams, as the hand written one.
nginx only allows letters, digits and underscores in a syslog tag, so
nginx -t rejected the gateway vhost ("syslog \"tag\" only allows
alphanumeric characters and underscore", nginx 1.27) and it could never
have been installed. The tag is now ooniapi_gateway.
Nothing caches oonimeasurements or oonirun today: on api.ooni.org every repeated request reaches uvicorn (only the reverseproxy's /api/_/ paths show x-cache-status: HIT), and for oonimeasurements that means ClickHouse, saturated on data2. The gateway now caches GET/HEAD responses of the routes routes.yaml marks cache (oonimeasurements, oonirun), as long as their Cache-Control allows, or 10 s without one, like the legacy ooni-api vhost's apicache. Requests with an Authorization header never use the cache (oonirun links can differ per user), and nginx doesn't store no-cache, private or max-age=0 responses, or any setting a cookie. Two zones, since nginx keeps an entry until it goes unused for inactive, expired or not: lists and aggregations valid for minutes would otherwise push out measurement bodies valid for a day. Sized per host as if it got all the traffic, from data2's query_log and responses of api.ooni.org: - long (raw_measurement, measurement_meta; valid 1 d): at most ~50K distinct bodies a day of ~76 KB (max seen ~160 KB), ~3.8 GB a day: max_size 8g, keys_zone 16m (~130K keys), inactive 1d. - short (everything else cached; valid 10 s to 1 h): ~7.5K requests an hour of up to ~60 KB, ~450 MB an hour if all were distinct: max_size 2g, keys_zone 8m, inactive 1h. The access log now records $upstream_cache_status, to tune these from real hit rates. Cache directives and X-Cache-Status are set per server, since an add_header in a location would drop the server's HSTS headers. Checked with nginx 1.27 and stub upstreams: lists, bodies (long zone) and header-less aggregations miss then hit; Authorization bypasses and isn't stored; no-cache isn't stored; ooniprobe and the default route aren't cached; every response keeps HSTS.
A deploy can change what a service returns, but the gateway would keep serving responses cached from the old slot, for up to a day for measurement bodies. nginx without the commercial purge API can't drop entries, so each service's cache key now starts with a version: the image tag, which deploy.py writes into <service>-upstream.conf as $ooniapi_cache_version_<service>, the same file and the same reload that put the new slot in rotation. From then on the old entries are never looked up and age out of the zones; other services' caches are left alone. (The slot letter wouldn't do: slots alternate, so it would bring back responses from two deploys ago.) When nginx -t rejects the new upstream conf, deploy.py now puts the previous file back as it was, cache version included, rather than rendering it again. The image tag is checked to be a plain tag, since it ends up in an nginx string. The placeholder upstream conf the role seeds before a first deploy defines an empty version. Checked with nginx 1.27: after a deploy of oonimeasurements its cached list missed (new upstream response) then hit again, while oonirun's cached response kept hitting; redeploying the previous tag found that tag's entries again, as long as they were still valid.
A service had to choose between ECS and the Hetzner hosts. Switching one to "blue_green" removes the ECS deploy action but leaves the ECS service running, so the ALB keeps sending api.ooni.org and <service>.prod.ooni.io traffic to tasks that no longer get new versions, until DNS moves to the hosts. "both" deploys the same build artifact to ECS and then, in the same Deploy stage, runs the blue/green deploy to the hosts. The ECS deploy runs first, so the hosts never get a version ECS rejected. Either failing fails the pipeline. The migration can then keep the single DNS cutover of the original design while every service runs in "both": the hosts are proven before the move, and DNS can go back to the ALB, which still serves the current version, if they aren't. "ecs" and "blue_green" plan exactly as before. Planned with fake inputs, one service per mode, and the account id lookup replaced by a constant: the 6 and 13 resources are identical. A "both" service gets the 13 blue/green resources and a Deploy stage with ECS "Deploy" (run order 1) then CodeBuild "DeployBlueGreen" (run order 2). Both actions need their own name and namespace. The pipeline role already allows ecs:* and codebuild:StartBuild on any resource. The per-service comments in dev and prod now mention "both", and no longer say the blue/green deploy uses systemd (it uses Docker Compose since e11b071).
aagbsn
marked this pull request as draft
October 4, 2026 04:43
Terraform Run Output 🤖Format and Style 🖌
|
| Pusher | @aagbsn |
| Action | pull_request |
| Environment | dev |
| Workflow | .github/workflows/check_terraform.yml |
| Last updated | Sun, 04 Oct 2026 04:48:22 GMT |
aagbsn
changed the title
aagbsn
added this pull request to stack #504
October 4, 2026 04:48
aagbsn
marked this pull request as ready for review
October 4, 2026 04:51
Contributor
Author
|
can not remove from stacked PR, and seems to require rebasing the stack rather than merging main into the original PR. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Review comments addressed (one commit each)
deploy.pylogs every ssh/scp/aws command it runs (secrets are only written to files, never logged).curl --max-time 5, two passes in a row,health_check_timeout(default 120 s, per service) instead of a single 20 s check.
nginx -tfails, the live slot's upstream is restored and the deploy aborts without reloading nginx, insteadof leaving a broken conf for the next reload.
worker_shutdown_timeout300 s (like the ALB's deregistration delay), and the next deploywaits for the old slot to finish draining.
Fixes needed after merging
mainmainintroduced (otherwiseterraform initfails).ooniapi_gateway. nginx rejected the old tag, so the vhost could never have beeninstalled.
One source of truth for routes
tf/modules/ooniapi_frontend/routes.yamldeclares every API route.for_each, withmovedblocks. Planned against the old code:the same 18 rules, so applying only moves them in state.
route nginx can't serve the way the ALB does. This also picks up
main's X-Protocol-Version rules, which thehand-written gateway was missing.
Caching in the gateway
routes.yamlmarks for caching (oonimeasurements, oonirun), honoringCache-Control, 10 s when there is none. Skipped for requests with
Authorizationand forno-cache/private/Set-Cookie responses. Two zones, for short-lived and long-lived responses.
the old entries for that service only.
New
deploy_mode = "both"failure in either fails the pipeline.
both, DNS moves once, and the ALB still serves thecurrent version if DNS has to move back.
ecsandblue_greenmodes plan exactly as before.