From 66686192748c1a6ed37f901966c7df28ed154e3c Mon Sep 17 00:00:00 2001 From: Adam Collard Date: Thu, 24 Feb 2022 09:27:10 +0000 Subject: [PATCH 1/3] Suppress FileNotFoundError when reading status The status file read here is a symlink which is not atomically updated, since `get_status_details` already copes with not being able to read the status file, we simply suppress the error reading it. LP:1962150 --- cloudinit/cmd/status.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/cloudinit/cmd/status.py b/cloudinit/cmd/status.py index 5176549dd51..3a77d7dc846 100644 --- a/cloudinit/cmd/status.py +++ b/cloudinit/cmd/status.py @@ -5,6 +5,7 @@ """Define 'status' utility and handler as part of cloud-init commandline.""" import argparse +import contextlib import enum import os import sys @@ -141,7 +142,8 @@ def get_status_details(paths=None): if os.path.exists(status_file): if not os.path.exists(result_file): status = UXAppStatus.RUNNING - status_v1 = load_json(load_file(status_file)).get("v1", {}) + with contextlib.suppress(FileNotFoundError): + status_v1 = load_json(load_file(status_file)).get("v1", {}) errors = [] latest_event = 0 for key, value in sorted(status_v1.items()): From f6beb15bb1df6421f9c6dcb5501e4c58971de093 Mon Sep 17 00:00:00 2001 From: Adam Collard Date: Tue, 22 Mar 2022 09:56:19 +0000 Subject: [PATCH 2/3] Avoid creation of race in the first place Suggested-by: Chad Smith --- cloudinit/util.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/cloudinit/util.py b/cloudinit/util.py index 1fadd196291..ae3311e3e28 100644 --- a/cloudinit/util.py +++ b/cloudinit/util.py @@ -1887,7 +1887,12 @@ def is_link(path): def sym_link(source, link, force=False): LOG.debug("Creating symbolic link from %r => %r", link, source) if force and os.path.lexists(link): - del_file(link) + # Provide atomic update of symlink to avoid races with status --wait + # LP: #1962150 + tmp_link = os.path.join(os.path.dirname(link), "tmp" + rand_str(8)) + os.symlink(source, tmp_link) + os.replace(tmp_link, link) + return os.symlink(source, link) From f9761f6040399f7351e80441e0dcd7a13b4a10ec Mon Sep 17 00:00:00 2001 From: James Falcon Date: Thu, 7 Apr 2022 10:42:56 -0500 Subject: [PATCH 3/3] review comments --- cloudinit/cmd/status.py | 4 +--- tests/unittests/test_util.py | 5 ++++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/cloudinit/cmd/status.py b/cloudinit/cmd/status.py index 3a77d7dc846..5176549dd51 100644 --- a/cloudinit/cmd/status.py +++ b/cloudinit/cmd/status.py @@ -5,7 +5,6 @@ """Define 'status' utility and handler as part of cloud-init commandline.""" import argparse -import contextlib import enum import os import sys @@ -142,8 +141,7 @@ def get_status_details(paths=None): if os.path.exists(status_file): if not os.path.exists(result_file): status = UXAppStatus.RUNNING - with contextlib.suppress(FileNotFoundError): - status_v1 = load_json(load_file(status_file)).get("v1", {}) + status_v1 = load_json(load_file(status_file)).get("v1", {}) errors = [] latest_event = 0 for key, value in sorted(status_v1.items()): diff --git a/tests/unittests/test_util.py b/tests/unittests/test_util.py index 3f3079b04e7..db6e2bc4be6 100644 --- a/tests/unittests/test_util.py +++ b/tests/unittests/test_util.py @@ -376,13 +376,16 @@ def test_sym_link_source_exists(self): tmpd = self.tmp_dir() link = self.tmp_path("link", tmpd) target = self.tmp_path("target", tmpd) + target2 = self.tmp_path("target2", tmpd) util.write_file(target, "hello") + util.write_file(target2, "hello2") util.sym_link(target, link) self.assertTrue(os.path.exists(link)) - util.sym_link(target, link, force=True) + util.sym_link(target2, link, force=True) self.assertTrue(os.path.exists(link)) + self.assertEqual("hello2", util.load_file(link)) def test_sym_link_dangling_link(self): tmpd = self.tmp_dir()