[1.4] Fix extension update wiping the running version when pull fails - #4338
Conversation
0ba998c to
d8db3f9
Compare
8776fcf to
209e1cb
Compare
Automated PR Review0. Summary
Fixes a nasty install/update path in Kraken where a failed Docker pull could delete the running image and leave the extension pointing at a version with no local backing. Introduces an atomic rollback (restoring the prior No further comments, nice job 👍 Generated by PR Review Bot. This is advisory, a human reviewer must still approve. |
4ccd3a8 to
c758d98
Compare
* Reduce code complexity of install function by spliting in multiple parts
max() floored every retry at 600s after the first failed start.
update() and v2 from_latest/tagged install used the non-atomic default, so a failed pull was treated as success and purged the running tag.
Failed pulls uninstalled the new tag and could delete the running image. Restore prior settings, refuse a sibling-image alias, and require a Docker success status on the install pull stream.
…tatus start() used a non-streaming pull that ignored in-band Docker errors.
c758d98 to
02dea1d
Compare
| docker_auth = f"{self.source.auth.username}:{self.source.auth.password}" | ||
| return base64.b64encode(docker_auth.encode("utf-8")).decode("utf-8") | ||
|
|
||
| async def _image_is_available_locally(self) -> bool: |
There was a problem hiding this comment.
can we avoid moving this function around ? it just add unnecessary diff
| if self.digest: | ||
| await client.images.tag(tag, f"{self.source.docker}:{self.tag}") | ||
| # First we should make sure no other tag is running | ||
| running_ext = await self._disable_running_extension() |
There was a problem hiding this comment.
we are doing nothing with this variable, is this expected ?
| user_permissions=self.source.user_permissions, | ||
| ) | ||
| # Save in settings first, if the image fails to install it will try to fetch after in main kraken check loop | ||
| # Atomic failure rolls this entry back and re-enables the previously running sibling. |
There was a problem hiding this comment.
should this be in anoter commit ?
| # start() and a successful pull run docker:tag. Catalog platform digests are not | ||
| # stored in RepoDigests after `docker pull repo:tag` (that records the index | ||
| # digest), so a digest match against the catalog cannot be required. | ||
| # sibling_image_id is only for the failed-pull fallback: a retag of the running | ||
| # sibling onto the new name is not the requested version. Do not pass it after a | ||
| # clean pull -- two tags can share an image Id (aliases) and still be the pull. |
There was a problem hiding this comment.
This is not friendly at all.
It took so much time to understand, and I'm still not sure.
We get an image from DockerHub, their "catalog" ID does not match with what we receive, is that right ?
sibling_image_id is the image that is used as base from a failed attempt ? Or is the image that has different "repo:tag" ?
| except Exception as rollback_error: | ||
| logger.warning(f"Failed to roll back {self.identifier}:{self.tag} after pull failure: {rollback_error}") | ||
| if not running_ext: | ||
| return |
There was a problem hiding this comment.
should we have a log message here ?
3 participants
Summary
update()and v2 tagged/from_latestinstall atomically (default). A failed pull restores a copy of prior settings for that tag (including same-tag reinstall) and re-enables the previously running sibling, withoutimages.deleteof a possible alias.docker:tagfor whatstart()actually runs. Only Docker 404 means the image is absent. Any other inspect/daemon error refuses the local-image fallback and rolls back — it does not report success.Downloaded newer imageorImage is up to date. Disconnect or empty completion rolls back instead of purging the running tag.docker:tagalready exists and is not the sibling's image, keep that tag and do not purge other tags.mininstead ofmax(was ~600s after the first failure).Closes #1643
Test plan
192.168.0.147(1.4-dev):update_to_versionCockpitv1.18.2→v1.19.0-beta.9with Docker Hub blocked returned HTTP 200 with an empty body, replaced settings with the new tag, deleted the old image, and the watchdog then failed withNo such image.install()/update()HTTP caller plus watchdogstart(), overlay confirmed (do notdocker restart blueos-coreon this DUT). Includes:update_to_versionHub blocked, target not local (settings + image + container stay)start()with image deleted and Hub blocked (Image not found locally+Failed to start, noNo such image; retry ~15s withminbackoff):tagis retagged and update succeedsfrom_latestinstall never-installed, Hub blockeddocker:A, aliasdocker:B, Hub blockedupdate_to_version→Failed to pull, A stays enabled. Empty pull stream (Pulling fs layeronly) →pull finished without a success status, A kept, B not enabled. 14/14.journey_http --extension-lifecycle --allow-mutating --identifier bluerobotics.cockpit --tag v1.18.2 --assert-unknown: 181 passed, 0 failed; DUT restored to baseline (blueos.major_tomonly).