Wait for the load balancer address instead of failing on the first read - #53
Merged
Conversation
The create path read the ingress address once, immediately after the rollout it waits for, and gave up if it was not there yet - "jq: error ... Cannot iterate over null" followed by "failed". The address is published by the ingress controller on a separate cycle, so that read is usually too early: the job failed and was retried until a later attempt happened to catch it. The Red Pepper MCP deploy failed three times this way in dev and once in live. Measured on both clusters by creating an ingress with a fresh hostname and timing the address: NonLive n=6 min 19.1s median 51.8s max 56.0s Live n=4 min 38.3s median 51.5s max 55.4s The hard ceiling near 56s with scattered minima is one publication cycle - the job arrives at a random point in it. ADDRESS_TIMEOUT therefore defaults to 120s, two cycles, so a single missed one is survivable; ADDRESS_POLL_SECONDS defaults to 5. On timeout the object's events are printed and the job exits 1. There is no state to check instead: IngressStatus carries only loadBalancer.ingress[] and no conditions, so a pending address and a permanently broken one are indistinguishable except by waiting. Also collapses the duplicated ingress/service and ip/hostname branches, which had four copies of the same fetch-and-check. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR addresses flaky create runs by waiting for Kubernetes to publish the external load balancer address (IP/hostname) instead of reading it once immediately after rollout and failing when the address is not yet present.
Changes:
- Add a bounded polling loop (with
ADDRESS_TIMEOUTandADDRESS_POLL_SECONDS) to wait for the ingress/service external address before writing the DNS record. - Reduce duplicated branches by unifying ingress vs service selection and IP vs hostname selection.
- Document the new waiting behavior and configuration knobs in the README, and bump the script version to
v0.0.25.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| readme.md | Documents the new bounded wait for ingress/service external address and the related environment variables. |
| k8s-tools.sh | Implements polling for the external address with configurable timeout/interval; bumps version string. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+99
to
+103
| resource=$(kubectl --namespace=$namespace get $kind $target --output=json 2>/dev/null) | ||
| if [ -n "$resource" ]; then | ||
| dns_record_value=$(echo "$resource" | jq -r ".status.loadBalancer.ingress[0].$field // empty") | ||
| [ -n "$dns_record_value" ] && break | ||
| fi |
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
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.
The problem
createreads the ingress address once, immediately after the rollout it waits for, and gives up if it is not there yet:The address is published by the ingress controller on its own cycle, independently of — and later than — the deployment rollout. So that read is usually too early. The job fails, Kubernetes retries it, and a later attempt eventually catches it. The Red Pepper MCP deploy hit this three times in dev and once in live; it only ever looked like flakiness.
How long it actually takes
Measured on both clusters by creating an ingress with a fresh hostname and timing until
.status.loadBalancer.ingress[0].ipappeared:The scattered minima against a hard ceiling near 56s is the signature of a single publication cycle — the job arrives at a random point in it, not variable load. Both clusters behave identically.
The fix
A bounded wait, polling until the address appears.
ADDRESS_TIMEOUTdefaults to 120s andADDRESS_POLL_SECONDSto 5s.120s is deliberately more than 1.5× the measurement (~84s). Because the mechanism is a discrete ~55s cycle rather than a continuous distribution, ~84s covers barely more than one cycle and would fail outright if a single publication were missed — a controller restart or leader-election handover. 120s survives that. The asymmetry favours it: too generous only delays reporting a genuine failure, too tight reverts to spurious deploy failures. Both are overridable.
On failing fast instead
There is nothing to check.
IngressStatusis only:No conditions, no phase. A pending address and a permanently broken one are byte-identical — both an empty
loadBalancer. (ports[].errorexists but ingress-nginx never populates it.) So the timeout prints the object's events, where a real failure does show up, and exits 1.Verification
Record-matching regression matrix from #52, all unchanged:
Also collapses the duplicated ingress/service and ip/hostname branches, which held four copies of the same fetch-and-check.
Rollout
Needs
0.0.25published, then a chart bump. k8s#126 currently pins0.0.24— if that has not merged yet it is cheaper to retarget it at0.0.25and ship both in one bump.🤖 Generated with Claude Code