From 2dc48b05fb7afe02da9506882d70a265f44abb0d Mon Sep 17 00:00:00 2001 From: Thibault Deutsch Date: Thu, 8 Jun 2023 19:45:46 +0100 Subject: [PATCH] diagd: atomically write the ADS config Write the generated ADS config to a temporary file first, then rename the file. This ensure that the update is atomic and that ambex won't load a partially written file. Fix #5093 Signed-off-by: Thibault Deutsch --- python/ambassador_diag/diagd.py | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/python/ambassador_diag/diagd.py b/python/ambassador_diag/diagd.py index a5edc8f136..b7c29feb10 100644 --- a/python/ambassador_diag/diagd.py +++ b/python/ambassador_diag/diagd.py @@ -1656,8 +1656,19 @@ def _load_ir(self, rqueue: queue.Queue, aconf: Config, fetcher: ResourceFetcher, with open(app.bootstrap_path, "w") as output: output.write(dump_json(bootstrap_config, pretty=True)) - with open(app.ads_path, "w") as output: - output.write(dump_json(ads_config, pretty=True)) + # Write the ADS config to a temporary file first and then rename to + # ensure atomic update. Otherwise, ambex could be reading the config + # while we are still writing it. + # + # Rename are only atomic if in the same FS. To ensure that this work + # with any setup, we store the temporary file at the same path but with + # an extension that is ignored by ambex. + ads_tmp_path = app.ads_path + ".tmp" + with open(ads_tmp_path, "w") as output: + # Don't use pretty here, to reduce the config size, and improve the + # dump and load speed of the config. + output.write(dump_json(ads_config, pretty=False)) + os.rename(ads_tmp_path, app.ads_path) with open(app.clustermap_path, "w") as output: output.write(dump_json(clustermap, pretty=True))