Skip to content

fix(nginx): re-resolve upstreams so a container restart cannot break ingest - #4492

Closed
spawnia wants to merge 2 commits into
getsentry:masterfrom
mll-lab:nginx-resolve-upstreams
Closed

fix(nginx): re-resolve upstreams so a container restart cannot break ingest#4492
spawnia wants to merge 2 commits into
getsentry:masterfrom
mll-lab:nginx-resolve-upstreams

Conversation

@spawnia

@spawnia spawnia commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Supersedes #4295, keeping @ilias-115 as co-author. Same fix as #3894 asked for, rescoped around the objections raised there and in #4295.

Why

nginx resolves the service names inside upstream { server ... } exactly once, at startup, and caches the answer for the life of the worker process. Docker does not guarantee a container the same address across starts. So after any restart nginx can be proxying to an address that now belongs to a different container — and it will keep doing that forever, because nothing in nginx ever looks again.

The failure is quiet, which is the part that hurts. web and relay are separate upstreams, so the UI stays fully functional while every ingest request returns 502. We lost roughly three hours of events to this before a human noticed; no health check on our side could have caught it, because nginx itself was healthy.

#3914 helps but does not close it. It restarts nginx when Compose restarts relay or web, which covers docker compose restart and not much else. Our incident began after a host-level Docker restart, where nginx came up 12 s before relay despite depends_on — nginx cached the address of whatever container held it at that moment. condition: service_healthy is not available either, since relay declares no healthcheck.

The DNS cost, since that was the sticking point

The concern in #3894 was "a DNS lookup every 5 seconds is not a good idea, especially under heavy load". That framing does not apply to resolve: re-resolution is driven by a timer per upstream server, never by a request.

With two upstream servers and valid=10s the steady state is 0.2 queries/s, total, forever — the same at 1 req/s as at 10k req/s. Each one goes to the container engine's own DNS on the same host, so there is no network hop and no external resolver involved. Requests never wait on it: nginx keeps serving from the cached address while the timer refreshes in the background.

valid=10s rather than 30s or 60s because it bounds the outage, not the load. The load is the same at any value; only the worst-case window during which nginx still points at a dead address changes.

Where the resolver address comes from

resolve needs a resolver, and only the container engine's own DNS knows the names relay and web at all — a resolver from the host's /etc/resolv.conf cannot resolve them. Its address is not the same everywhere: 127.0.0.11 on Docker, the network gateway on Podman, which this repo also supports.

So nothing is hardcoded. The nginx image already derives the address from the container's /etc/resolv.conf in /docker-entrypoint.d/15-local-resolvers.envsh and exports it as NGINX_LOCAL_RESOLVERS, but that only reaches the config through envsubst. nginx.conf is therefore mounted at /etc/nginx/templates/nginx.conf.template and rendered over the image default via NGINX_ENVSUBST_OUTPUT_DIR. NGINX_ENVSUBST_FILTER restricts substitution to that single variable, so the nginx runtime variables in the config ($remote_addr, $request_id, and the rest) are left alone.

The only user-visible consequence is the bind mount target. Anyone who mounts their own nginx.conf over /etc/nginx/nginx.conf keeps working exactly as before — the template is simply not rendered for them, and they supply their own resolver.

No image bump needed

resolve in nginx OSS landed in 1.27.3, and docker-compose.yml currently pins nginx:1.31.4-alpine, so this applies to master as-is. It is worth noting for anyone backporting: on the 1.25.4 that older release tags carry, nginx refuses to start with this config.

Test plan

That the template renders to the engine's actual DNS address, that the nginx runtime variables survive envsubst, and that the result is a valid config:

docker network create resolver-test
docker run --rm --network=resolver-test \
  --add-host=relay:127.0.0.1 --add-host=web:127.0.0.1 \
  --env NGINX_ENTRYPOINT_LOCAL_RESOLVERS=1 \
  --env NGINX_ENVSUBST_OUTPUT_DIR=/etc/nginx \
  --env 'NGINX_ENVSUBST_FILTER=^NGINX_LOCAL_RESOLVERS$' \
  --volume="$PWD/nginx.conf:/etc/nginx/templates/nginx.conf.template:ro" \
  --entrypoint=/bin/sh nginx:1.31.4-alpine -c '
    export NGINX_ENTRYPOINT_LOCAL_RESOLVERS=1
    . /docker-entrypoint.d/15-local-resolvers.envsh
    /docker-entrypoint.d/20-envsubst-on-templates.sh
    grep -n "resolver\|remote_addr\|request_id" /etc/nginx/nginx.conf
    nginx -t'
docker network rm resolver-test

On a user-defined bridge network that prints resolver 127.0.0.11 valid=10s ipv6=off;, leaves $remote_addr and $request_id untouched, and the config test passes. I have no Podman host to try, so the gateway case rests on 15-local-resolvers.envsh reading the same /etc/resolv.conf nginx would have used anyway — a Podman run in CI would settle it.

The version floor is real, worth noting for anyone backporting:

docker run --rm --add-host=relay:127.0.0.1 --add-host=web:127.0.0.1 \
  --volume="$PWD/nginx.conf:/etc/nginx/nginx.conf:ro" \
  nginx:1.25.4-alpine nginx -t   # [emerg] invalid parameter "resolve"

Then the behaviour that matters — force relay to a new address and confirm ingest survives without touching nginx:

docker compose up -d
docker compose rm --stop --force relay && docker compose up -d relay
sleep 15
# events keep arriving; before this change every ingest request 502s from here on

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

…ingest

nginx resolves the service names in an upstream block once at startup and
keeps the address for the life of the worker process. Docker does not
guarantee a container the same address across starts, so any restart can
leave nginx proxying to a container that no longer owns the old address.
Ingest then answers 502 on every request while the web UI keeps working,
and nothing recovers until someone restarts nginx by hand.

The "resolve" parameter makes nginx re-resolve per DNS TTL, bounded by
"valid". That is two lookups per 10s against Docker's in-process embedded
DNS regardless of request volume, so the cost does not scale with load.

Co-authored-by: Ilias Aaguida <ilias.aaguida@beta.gouv.fr>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Want reviews to match your repository better? Bugbot Learning can learn team-specific rules from PR activity. A team admin can enable Learning in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 068b91e. Configure here.

Comment thread nginx.conf Outdated
Pinning 127.0.0.11 assumed Docker's embedded DNS. Podman answers on the
network gateway instead, so "resolve" would have failed every lookup there
and nginx would have returned 502 for every request.

The nginx image already derives the address from /etc/resolv.conf into
NGINX_LOCAL_RESOLVERS, which reaches the config through envsubst only, so
nginx.conf is now mounted as a template and rendered over the image default.
NGINX_ENVSUBST_FILTER limits substitution to that one variable so the nginx
runtime variables in the config survive.
@spawnia

spawnia commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Deployed this on our self-hosted instance (Sentry 24.12.2, ~57 containers, nginx:1.30.4-alpine).

Verified the rendered config inside the container:

resolver 127.0.0.11 valid=10s ipv6=off;
server relay:3000 resolve;
server web:9000 resolve;

Then reproduced the original failure scenario: docker compose rm --stop --force relay && docker compose up -d relay. Relay came back on a new address and ingest kept working — a test event arrived without touching nginx, and nginx logged zero 502s.

Before this change, that exact sequence made every ingest request return 502 while the UI stayed fully functional, which cost us ~3 hours of events.

@aminvakil aminvakil closed this Aug 27, 2026
@spawnia

spawnia commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

Hey @aminvakil, would you mind sharing why you closed this? I put decent effort into this, and I believe it fixes a real problem. Would love to be able to get closer to upstream and not need to keep local patches around.

@aminvakil

Copy link
Copy Markdown
Collaborator

I've told this a thousand times, this is too much change for a specific problem which happens in very specific conditions, there is no easy way to fix this, but this fix is not correct.

I would not definitely agree with merging a config change which has 9 documentation lines above that.

	# Without "resolve", nginx looks the service names up once at startup and keeps that address
	# for the life of the worker process. Docker does not guarantee a container the same address
	# across starts, so nginx can end up proxying to a container that no longer owns it and
	# answer every request with 502 until someone restarts nginx by hand.
	# "resolve" needs a resolver, and the container engine's own DNS is the only thing on a Compose
	# network that knows the service names. Its address differs per engine -- 127.0.0.11 on Docker,
	# the network gateway on Podman -- so it is read from /etc/resolv.conf at startup by
	# /docker-entrypoint.d/15-local-resolvers.envsh, which is why this file is a template.
	# There are no AAAA records worth waiting for on a Compose network.
	resolver ${NGINX_LOCAL_RESOLVERS} valid=10s ipv6=off;

What is this seriously?

@aminvakil

aminvakil commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Edit: This is just a joke, do not read it.

Thank you for putting this together. I have now reviewed the change from several angles, including the nginx angle, the Docker angle, the Podman angle, the DNS angle, the YAML angle, the shared-memory angle, the temporal-container-identity angle, and, perhaps most importantly, the angle from which a configuration file begins to question whether it is still a configuration file or has quietly become an orchestration framework with opinions about municipal zoning.

At first glance, the proposed behavior sounds straightforward: nginx resolves an upstream hostname, a container restarts, the container may receive another address, and nginx should eventually become aware of that address without requiring a human to restart nginx. However, “straightforward” is often only a temporary visual artifact caused by observing a system from sufficiently far away. Once we zoom in, we discover DNS, shared memory, entrypoint scripts, environment substitution, Compose interpolation, multiple container engines, IPv4, IPv6, resolver validity windows, upstream keepalive pools, page sizes, comments, and the philosophical question of whether a restarted container is the same container or merely another container carrying the previous container’s service name.

Before considering the implementation itself, I think we need to clarify what “resolve” means in this context. It may mean:

  1. Resolve a hostname into an IP address.
  2. Resolve an operational incident.
  3. Resolve a disagreement regarding whether resolving should occur.
  4. Resolve nginx’s inability to resolve the thing that resolves the resolver.
  5. Re-resolve a previously resolved resolution whose resolved result is no longer sufficiently resolved.

The PR primarily addresses the first meaning, while implicitly attempting the second, but the configuration introduces enough machinery that we may accidentally arrive at the fourth before reaching the fifth. This is not necessarily wrong, but it should be acknowledged because otherwise a future reader may assume the resolver merely resolves, when in reality it participates in a broader resolution lifecycle involving Docker, Podman, /etc/resolv.conf, envsubst, and at least two dollar signs.

Regarding the comments above the resolver directive: the ratio between explanatory comments and executable configuration is currently approximately nine-to-one. This creates a substantial semantic mass around a single directive. While comments are free at runtime, they are not free in the cognitive economy of the repository. Every comment must be parsed by a human resolver, cached for an unspecified validity period, and potentially invalidated after a maintainer restart. Unlike Docker DNS, human DNS does not reliably answer on 127.0.0.11, and its negative-cache behavior varies considerably depending on coffee availability.

The following block is particularly significant:

resolver ${NGINX_LOCAL_RESOLVERS} valid=10s ipv6=off;

There are several independent dimensions here.

First, NGINX_LOCAL_RESOLVERS is plural. This raises the question of whether the value is always plural, sometimes plural, conceptually plural, or merely future-plural. If there is only one resolver, we are placing a singular value in a plural variable. This is legal, but semantically asymmetrical. If there are multiple resolvers, nginx receives multiple addresses, but then which resolver resolves the resolver disagreement if the resolvers return different resolutions? Is the first resolver primary? Are they consulted in round-robin order? Does the round robin itself require a shared-memory zone? We should avoid accidentally creating a resolver upstream for the upstream resolver unless that resolver upstream also has a resolver, at which point the configuration could become recursive in a way that is technically elegant but operationally indistinguishable from a spiral.

Second, the value is generated by an entrypoint script rather than being intrinsically known to nginx. This means nginx does not know its resolver; it knows a textual representation of what another shell script believed /etc/resolv.conf said at container startup. envsubst then acts as an intermediary. It does not understand DNS, nginx, Docker, Podman, or Sentry. It simply sees a dollar sign and becomes enthusiastic. The filter is therefore required to prevent it from enthusiastically substituting nginx’s own runtime variables into emptiness.

This introduces the following chain:

  1. The container engine generates /etc/resolv.conf.
  2. The nginx image reads /etc/resolv.conf.
  3. The nginx image exports NGINX_LOCAL_RESOLVERS.
  4. Compose provides a filter describing which exported value may be substituted.
  5. envsubst reads the template.
  6. envsubst writes an nginx configuration.
  7. nginx reads that configuration.
  8. nginx asks the resolver identified by the configuration to resolve the upstream.
  9. The resolver returns the container address managed by the engine that generated /etc/resolv.conf in step one.
  10. Ten seconds later, everybody checks whether reality has changed.

This is circular, although not necessarily incorrectly circular. Wheels are circular and generally regarded as useful. Infinite recursion is also circular and is regarded less favorably. We should establish whether this is wheel-circular or recursion-circular before merging.

Third, valid=10s introduces a decimal commitment. Why ten seconds? Ten is attractive because humans have ten fingers, but nginx has no fingers. Docker also has no fingers, although it may have file descriptors. A binary-oriented system might prefer eight seconds, sixteen seconds, or 10.24 seconds. A calendar-oriented system might prefer twelve seconds. A DNS-oriented system might prefer the TTL. A maintainer-oriented system might prefer “whenever the incident happens, but approximately five seconds before the alert fires.”

The PR explains that ten seconds bounds the outage window, but technically it bounds one particular dimension of one possible outage window, assuming the resolver is available, the answer is current, the container has started, the port is listening, the network namespace exists, the upstream zone has enough memory, the worker processing the timer is alive, and time itself continues advancing monotonically. This is probably acceptable, but the number should not acquire an authority greater than the assumptions beneath it.

Fourth, ipv6=off is an explicit statement that six is not currently invited. This may be reasonable for the default Compose network, but “off” is a strong word. Could IPv6 instead be “discouraged,” “not preferred,” “temporarily excused,” or “available by appointment”? Disabling it entirely risks establishing a precedent under which future protocol versions are judged numerically. If IPv7 is ever created, would it inherit IPv6’s disabled status, or would it require its own directive? We should avoid encoding inter-protocol family disputes into an nginx configuration without an architectural decision record.

The upstream zones introduce another category of concern:

zone relay 64k;
zone sentry 64k;

The use of the word “zone” suggests geographic or municipal boundaries, but the zone is actually shared memory. This discrepancy should be considered carefully. An operator reading “zone relay” may reasonably expect a designated area in which relaying is permitted. Instead, it allocates memory for upstream state. No zoning map is supplied, no planning permission is requested, and there is no public consultation period.

The 64k size is also suspiciously specific. It is neither obviously derived from the number of upstreams nor from the number of possible resolver answers. Does the zone contain exactly 65,536 units of relay? What happens when it contains 65,537? Is the final relay discarded, evicted, compressed, or rezoned as mixed-use memory?

More concretely, nginx requires upstream zones to be at least eight system pages. Therefore, 64k works on systems with 4 KiB or 8 KiB pages but fails on systems with 16 KiB or 64 KiB pages. On such hosts, including some ARM64 environments, nginx rejects the configuration with a “zone is too small” error and does not start. This means the zoning issue is not entirely metaphorical. If we are going to establish these zones, they should be large enough to comply with the strictest applicable building code. A value of at least 512k would cover 64 KiB pages, though I would also like confirmation that the extra 448 KiB will not attract speculative real-estate investment.

The mount transition also deserves extended consideration:

target: /etc/nginx/templates/nginx.conf.template

Previously, the file was mounted as the nginx configuration. It is now mounted as a template from which the nginx configuration is produced. This changes the ontological status of nginx.conf: on the host it is named like a configuration, inside the container it is treated as a template, and after substitution another file becomes the actual configuration while retaining the original name.

In other words, nginx.conf is a configuration outside the container, a template inside one directory, and a configuration again after being copied into another directory. This resembles a caterpillar becoming a butterfly and then being renamed caterpillar.conf for compatibility.

There is also a concrete compatibility issue. Compose merges volume entries by their target path. Existing override files that mount a custom configuration at /etc/nginx/nginx.conf will no longer replace the base mount because the base mount now targets /etc/nginx/templates/nginx.conf.template. Both mounts will remain active. The entrypoint will then attempt to render the template into /etc/nginx/nginx.conf.

If the custom mount is read-only, the entrypoint fails while trying to write to it, and nginx does not start. If it is writable, the rendering process may overwrite the custom configuration. We therefore have two modes:

  • Safe custom configuration, broken startup.
  • Successful startup, potentially erased custom configuration.

This is a refreshingly balanced failure model, but probably not the balance users expect.

The environment variable filter introduces additional punctuation concerns:

NGINX_ENVSUBST_FILTER: "^NGINX_LOCAL_RESOLVERS$$"

There are two dollar signs because one dollar sign must survive Compose so that another component can understand that the first dollar sign was intended as a regex end anchor rather than as an invitation to interpolate a Compose variable. This is technically explainable, but it means the reader must simultaneously understand YAML quoting, Compose interpolation, shell environment semantics, awk regular expressions, and envsubst variable selection.

A single line therefore spans at least five interpretation layers:

  1. YAML sees a string.
  2. Compose sees an escaped dollar sign.
  3. The container environment sees a regular expression.
  4. awk sees an end anchor.
  5. envsubst sees a list of permitted substitutions.
  6. A future maintainer sees two dollar signs and wonders whether somebody made a typo.

I realize that this list contains six layers after promising five. This demonstrates the core problem: once substitution begins, the layer count itself is not stable.

The proposed test plan is directionally useful, but I believe it should be expanded to cover several additional scenarios:

  • Docker while Docker is running.
  • Docker immediately after Docker was not running.
  • Podman while pretending to be Docker.
  • Docker while pretending not to be Docker.
  • One resolver.
  • Multiple resolvers.
  • No resolver, but considerable optimism.
  • IPv4-only networking.
  • IPv6-only networking.
  • Dual-stack networking in which IPv6 has been switched off and takes this personally.
  • 4 KiB pages.
  • 16 KiB pages.
  • 64 KiB pages.
  • A container that restarts with the same address.
  • A container that restarts with a different address.
  • A container that does not restart but nevertheless develops a new perspective.
  • A resolver response arriving just before the ten-second validity boundary.
  • A resolver response arriving just after the boundary.
  • A resolver response arriving exactly on the boundary, which may require relativistic treatment.
  • An existing read-only custom nginx configuration.
  • An existing writable custom nginx configuration.
  • An nginx configuration mounted read-only but emotionally writable.
  • A service named web.
  • A service named relay.
  • A service named resolver, to ensure no naming-based feedback loop.
  • A host with ten CPU cores, to verify that the ten-second interval is not interpreted as one second per core.
  • A leap second, if one can be scheduled conveniently.

I would also like the DNS cost described in units beyond queries per second. For example:

  • Queries per container restart.
  • Queries per successful ingest.
  • Queries per failed ingest.
  • Queries per maintainer comment.
  • Queries per nine-line explanatory block.
  • Queries per cubic meter of shared-memory zone.

This is not because those units are operationally meaningful, but because the current explanation has already established a high standard of numerical specificity, and consistency requires us to continue until the numbers stop helping.

At a broader architectural level, this PR transfers responsibility for upstream identity from nginx startup to a periodically refreshed DNS answer. That is likely the correct mechanism, but responsibility transfers should be explicit. Previously nginx said, “I resolved this once, and therefore it shall remain true.” After this change nginx says, “I resolved this recently, and therefore it is probably still true for up to ten seconds.” This is epistemically healthier but introduces uncertainty into a system that previously expressed confidence even when wrong.

We should decide whether nginx’s goal is certainty, correctness, recency, availability, or merely avoiding 502 responses long enough for a human to finish lunch. These goals overlap but are not identical.

My current conclusion is that the underlying incident is real, the stale-address behavior is real, dynamic resolution is a legitimate nginx mechanism, and the implementation is conceptually reasonable. However, the change also introduces a templating pipeline, changes a public mount target, depends on image entrypoint behavior, allocates page-size-sensitive shared-memory zones, disables IPv6 resolution, introduces a ten-second temporal policy, and requires two dollar signs to communicate one dollar sign through multiple interpretive layers.

Before merging, I would therefore request the following:

  1. Increase both upstream zones to a size that works on supported large-page ARM64 systems.
  2. Preserve or explicitly migrate existing /etc/nginx/nginx.conf volume overrides.
  3. Confirm that a read-only custom config cannot cause the entrypoint to fail unexpectedly.
  4. Confirm that a writable custom config cannot be overwritten unexpectedly.
  5. Test Podman rather than relying on Podman’s theoretical agreement with /etc/resolv.conf.
  6. Explain whether ipv6=off is a temporary operational decision or a permanent diplomatic sanction.
  7. Confirm that ten seconds remains ten seconds under all supported container engines.
  8. Consider reducing the comment-to-directive ratio, unless comments are now considered an upstream dependency.
  9. Add a brief explanation for the two dollar signs, followed by another explanation clarifying why the first explanation does not itself require escaping.
  10. Provide a zoning diagram for the relay and sentry shared-memory districts.
  11. Verify the configuration during a full moon, because DNS behavior under lunar influence remains conspicuously absent from the test plan.
  12. Re-resolve this review after its valid period expires.

Until those points are addressed, I am not comfortable approving the current version. To be clear, this is not because the resolver fails to resolve the upstream. It is because resolving the upstream requires resolving the resolver, rendering the resolver into a template, preserving the variables that must not be resolved by the renderer, allocating a zone in which the resolved state may be shared, and ensuring that the mechanism used to resolve a stale address does not itself create a stale understanding of where the configuration lives.

In summary: the proposed resolution may resolve the unresolved upstream, but the resolution process has not yet fully resolved the unresolved questions introduced by resolving it.

Please resolve accordingly.

@spawnia

spawnia commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @aminvakil for letting me in on your frustration. I apologize on behalf of my agent, my PR was lazy in form and lacked the polish that busy maintainers deserve. I am now taking the time to work through the actionable suggestions in your admittedly funny review comment and plan to come back with a better and more readable solution. I do care about getting this fixed, and I do care about maintainability and staying in control.

@aminvakil

Copy link
Copy Markdown
Collaborator

Thanks @aminvakil for letting me in on your frustration. I apologize on behalf of my agent, my PR was lazy in form and lacked the polish that busy maintainers deserve. I am now taking the time to work through the actionable suggestions in your admittedly funny review comment and plan to come back with a better and more readable solution. I do care about getting this fixed, and I do care about maintainability and staying in control.

Feel free to push to this branch and reopen this or create another PR, that's ok both.

@aminvakil

Copy link
Copy Markdown
Collaborator

I am now taking the time to work through the actionable suggestions in your admittedly funny review comment and plan to come back with a better and more readable solution.

Sorry, I just realized what you meant, that review comment is a complete joke, yes, there are reasonable findings in it as well, but it's missing the big picture completely which I've stated before.

You see it's just trying to fixing something broken rather than rejecting the whole idea. Ignore that comment please, that's not me.

@spawnia

spawnia commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

I did push mll-lab@4a8ee28, essentially a polished version of the change this PR originally proposed. Mostly just cleans up the syntax though and uses an extra file instead of a more unconventional templating approach.

Feel free to ignore if you think the resolve solution does not belong in this project at all - it is just the most polished version of that idea.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants