fix: defer --update cleanup until after preflight checks pass#705
Open
fix: defer --update cleanup until after preflight checks pass#705
Conversation
Marketen
approved these changes
May 6, 2026
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.
Summary
--updatedata-loss bug: a failing preflight check (e.g. VPN port conflict) used to wipe on-disk artifacts before the script had decided whether the install would proceed.docker-compose-*.yml,dappnode_package-*.json,*.txz,*.tar.xz,packages-content-hash.csv) is now deferred to a newclean_for_updatefunction called afterresolve_packages.bootstrap_filesystemstill removes the logfile and.dappnode_profileearly, soensure_profile_loadedre-downloads the latest profile (preserving existing behavior).check_vpn_ports_conflictnow recognizes ports held by our own VPN/Wireguard core containers (mirroring the existingcheck_https_ports_conflictpattern), so an--updateon a running node no longer aborts on its own ports.Why
In
main(),bootstrap_filesystemran theUPDATE=truecleanup block beforeresolve_packagesinvokedcheck_vpn_ports_conflict. If the port check failed and exited the script,/usr/src/dappnode/DNCORE/ended up with no compose files on disk. The currently running containers stayed up (they hold the ports), so the node kept working until the next restart — at which point nothing came back, because the YAML was gone.Repro (before this PR)
On any existing dappnode where the OpenVPN container is bound to 1194/UDP + 8092/TCP:
sudo wget -O - https://installer.dappnode.io | sudo bash -s -- --updateThe script aborts on the port conflict and
/usr/src/dappnode/DNCORE/is left empty of composes/manifests/archives.After this PR
--updateon a running node now proceeds normally: the self-aware port check recognizes our own VPN/Wireguard containers as non-conflicts and lets the install replace them. Even if a real port conflict is detected (foreign process), the destructive cleanup is skipped, so the previous artifacts remain on disk and the node is unaffected.Test plan
--update): unchanged behavior.--updateon a node with the OpenVPN container holding 1194/UDP + 8092/TCP: script logs that the existing VPN container will be replaced and proceeds with the update.--updateon a node with the Wireguard container holding 51820/UDP: script logs that the existing Wireguard container will be replaced and proceeds with the update.--updatewith a real foreign-process conflict on 1194/UDP, 8092/TCP, or 51820/UDP: script aborts with the port conflict error AND/usr/src/dappnode/DNCORE/still contains the previousdocker-compose-*.yml,dappnode_package-*.json, and*.txzfiles.