mirror of
https://github.com/mudler/LocalAI.git
synced 2026-09-12 22:33:54 -04:00
Ettore Di Giacinto
3338d7bc56
Review round 1 on the change that stopped workers listening. One blocking item
and seven notes.
LOCALAI_WORKER_TUNNEL=false was the blocking one, and the ruling was to make it
fatal rather than to correct the comment that still promised it fell back to the
advertised address. There is no fallback left: a worker on this branch
advertises nothing and binds only loopback, so turning the tunnel off leaves it
reachable by nothing while it registers, heartbeats and reports healthy, and the
scheduler keeps placing models on it. That is the worst available failure shape,
so a new Config.validateStartup refuses it before prefetch, registration and
NATS, while the worker is still invisible to the cluster. It absorbs the
pre-existing empty-registration-token check, which had the same shape and no
spec. The flag is kept rather than deleted so an operator who set it is told the
promise is gone instead of having the setting ignored, and the guard around
StartTunnel is removed, because a branch nothing can take reads as a supported
no-tunnel mode that does not exist.
The justification for erroring on an install that names no address was wrong,
and the review is right that this is the dangerous form of overclaiming, because
the conclusion holds and the mechanism does not. It said the resulting empty
target would be refused as an invalid stream and that the refusal would read as
the worker answering about its backend. Nothing in this repo branches on
cluster.ErrNoRoute, and nodes.unroutable treats any recorded dial error as
unroutable, so that refusal reaches every reap guard as ProbeUnknown and deletes
nothing. The site now stands on what holds, that an install naming no port
produced nothing routable and the failure belongs to the install rather than to
a later probe, and records the retracted claim so nobody re-derives it. This
retracts the same paragraph in the body of 1cf847f29.
The reviewer deleted the whole tryWarmPath unnamed-replica guard and the suite
stayed green, including the reservation release. It is specced now, and the
asymmetry the review asked about is decided at the site: the row stays, unlike
the sibling !alive branch which removes it. That branch has observed a backend
dead; this one has observed only that the row is unreadable, which says nothing
about whether a process is running, and the row is the last record that one
might be, since the acknowledged stop path refuses a stop whose ExpectedAddress
does not match and an empty one cannot be cleaned up through it either.
The cross-version wire claim rested on two struct tags nobody asserted:
renaming only the json keys survived mutation while the gorm column rename went
red through raw SQL. Both keys are pinned now, marshal and unmarshal, per
struct.
A worker-first upgrade showed the operator a status code and not the reason. The
registration client discarded the body, so "address is required for backend
workers" was read off the socket and thrown away, and the ladder then spent four
minutes on a verdict the frontend reached instantly. Refusals now quote the body
and carry ErrRegistrationRejected, and both the ladder and the credential
manager's Acquire stop on the first one. Acquire matters more than the ladder:
it is the default path and its bound is 100 attempts, not 10. 408 and 429 are
deliberately not refusals, since both are the frontend asking for the same
request again.
Also: the stale "not blocked by firewalls" troubleshooting line, which now names
the real cause and the knobs that move the port range; and the inert address
fields on the MCP Node DTO, which the Assistant was still being handed. The
review named http_address there and I removed address too, because it is inert
by the same argument and leaving one of a pair is arbitrary.
Five mutations, all red. Deleting the warm-path guard reddens four specs and
falsifying only its reservation release reddens one, so the two halves are
pinned separately. Renaming only the json keys reddens both wire suites.
Discarding the refusal body reddens two. Dropping the rejection classification
does not fail the suite, it hangs it, which is the operator-visible symptom, so
it is recorded red under a ginkgo timeout.
The verify list is now derived from the diff rather than from the brief, which
is what let the previous round ship a spec asserting 200 where the endpoint
returns 201: nine ginkgo suites, the e2e vet, route auth coverage, the leaf
check, build, the healthcheck shell suite and lint. The two jsx files have no
harness in this worktree and are recorded as the one unverified surface.
Assisted-by: Claude Opus 5 [claude-code]
Signed-off-by: Ettore Di Giacinto <mudler@localai.io>