From e99e0a28b8a07587c97bb2dc76e02fb1a83b2150 Mon Sep 17 00:00:00 2001 From: Josh Poole Date: Thu, 24 Sep 2026 11:31:13 +0100 Subject: [PATCH] 20260924 - Write config.yml and user.yml atomically too #42 routed the compose .env copies, tar1090.env and retina-tracker.yaml through write_file_atomic(), but not blah2's config.yml or either write of user.yml. On ret9573ecda one power cut at 2026-08-27 23:23:18 zeroed every file the config-merger wrote in that run, including the two written by temp file and rename without fsync. With config.yml zeroed, blah2 aborts in its YAML parser, and that node has produced no radar data since. Every write now goes through the helper, and a test parses the script and fails on any write-mode open(), copy or rename outside it. Co-Authored-By: Claude Opus 5.5 (1M context) --- config-merger/script/merge_config.py | 26 +++++++---------------- config-merger/test/test_merge_config.py | 28 +++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/config-merger/script/merge_config.py b/config-merger/script/merge_config.py index 5ae1c92..8c1c07d 100755 --- a/config-merger/script/merge_config.py +++ b/config-merger/script/merge_config.py @@ -26,7 +26,6 @@ """ import os -import shutil import sys import yaml @@ -383,11 +382,8 @@ def ensure_node_id(user_config_path): user_config['network'] = {} user_config['network']['node_id'] = node_id - # Write back to user config (atomic) - temp_path = user_config_path + '.node_id_tmp.' + str(os.getpid()) - with open(temp_path, 'w') as f: - yaml.dump(user_config, f, default_flow_style=False, sort_keys=False) - os.rename(temp_path, user_config_path) + write_file_atomic(user_config_path, yaml.dump( + user_config, default_flow_style=False, sort_keys=False)) print(f"Node ID set in {user_config_path}") @@ -426,17 +422,9 @@ def main(): print("User config not found, copying from defaults...") os.makedirs(os.path.dirname(user_config_path), exist_ok=True) - # Write to temp file first, then atomic rename - temp_path = user_config_path + '.tmp.' + str(os.getpid()) - shutil.copy(default_config, temp_path) - try: - os.rename(temp_path, user_config_path) # Atomic on POSIX - print(f"Created {user_config_path}") - except (FileExistsError, OSError): - # Another process created it first, clean up temp - if os.path.exists(temp_path): - os.remove(temp_path) - print("User config was created by another process") + with open(default_config) as f: + write_file_atomic(user_config_path, f.read()) + print(f"Created {user_config_path}") # Ensure node_id exists and matches hardware (add/update if needed, Pi only) ensure_node_id(user_config_path) @@ -473,8 +461,8 @@ def main(): # Write merged config to output print(f"Writing merged config to {output_config_path}") os.makedirs(os.path.dirname(output_config_path), exist_ok=True) - with open(output_config_path, 'w') as f: - yaml.dump(config, f, default_flow_style=False, sort_keys=False) + write_file_atomic(output_config_path, yaml.dump( + config, default_flow_style=False, sort_keys=False)) # Generate .env file for tar1090-node generate_env_file(config, os.path.dirname(output_config_path)) diff --git a/config-merger/test/test_merge_config.py b/config-merger/test/test_merge_config.py index 68f1070..0138a79 100755 --- a/config-merger/test/test_merge_config.py +++ b/config-merger/test/test_merge_config.py @@ -63,6 +63,14 @@ def run_merge(self): return output_yml + def test_config_yml_is_replaced_not_rewritten(self): + self.write_yaml(os.path.join(self.defaults_dir, 'default.yml'), {'network': {'ip': '0.0.0.0'}}) + output = self.run_merge() + before = os.stat(output).st_ino + self.run_merge() + self.assertNotEqual(os.stat(output).st_ino, before) + self.assertEqual(self.read_yaml(output)['network']['ip'], '0.0.0.0') + def test_defaults_only(self): """Test merge with only default.yml""" default_config = { @@ -1061,6 +1069,26 @@ def test_env_reaches_both_compose_slots_as_new_files(self): self.assertEqual(f.read(), expected) self.assertNotIn('\x00', expected) + def test_every_write_goes_through_write_file_atomic(self): + # ret9573ecda, 2026-08-27 23:23:18: config.yml, tar1090.env, + # retina-tracker.yaml and .env were all zeroed by one power cut. + # Temp-file-and-rename without fsync did not save the two written + # that way. The only write-mode open() allowed is the helper's own. + import ast + script = os.path.join(os.path.dirname(os.path.dirname(__file__)), 'script', 'merge_config.py') + with open(script) as f: + tree = ast.parse(f.read()) + writers = [] + for func in [n for n in ast.walk(tree) if isinstance(n, ast.FunctionDef)]: + for call in [n for n in ast.walk(func) if isinstance(n, ast.Call)]: + name = getattr(call.func, 'id', None) or getattr(call.func, 'attr', None) + mode = call.args[1].value if len(call.args) > 1 and isinstance(call.args[1], ast.Constant) else 'r' + if name == 'open' and any(c in str(mode) for c in 'wax'): + writers.append(func.name) + if name in ('copy', 'copyfile', 'rename'): + writers.append(func.name) + self.assertEqual(sorted(set(writers)), ['write_file_atomic']) + 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'))