Skip to content

fix(stackresource): report PortNotListening on workloads that never serve - #61

Merged
ashishmax31 merged 1 commit into
mainfrom
fix/port-check-unify-failed-verdict
Aug 4, 2026
Merged

fix(stackresource): report PortNotListening on workloads that never serve#61
ashishmax31 merged 1 commit into
mainfrom
fix/port-check-unify-failed-verdict

Conversation

@ashishmax31

Copy link
Copy Markdown
Contributor

The bug

A container listening on the wrong port (or on 127.0.0.1 instead of 0.0.0.0) never turns Ready. No further workload events arrive, so nothing wakes the reconcile again.

main handled that case by capturing the port diagnosis into LastFailureDetails and stopping. It never started the grace clock, never filed a verdict, and never requeued — so the StackResource sat in Phase=Pending forever with the reason buried in status. The serving path did report a failure, but only for a dead secondary port, and never re-checked afterwards.

The change

Both paths now go through one checkPorts, so a declared port nobody listens on reaches the same terminal verdict either way.

  • One grace clock. It runs from the first refusal by a running pod on the revision, serving or not. FailingSince was previously serving-only.
  • The reconcile requeues itself while the clock runs, because a refused port produces no Kubernetes event that would bring it back. Without this the clock could never expire.
  • A failed serving workload keeps being re-dialed, so an app that binds its port late recovers without needing a new revision. The wait is the length of the outage so far, which doubles it on each retry between 1m and 10m — no attempt counter needed on status, and it survives an operator restart.
  • Unchanged: a crash detail still outranks a port verdict, dialing still never happens inline, and a serving workload with a dead secondary port still keeps Available=True (ServingButStalled).

Resulting status: Phase=Failed, Stalled=True, reason PortNotListening, with a message naming the dead port.

Alongside it

verifyPorts now writes only Status.PortCheck; checkPorts is the only writer of LastFailureDetails on the port path. The failure message is rendered from status rather than from the dial, so a repeated verdict reads identically to a fresh one. The four duplicated caller blocks in deployment.go / statefulset.go collapsed to one line each.

status_derive.go is restructured for readability only — each verdict kind gets a named function and the stale-generation lookup is shared. No derivation behaviour changes; existing tests cover it untouched.

Review notes

A code review of this branch raised one real defect, now fixed: checkPorts discarded the "a dial is in flight" signal, so the recovery poll ran at half the intended rate and every answer it read was a full interval stale. The existing test had encoded that behaviour, and was corrected.

Two items were assessed and deliberately left:

  • A crash detail latches per revision. If a container crashes once on a revision then starts clean but binds the wrong port, the crash detail is latched and the port failure is never reported. Pre-existing on main; called out as a known limitation rather than fixed here.
  • A successful verdict stays cached for the revision, so a proven-good workload stops being re-dialed. A port that dies after proving itself open, within the same revision and the same operator process, is not re-detected. The alternative puts every healthy StackResource into a permanent dial loop.

portCheckRequeueInterval is 30s rather than 10s: six samples inside a three-minute window is enough to decide something only acted on at the end of it, and the dials go against user pods.

Test plan

  • make test (codegen, fmt, vet, unit suite with envtest)
  • New unit specs: the not-serving StatefulSet verdict, the poll alternating between a short pass and the slow poll, the detail clearing when ports recover, and portRetryAfter's doubling and cap
  • make test-integration — includes a new spec asserting Phase=Failed / Stalled=True / PortNotListening for a workload that never serves. Not run locally; needs Docker and ~11 minutes.

…erve

A container listening on the wrong port never turns Ready, so no further
workload events arrive and nothing wakes the reconcile again. The
not-serving path captured the port diagnosis into LastFailureDetails and
then stopped: it never started the grace clock, never filed a verdict and
never requeued, so the StackResource sat in Phase=Pending forever with the
reason buried in status.

Unify both paths behind checkPorts, so a declared port nobody listens on
reaches the same terminal verdict whether or not the workload serves:

- One grace clock for both. The clock runs on the first refusal from a
  running pod on the revision, serving or not.
- The reconcile requeues itself while the clock runs, because a refused
  port produces no Kubernetes event that would otherwise bring it back.
- A failed serving workload keeps being re-dialed, so an app that binds
  its port late recovers without a new revision. The wait is the length of
  the outage so far, which doubles it on every retry between 1m and 10m.

Alongside the behaviour change, verifyPorts now writes only Status.PortCheck
and checkPorts is the only writer of LastFailureDetails on the port path, so
the verdict has a single owner. A crash detail still outranks a port verdict
at every call site.

status_derive.go is restructured only for readability: each verdict kind
gets a named function and the stale-generation condition lookup is shared.
No derivation behaviour changes.
@ashishmax31
ashishmax31 merged commit b472b45 into main Aug 4, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant