Answer a loopback control API for the integration to drive #57
Loading…
Reference in a new issue
No description provided.
Delete branch "feat/updater-control-api"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
CCS-UHA-6. The updater now answers HTTP on
127.0.0.1:8787: what firmware iscached, what each node is running, a check now, and an update request per node.
Loopback is the whole of the access control, and the port is a constant in
updater/config.pyrather than a setting.It is a service API, not a Home Assistant private one — with Home Assistant
stopped,
curlgets the same answers. Stdlib only,ThreadingHTTPServer, asthe rest of the service is. This is the loopback control API, separate from
the node-facing file server on 8080 by design: different audience, different
binding.
Routes and shapes, since CCS-UHA-7 and -8 build against them:
GET /api/status—service,firmware_cache(root,images),node_link(connected,nodes_reporting).GET /api/firmware—images, eachenvironment,node_type,node,version,image,size,sha256.GET /api/nodes—nodes, eachnode_type,node,installed_version,latest_version,update_available,latest_image.POST /api/check—checked,registry_reached,cached(what this checkfetched),
failed(what it could not fetch),images(the whole cacheafter it). 409 if a check is already running.
POST /api/nodes/<node>/update— 202 withaccepted,started,reason(
sent,no_link,link_refused),detailand the image.<node>takes0x10or16.installed_versionandupdate_availablearenullwhile nothing has beenheard from a node, rather than false: a controller that has not answered is not
a controller that is up to date.
The CAN side is CCS-UHA-5's and is not built here.
NodeLinkis the seam itfills; until then an update is accepted with
"started": falseand"reason": "no_link". Reading the cache goes throughFirmwareCache, not thetree.
POST /api/checkanswers when the check is done, downloads and all, andnothing bounds how long that takes — the registry timeout is per read. Callers
want a long timeout of their own.
firmware.REFRESH_LOCKkeeps the poll and anAPI check from sharing a part-file; the API refuses rather than queues.
A port that will not bind is logged and left: the firmware cache matters more
than the API, and a service that restart-loops over a busy port stops checking
for updates too.
Tested without a socket, bar one: answers through
ControlApi, HTTP framingthrough a handler fed from memory, and the wiring through a stand-in server.
The exception is a single test that binds an ephemeral port on loopback under
socket_enabled, because nothing else proves the real server answers a realclient. The harness blocks
socket.socketoutright, so starting the service ina test needs the stand-in — hence the autouse fixture in
tests/conftest.pyand
FakeControlServerintests/fakes.py, now shared with the file server's.Two things to fix before this lands:
/api/checkcan race the background poll into a corrupt cached image, and a check that had no network is indistinguishable from one that found nothing. The rest are smaller.Also a heads-up:
feat/node-file-serveradds a near-identical autouse fixture and fake to the same lines oftests/conftest.pyandtests/fakes.pyand editsservetoo, so one of you gets a conflict. Worth one shared fixture rather than two.@ -41,1 +42,4 @@@pytest.fixture(autouse=True)def no_real_control_api_socket() -> Generator[None]:"""Keep the updater's control API off a real port.With this autouse everywhere and
FakeControlServerrestating the semantics under test, the realControlServeris never constructed —ControlServer.__init__is the only uncovered code inupdater/api.py, and the serve/shutdown ordering the PR description calls out is never actually run. One opt-in test (@pytest.mark.enable_socket) binding port 0 on 127.0.0.1 would cover it without claiming a fixed port.@ -0,0 +179,4 @@"image": "node_lighting-2026.09.4-firmware.bin","node": "lighting","node_type": LIGHTING,"sha256": firmware.FirmwareCache(tmp_path).images()[1].sha256,The expected hash is read back out of the cache, so this only asserts the API echoes
CachedImage.hashlib.sha256(LIGHTS).hexdigest()would actually check it.@ -0,0 +335,4 @@"""Node 0 is unassigned and 0xFF is the broadcast address: neither is a node."""response = control_for(tmp_path).respond("POST", f"/api/nodes/{node}/update")assert response.status in {400, 404}in {400, 404}can't tell which happened:""misses the route and gives 404, the rest give 400. Parametrise the expected status alongside the input.@ -0,0 +177,4 @@{"checked": False, "error": "A check is already running."},)try:cached = refresh(self.cache, self.releases)checkingonly excludes one API check from another.firmware.pollcallsrefreshwith its ownFirmwareCacheandForgejoReleasesand takes no lock, so aPOST /api/checklanding during the six-hourly poll puts two threads inFirmwareCache.storefor the same asset, writing the same<asset>.part. Whichever finishes first hashes it, renames it into place and writesimage.json; the other's handle follows the rename and keeps appending to the final file, so the recorded sha256 no longer matches the image now offered as installable. Silent, and exactly the failure mode this repo is built to avoid.Share one lock between the poll and the API, or have
/api/checknudge the poll task rather than callrefreshitself.Separately,
refreshruns inline in the request thread, so this holds the connection open for the whole download — 30 s per read, minutes over the van's link, with Home Assistant calling it from a button.@ -0,0 +181,4 @@finally:self.checking.release()return Response(HTTPStatus.OK,refreshswallowsOSError/ValueErrorand returns(), so a van with no network gets the same200 {"checked": true, "cached": []}as a van that is up to date. CCS-UHA-7/8 build an update entity on this shape and need to tell those apart — report whether the registry was reached, and consider a last-checked time on/api/status.@ -0,0 +209,4 @@"version": image.version,"accepted": True,"started": started,"detail": _detail(started, self.link.connected),detailis the only thing carrying why nothing was sent, and it is an English sentence. The test already substring-matches it (assert "bus link is not up" in ...), which is what every later caller will end up doing. Add a stablereasontoken (link_down,link_refused,sending) and keepdetailfor humans.@ -0,0 +305,4 @@"""Turn a request into a call on the API, and the answer into JSON."""server_version = "CampervanUpdater"protocol_version = "HTTP/1.1"HTTP/1.1 with no
timeouton the handler: an idle keep-alive connection blocks inreadlineforever and pins a thread for the life of the process. SettimeoutonControlHandler.@ -0,0 +346,4 @@out of step with the next request on it."""try:length = int(self.headers.get("Content-Length") or 0)_drainonly looks atContent-Length. A chunked body is left in the stream with the connection kept alive, so the next request line read is chunk data. Setclose_connectionwhenTransfer-Encodingis present.@ -0,0 +379,4 @@except OSError as error:_LOGGER.error("The control API could not bind port %d: %s", port, error)returnrunning = threading.Event()The
runningevent doesn't close the window the docstring describes: it is set beforeserve_foreveris entered, soshutdown()can still be called first.It is safe anyway —
serve_foreverclears__is_shut_down, checks__shutdown_requestbefore selecting and sets__is_shut_downin itsfinally, so an earlyshutdown()returns as soon as the thread starts. Drop the event and theasyncio.to_thread(running.wait)that burns a second executor thread waiting on it.Taken, in
190520e:firmware.REFRESH_LOCKnow covers both callers.refreshwaits for it, which is what the poll wants; the newrefresh_nowrefuses rather than queues, which is what the API wants, so 409 is a real answer rather than a guess._checkreturns aCheckcarryingreached, andPOST /api/checkreportsregistry_reached.refreshkeeps its old signature, so the poll and its tests are untouched.detailas prose. There is areasontoken beside it now:sent,no_linkorlink_refused. The tests match on the token.runninggate. Right, it narrowed the window without closing it, and the window is harmless. Gone, along with the extra pool worker; the "listening" line moved to just after the bind, which is where the socket actually starts listening.timeout = 30on the handler.Transfer-Encodingheader now ends the connection rather than leaving the stream out of step./api/nodes//updatesplit out as its own 404, and the expected hash is computed fromLIGHTS.socket_enabledand fetches/api/statuswithurllib. Ephemeral, so no fixed port has to be free.updater/api.pyis at 100% now.Not taken:
POST /api/checkblocks for the whole download. Deliberate, for now. An answer that says what it found is the useful one at a terminal, the threading server means a slow check holds nothing else up, and each request is bounded byREQUEST_TIMEOUT. Making it return immediately means keeping in-progress and last-result state, which is the same state CCS-UHA-5 will need for a transfer in flight — better added once there is something real to report than invented here. A caller should set its own timeout generously.The locking, the
Checkshape, thereasontoken, the simplified shutdown and the real-socket test all look right. Three things left, none of them blockers.On the question you asked: no,
refreshwaiting behind an API check can't hurt the poll.pollis sequential, so there is at most one waiter and no pile-up, and the wait replaces work it was about to do anyway.Separately, and pre-existing on the base rather than yours: a SIGTERM mid-check doesn't stop the download.
checking.cancel()cancels the coroutine, not theto_threadworker, andasyncio.runcloses by joining the default executor for up toTHREAD_JOIN_TIMEOUT(300 s), so the container sits there until the download finishes or Docker's 10 s grace SIGKILLs it. The new lock makes it slightly easier to hit — worth its own ticket, not this PR.@ -0,0 +524,4 @@content_type = response.headers["Content-Type"]finally:server.shutdown()answering.join(timeout=5)join(timeout=5)with nothing asserted after it: a server that never stops passes silently.assert not answering.is_alive().@ -0,0 +175,4 @@def check(self) -> Response:"""Look for a release now, rather than waiting for the next poll.This answers when the check is done, downloads and all, so a callerStaying synchronous is a fair call, but the reason given in the PR comment — "each request is bounded by
REQUEST_TIMEOUT" — isn't right.REQUEST_TIMEOUTgoes to_OPENER.open(..., timeout=...), which is a per-read socket timeout, not a deadline on the transfer. A link that trickles steadily never trips it, socopyfileobjcan run for as long as the image takes, once per asset. The request is unbounded.Either give the check an overall deadline, or say plainly here and in the README that it can run for minutes, since "set your timeout generously" needs something to be generous against.
@ -236,3 +278,3 @@_LOGGER.info("Cached %s %s, %d bytes", environment, version, image.size)cached.append(image)return tuple(cached)return Check(reached=True, cached=tuple(cached))reachedis only false whenlatest_release()throws.storeswallows its ownOSErrorand the short-download case and returnsNone, so a check that reached the registry and failed every download still answersregistry_reached: true, cached: []— indistinguishable from up to date. That is the same ambiguityreachedwas added to remove, one step further in. Carry what it tried and couldn't fetch.Clean. Nothing left to act on.
Agreed on the SIGTERM point being a separate ticket.