20260924 - Repair node state before install and write .env atomically

.github/workflows/lint.yml

@@ -30,8 +30,9 @@ jobs:
       # installs the same two unpinned — keep the two in step.
       - run: pip install pyyaml mergedeep 'pytest>=8.0'
 
-      # Tests live under config-merger/test/, not tests/, so the path is
-      # explicit. This suite had never run in CI until 86cb4jfk7; the failure
-      # it now protects against had gone unnoticed for five months.
+      # Tests live under config-merger/test/ and scripts/test/, not tests/, so
+      # the paths are explicit. This suite had never run in CI until
+      # 86cb4jfk7; the failure it now protects against had gone unnoticed for
+      # five months. scripts/test/ runs the Mender state scripts under /bin/sh.
       - name: Tests
-        run: pytest config-merger/test/
+        run: pytest config-merger/test/ scripts/test/

config-merger/script/merge_config.py

@@ -69,6 +69,33 @@ def get_node_id_from_mender():
     return None
 
 
+MENDER_COMPOSE_ROOT = '/data/mender-docker-compose'
+
+
+def write_file_atomic(path, content):
+    """Replace path with content so that a power cut leaves the old file or the
+    new one, never a mix.
+
+    A plain rewrite in place can leave the file at its new length with its data
+    never written, which ext4 on an SD card reads back as NUL bytes. A NUL-filled
+    manifests/.env stops compose parsing the project at all, including the
+    `run config-merger` that would regenerate it, so a node that loses power at
+    the wrong moment cannot recover by itself. The fsyncs put the data on disk
+    before the rename is committed, and the rename itself on disk before return.
+    """
+    temp_path = path + '.tmp.' + str(os.getpid())
+    with open(temp_path, 'w') as f:
+        f.write(content)
+        f.flush()
+        os.fsync(f.fileno())
+    os.replace(temp_path, path)
+    dir_fd = os.open(os.path.dirname(path) or '.', os.O_RDONLY)
+    try:
+        os.fsync(dir_fd)
+    finally:
+        os.close(dir_fd)
+
+
 def generate_env_file(config, output_dir):
     """Generate .env file for ADS-B services (readsb, tar1090) from merged config"""
     if 'tar1090' not in config:
@@ -82,52 +109,50 @@ def generate_env_file(config, output_dir):
     env_path = os.path.join(output_dir, 'tar1090.env')
     print(f"Writing tar1090 .env to {env_path}")
 
-    # Write to temp file first, then atomic rename
-    temp_path = env_path + '.tmp.' + str(os.getpid())
     lat = location.get('latitude')
     lon = location.get('longitude')
     alt = location.get('altitude')
     sited = lat is not None and lon is not None
 
-    with open(temp_path, 'w') as f:
-        f.write("# Auto-generated by config-merger - do not edit manually\n\n")
-
-        # Receiver location (matches tar1090-node .env.example format).
-        # Omitted when unset. This used to default to 0, which is Null Island:
-        # a real place the node then claimed to be.
-        f.write("# Receiver location\n")
-        if sited:
-            f.write(f"RECEIVER_LAT={lat}\n")
-            f.write(f"RECEIVER_LON={lon}\n")
-            f.write(f"RECEIVER_ALT={alt if alt is not None else 0}\n\n")
-        else:
-            f.write("# unset: no receiver location has been configured\n\n")
-
-        # adsb.lol integration. Forced off without a location whatever the
-        # config says: the query is a radius around the receiver, so with no
-        # receiver there is no query to make, only a wrong one.
-        adsblol = bool(tar1090_config.get('adsblol_fallback', False)) and sited
-        f.write("# adsb.lol integration\n")
-        f.write(f"ADSBLOL_ENABLED={'true' if adsblol else 'false'}\n")
-        f.write(f"ADSBLOL_RADIUS={tar1090_config.get('adsblol_radius', 40)}\n\n")
-
-        # External ADS-B feed for readsb
-        adsb_source = tar1090_config.get('adsb_source', '')
-        if adsb_source:
-            f.write("# readsb external feed\n")
-            f.write(f"READSB_NET_CONNECTOR={adsb_source}\n")
-
-    os.rename(temp_path, env_path)
+    lines = ["# Auto-generated by config-merger - do not edit manually\n\n"]
+
+    # Receiver location (matches tar1090-node .env.example format).
+    # Omitted when unset. This used to default to 0, which is Null Island:
+    # a real place the node then claimed to be.
+    lines.append("# Receiver location\n")
+    if sited:
+        lines.append(f"RECEIVER_LAT={lat}\n")
+        lines.append(f"RECEIVER_LON={lon}\n")
+        lines.append(f"RECEIVER_ALT={alt if alt is not None else 0}\n\n")
+    else:
+        lines.append("# unset: no receiver location has been configured\n\n")
+
+    # adsb.lol integration. Forced off without a location whatever the
+    # config says: the query is a radius around the receiver, so with no
+    # receiver there is no query to make, only a wrong one.
+    adsblol = bool(tar1090_config.get('adsblol_fallback', False)) and sited
+    lines.append("# adsb.lol integration\n")
+    lines.append(f"ADSBLOL_ENABLED={'true' if adsblol else 'false'}\n")
+    lines.append(f"ADSBLOL_RADIUS={tar1090_config.get('adsblol_radius', 40)}\n\n")
+
+    # External ADS-B feed for readsb
+    adsb_source = tar1090_config.get('adsb_source', '')
+    if adsb_source:
+        lines.append("# readsb external feed\n")
+        lines.append(f"READSB_NET_CONNECTOR={adsb_source}\n")
+
+    content = ''.join(lines)
+    write_file_atomic(env_path, content)
     print("tar1090 .env generated successfully!")
 
     # Copy .env to Mender compose directories for variable substitution
     # mender-docker-compose uses current/ (active) and new/ (staging during update)
     for slot in ['current', 'new']:
-        manifests_dir = f'/data/mender-docker-compose/{slot}/manifests'
+        manifests_dir = os.path.join(MENDER_COMPOSE_ROOT, slot, 'manifests')
         env_dest = os.path.join(manifests_dir, '.env')
         try:
             if os.path.isdir(manifests_dir):
-                shutil.copy(env_path, env_dest)
+                write_file_atomic(env_dest, content)
                 print(f"Copied .env to {env_dest}")
         except Exception as e:
             print(f"Note: Could not copy to {env_dest}: {e}")
@@ -152,11 +177,8 @@ def generate_retina_tracker_config(config, output_dir):
     output_path = os.path.join(output_dir, 'retina-tracker.yaml')
     print(f"Writing retina-tracker config to {output_path}")
 
-    # Write to temp file first, then atomic rename
-    temp_path = output_path + '.tmp.' + str(os.getpid())
-    with open(temp_path, 'w') as f:
-        yaml.dump({'tracker': config['retina_tracker']}, f, default_flow_style=False, sort_keys=False)
-    os.rename(temp_path, output_path)
+    write_file_atomic(output_path, yaml.dump(
+        {'tracker': config['retina_tracker']}, default_flow_style=False, sort_keys=False))
     print("retina-tracker.yaml generated successfully!")
 
 

config-merger/test/test_merge_config.py

@@ -1005,5 +1005,73 @@ def test_tracker_removal_does_not_rewrite_user_yml(self):
         self.assertTrue(user['process']['tracker']['enable'])
         self.assertEqual(user['network']['ports']['track'], 3003)
 
+
+class TestAtomicWrites(unittest.TestCase):
+    """The compose .env must be replaced whole, never rewritten in place.
+
+    A NUL-filled manifests/.env (seen on ret9573ecda) stops compose loading the
+    project, so the node can neither update nor regenerate the file.
+    """
+
+    def setUp(self):
+        import importlib.util
+        script = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'script', 'merge_config.py')
+        spec = importlib.util.spec_from_file_location('merge_config', script)
+        self.mc = importlib.util.module_from_spec(spec)
+        spec.loader.exec_module(self.mc)
+        self.test_dir = tempfile.mkdtemp()
+
+    def tearDown(self):
+        shutil.rmtree(self.test_dir)
+
+    def test_replaces_the_file_rather_than_rewriting_it(self):
+        path = os.path.join(self.test_dir, '.env')
+        with open(path, 'w') as f:
+            f.write('OLD=1\n')
+        before = os.stat(path).st_ino
+
+        self.mc.write_file_atomic(path, 'NEW=1\n')
+
+        with open(path) as f:
+            self.assertEqual(f.read(), 'NEW=1\n')
+        self.assertNotEqual(os.stat(path).st_ino, before)
+
+    def test_leaves_no_temp_file_behind(self):
+        path = os.path.join(self.test_dir, '.env')
+        self.mc.write_file_atomic(path, 'A=1\n')
+        self.assertEqual(os.listdir(self.test_dir), ['.env'])
+
+    def test_env_reaches_both_compose_slots_as_new_files(self):
+        root = os.path.join(self.test_dir, 'mender-docker-compose')
+        for slot in ('current', 'new'):
+            os.makedirs(os.path.join(root, slot, 'manifests'))
+        stale = os.path.join(root, 'current', 'manifests', '.env')
+        with open(stale, 'wb') as f:
+            f.write(b'\x00' * 208)
+        self.mc.MENDER_COMPOSE_ROOT = root
+        output_dir = os.path.join(self.test_dir, 'config')
+        os.makedirs(output_dir)
+
+        self.mc.generate_env_file({'tar1090': {'adsblol_fallback': False}}, output_dir)
+
+        with open(os.path.join(output_dir, 'tar1090.env')) as f:
+            expected = f.read()
+        for slot in ('current', 'new'):
+            with open(os.path.join(root, slot, 'manifests', '.env')) as f:
+                self.assertEqual(f.read(), expected)
+        self.assertNotIn('\x00', expected)
+
+    def test_skips_a_slot_that_does_not_exist(self):
+        root = os.path.join(self.test_dir, 'mender-docker-compose')
+        os.makedirs(os.path.join(root, 'current', 'manifests'))
+        self.mc.MENDER_COMPOSE_ROOT = root
+        output_dir = os.path.join(self.test_dir, 'config')
+        os.makedirs(output_dir)
+
+        self.mc.generate_env_file({'tar1090': {}}, output_dir)
+
+        self.assertFalse(os.path.exists(os.path.join(root, 'new')))
+
+
 if __name__ == '__main__':
     unittest.main()

scripts/build_mender_artifact.sh

@@ -90,7 +90,9 @@ gen_docker-compose \
     --software-filesystem data-docker \
     --clears-provides "rootfs-image.retina-node.version" \
     --script "${SCRIPT_DIR}/ArtifactInstall_Enter_00_retina_state" \
+    --script "${SCRIPT_DIR}/ArtifactInstall_Enter_10_retina_preflight" \
     --script "${SCRIPT_DIR}/ArtifactCommit_Leave_00_retina_state" \
+    --script "${SCRIPT_DIR}/ArtifactCommit_Leave_10_retina_env_reload" \
     --script "${SCRIPT_DIR}/ArtifactFailure_Enter_00_retina_state"
 
 # Validate artifact

scripts/mender-state-scripts/ArtifactCommit_Leave_10_retina_env_reload

@@ -0,0 +1,31 @@
+#!/bin/sh
+# Recreate tar1090 from the committed composition, so it picks up the node's
+# own .env.
+#
+# The docker-compose Update Module starts the new stack from new/manifests,
+# which it copies out of the artifact and which has no .env. Compose therefore
+# interpolates tar1090's receiver location and adsb.lol settings with their
+# defaults (0,0 and off), and the config-merger writes the real .env a moment
+# too late. Nothing interpolates again until the stack next starts, which on
+# most releases is hidden by the owl-os reboot that follows the retina-node one.
+#
+# By ArtifactCommit_Leave, new/ has become current/ and holds the .env the
+# config-merger wrote. `up --no-deps tar1090` touches no other service, and
+# recreates tar1090 only if its interpolated config changed. This never fails
+# the deployment: the update is already committed.
+
+COMPOSE_ROOT="${RETINA_ENV_RELOAD_COMPOSE_ROOT:-/data/mender-docker-compose}"
+manifests="$COMPOSE_ROOT/current/manifests"
+
+if [ ! -f "$manifests/.env" ]; then
+    echo "retina-env-reload: no $manifests/.env, leaving tar1090 as started" >&2
+    exit 0
+fi
+
+if out=$(cd "$manifests" && docker compose -p retina-node up -d --no-deps tar1090 2>&1); then
+    echo "retina-env-reload: tar1090 matches $manifests/.env" >&2
+else
+    echo "retina-env-reload: could not update tar1090: $out" >&2
+fi
+
+exit 0

scripts/mender-state-scripts/ArtifactInstall_Enter_10_retina_preflight

@@ -0,0 +1,92 @@
+#!/bin/sh
+# Repair the node state that makes a retina-node install fail, before the
+# docker-compose Update Module touches the running stack.
+#
+# The module renames current/ to previous/ and stops it, and only then finds
+# out whether the new stack can start. Two kinds of drift have failed that
+# install and then failed its rollback for the same reason, leaving the radar
+# stopped:
+#
+#   * a NUL-filled manifests/.env (power cut mid-write). Compose refuses to
+#     load the project at all, so it can neither stop the old stack nor start
+#     the new one.
+#   * a container that is not part of this compose project holding one of the
+#     project's fixed container_names. Compose cannot create its own.
+#
+# This repairs and never aborts. If this script fails the install, the
+# module's rollback finds neither new/ nor previous/ and takes its "rollback
+# after a commit" branch, which moves the live current/ out of the way. So
+# every step is best effort, nothing is deleted, and the exit status is 0.
+
+COMPOSE_ROOT="${RETINA_PREFLIGHT_COMPOSE_ROOT:-/data/mender-docker-compose}"
+PAYLOAD_FILES="${RETINA_PREFLIGHT_PAYLOAD_FILES:-/var/lib/mender/modules/v3/payloads/0000/tree/files}"
+PROJECT=retina-node
+LOG_FILE="$COMPOSE_ROOT/preflight.log"
+LOG_KEEP_LINES=200
+STAMP=$(date -u +%Y%m%dT%H%M%SZ)
+
+log() {
+    # stderr reaches the Mender deployment log; the file survives the deployment.
+    echo "retina-preflight: $*" >&2
+    echo "$STAMP $*" >> "$LOG_FILE" 2>/dev/null
+}
+
+current_manifests="$COMPOSE_ROOT/current/manifests"
+
+# --- 1. A .env compose cannot parse -------------------------------------------
+
+env_file="$current_manifests/.env"
+if [ -f "$env_file" ]; then
+    reason=""
+    if [ "$(tr -d '\000' < "$env_file" | wc -c)" -ne "$(wc -c < "$env_file")" ]; then
+        reason="contains NUL bytes"
+    elif compose_err=$(cd "$current_manifests" && docker compose -p "$PROJECT" config -q 2>&1); then
+        :
+    else
+        case "$compose_err" in
+            *"$env_file"*|*"/.env:"*) reason="compose cannot parse it" ;;
+        esac
+    fi
+    if [ -n "$reason" ]; then
+        if mv "$env_file" "$env_file.corrupt-$STAMP"; then
+            log "set aside $env_file ($reason) as .env.corrupt-$STAMP; config-merger regenerates it on the next start"
+        else
+            log "could not set aside $env_file ($reason)"
+        fi
+    fi
+fi
+
+# --- 2. Stray containers holding the project's names --------------------------
+
+container_names() {
+    # The incoming manifest is authoritative; the current one covers a name the
+    # new release dropped but a stray container may still hold.
+    {
+        if [ -f "$PAYLOAD_FILES/manifests.tar" ]; then
+            tar -xOf "$PAYLOAD_FILES/manifests.tar" 2>/dev/null
+        fi
+        cat "$current_manifests"/*.yaml "$current_manifests"/*.yml 2>/dev/null
+    } | sed -n 's/^[[:space:]]*container_name:[[:space:]]*["'\'']\{0,1\}\([A-Za-z0-9_.-]*\).*/\1/p' | sort -u
+}
+
+for name in $(container_names); do
+    owner=$(docker inspect --type container \
+        -f '{{index .Config.Labels "com.docker.compose.project"}}' "$name" 2>/dev/null) || continue
+    [ "$owner" = "$PROJECT" ] && continue
+
+    parked="$name-parked-$STAMP"
+    docker update --restart=no "$name" > /dev/null 2>&1
+    docker stop -t 10 "$name" > /dev/null 2>&1
+    if docker rename "$name" "$parked" > /dev/null 2>&1; then
+        log "parked container $name (compose project '${owner:-none}') as $parked; it is stopped, not removed"
+    else
+        log "could not park container $name (compose project '${owner:-none}'); the install will fail on its name"
+    fi
+done
+
+# Keep the local record small: /data is shared with everything else on the node.
+if [ -f "$LOG_FILE" ]; then
+    tail -n "$LOG_KEEP_LINES" "$LOG_FILE" > "$LOG_FILE.tmp" 2>/dev/null && mv "$LOG_FILE.tmp" "$LOG_FILE"
+fi
+
+exit 0