From f832c1484c69563a95171dfb64f92ad70be89893 Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Sat, 20 Jul 2024 21:56:59 -0700 Subject: [PATCH 01/12] create symlink to latest log directories --- sky/backends/cloud_vm_ray_backend.py | 5 +++-- sky/callbacks/sky_callback/base.py | 4 +++- sky/provision/logging.py | 3 ++- sky/skylet/constants.py | 1 + sky/skylet/log_lib.py | 2 +- sky/utils/command_runner.py | 5 +++-- sky/utils/log_utils.py | 23 +++++++++++++++++++++++ 7 files changed, 36 insertions(+), 7 deletions(-) diff --git a/sky/backends/cloud_vm_ray_backend.py b/sky/backends/cloud_vm_ray_backend.py index ed157736007..6f46a12e34d 100644 --- a/sky/backends/cloud_vm_ray_backend.py +++ b/sky/backends/cloud_vm_ray_backend.py @@ -1307,7 +1307,8 @@ def _retry_zones( log_path = os.path.join(self.log_dir, 'provision.log') log_abs_path = os.path.abspath(log_path) if not dryrun: - os.makedirs(os.path.expanduser(self.log_dir), exist_ok=True) + log_utils.create_and_symlink_log_dir( + os.path.expanduser(self.log_dir)) os.system(f'touch {log_path}') tail_cmd = f'tail -n100 -f {log_path}' logger.info('To view detailed progress: ' @@ -3052,7 +3053,7 @@ def _sync_workdir_node(runner: command_runner.CommandRunner) -> None: f'{style.BRIGHT}{workdir}{style.RESET_ALL}' f' -> ' f'{style.BRIGHT}{SKY_REMOTE_WORKDIR}{style.RESET_ALL}') - os.makedirs(os.path.expanduser(self.log_dir), exist_ok=True) + log_utils.create_and_symlink_log_dir(os.path.expanduser(self.log_dir)) os.system(f'touch {log_path}') tail_cmd = f'tail -n100 -f {log_path}' logger.info('To view detailed progress: ' diff --git a/sky/callbacks/sky_callback/base.py b/sky/callbacks/sky_callback/base.py index 06fa073482c..7f4c5e862a5 100644 --- a/sky/callbacks/sky_callback/base.py +++ b/sky/callbacks/sky_callback/base.py @@ -9,6 +9,8 @@ import psutil +from sky.utils import log_utils + # NOTE: This must be the same as _SKY_REMOTE_BENCHMARK_DIR_SYMLINK # in sky/benchmark/benchmark_utils.py. _SKY_REMOTE_BENCHMARK_DIR = '~/sky_benchmark_dir' @@ -39,7 +41,7 @@ def __init__(self, log_dir, 'sky-callback-' + datetime.datetime.now().strftime('%Y-%m-%d-%H-%M-%S-%f')) log_dir = os.path.expanduser(log_dir) - os.makedirs(log_dir, exist_ok=True) + log_utils.create_and_symlink_log_dir(log_dir) # TODO(woosuk): Do not store the entire timestamps. self._step_begins = [] diff --git a/sky/provision/logging.py b/sky/provision/logging.py index 2ea3967d15c..4839d4352e1 100644 --- a/sky/provision/logging.py +++ b/sky/provision/logging.py @@ -9,6 +9,7 @@ import threading from sky import sky_logging +from sky.utils import log_utils @dataclasses.dataclass @@ -24,7 +25,7 @@ def setup_provision_logging(log_dir: str): try: # Redirect underlying provision logs to file. log_path = os.path.expanduser(os.path.join(log_dir, 'provision.log')) - os.makedirs(os.path.dirname(log_path), exist_ok=True) + log_utils.create_and_symlink_log_dir(os.path.dirname(log_path)) log_abs_path = pathlib.Path(log_path).expanduser().absolute() fh = logging.FileHandler(log_abs_path) fh.setFormatter(sky_logging.FORMATTER) diff --git a/sky/skylet/constants.py b/sky/skylet/constants.py index 84a6491605a..e13d81546d7 100644 --- a/sky/skylet/constants.py +++ b/sky/skylet/constants.py @@ -6,6 +6,7 @@ import sky SKY_LOGS_DIRECTORY = '~/sky_logs' +SKY_LATEST_LINK = 'sky_latest' SKY_REMOTE_WORKDIR = '~/sky_workdir' # Default Ray port is 6379. Default Ray dashboard port is 8265. diff --git a/sky/skylet/log_lib.py b/sky/skylet/log_lib.py index d184abd107e..aa8e3713efb 100644 --- a/sky/skylet/log_lib.py +++ b/sky/skylet/log_lib.py @@ -163,7 +163,7 @@ def run_with_log( log_path = os.path.expanduser(log_path) dirname = os.path.dirname(log_path) - os.makedirs(dirname, exist_ok=True) + log_utils.create_and_symlink_log_dir(dirname) # Redirect stderr to stdout when using ray, to preserve the order of # stdout and stderr. stdout_arg = stderr_arg = None diff --git a/sky/utils/command_runner.py b/sky/utils/command_runner.py index 8529874092a..73ac20be801 100644 --- a/sky/utils/command_runner.py +++ b/sky/utils/command_runner.py @@ -11,6 +11,7 @@ from sky.skylet import constants from sky.skylet import log_lib from sky.utils import common_utils +from sky.utils import log_utils from sky.utils import subprocess_utils from sky.utils import timeline @@ -571,7 +572,7 @@ def run( command = base_ssh_command + [shlex.quote(command_str)] log_dir = os.path.expanduser(os.path.dirname(log_path)) - os.makedirs(log_dir, exist_ok=True) + log_utils.create_and_symlink_log_dir(log_dir) executable = None if not process_stream: @@ -750,7 +751,7 @@ def run( ] log_dir = os.path.expanduser(os.path.dirname(log_path)) - os.makedirs(log_dir, exist_ok=True) + log_utils.create_and_symlink_log_dir(log_dir) executable = None if not process_stream: diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index 90928b8014d..16903b41766 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -1,5 +1,6 @@ """Logging utils.""" import enum +import os from typing import List, Optional import colorama @@ -7,6 +8,7 @@ import prettytable from sky import sky_logging +from sky.skylet import constants from sky.utils import rich_utils logger = sky_logging.init_logger(__name__) @@ -191,3 +193,24 @@ def readable_time_duration(start: Optional[float], diff = diff.replace('hour', 'hr') return diff + + +def create_and_symlink_log_dir(log_dir: str): + """Creates a new log directory and symlinks to it from parent directory. + + Args: + log_dir (str): Expanded log directory path. + """ + if log_dir == os.devnull: + return + os.makedirs(log_dir, exist_ok=True) + symlink_path = os.path.join(log_dir, os.pardir, constants.SKY_LATEST_LINK) + if os.path.exists(symlink_path): + if os.path.islink(symlink_path): + os.remove(symlink_path) + else: + logger.warning( + 'Failed to create symlink to latest logs at \'{symlink_path}\'. Please remove the existing file/directory.' + ) + return + os.symlink(log_dir, symlink_path) From 79ae197f407cc2f34a2f930e2b7a2461e6e90edb Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Sat, 20 Jul 2024 22:18:04 -0700 Subject: [PATCH 02/12] fix formatting --- sky/utils/log_utils.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index 16903b41766..46f9e19e03d 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -209,8 +209,8 @@ def create_and_symlink_log_dir(log_dir: str): if os.path.islink(symlink_path): os.remove(symlink_path) else: - logger.warning( - 'Failed to create symlink to latest logs at \'{symlink_path}\'. Please remove the existing file/directory.' - ) + logger.warning(( + 'Failed to create symlink to latest logs at \'{symlink_path}\'.' + 'Please remove the existing file/directory.')) return os.symlink(log_dir, symlink_path) From b967306be1ff55a52893b6fef30ec4a07690d668 Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Sun, 21 Jul 2024 20:21:21 -0700 Subject: [PATCH 03/12] Add symlink log smoke test and fix formatting in warning string --- sky/utils/log_utils.py | 2 +- tests/test_smoke.py | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index 46f9e19e03d..a6871eca901 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -210,7 +210,7 @@ def create_and_symlink_log_dir(log_dir: str): os.remove(symlink_path) else: logger.warning(( - 'Failed to create symlink to latest logs at \'{symlink_path}\'.' + f'Failed to create symlink to latest logs at \'{symlink_path}\'.' 'Please remove the existing file/directory.')) return os.symlink(log_dir, symlink_path) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 325a836cf4c..bca18777235 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -1317,6 +1317,24 @@ def test_scp_logs(): run_one_test(test) +@pytest.mark.kubernetes +def test_symlink_latest_logs(): + test = Test( + 'symlink_latest_logs', + [ + 'sky local up || true', + 'eval symlink_path="~/sky_logs/sky_latest"; [ -L "$symlink_path" ] || exit 1 ;' + 'target_file=$(readlink -f "$symlink_path") || exit 1 ;' + 'sky local down || true ;' + '[ -L "$symlink_path" ] || exit 1 ;' + 'target_file2=$(readlink -f "$symlink_path") ;' + '[ "$target_file" != "$target_file2" ] || exit 1' + ], + 'sky local down', + ) + run_one_test(test) + + # ---------- Job Queue. ---------- @pytest.mark.no_fluidstack # FluidStack DC has low availability of T4 GPUs @pytest.mark.no_lambda_cloud # Lambda Cloud does not have T4 gpus From a8517e0842c1b0f183815a52f0f02bf57a32ba99 Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Sun, 21 Jul 2024 20:36:20 -0700 Subject: [PATCH 04/12] grapple with pep8 --- sky/utils/log_utils.py | 6 +++--- tests/test_smoke.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index a6871eca901..04ec07a3a80 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -209,8 +209,8 @@ def create_and_symlink_log_dir(log_dir: str): if os.path.islink(symlink_path): os.remove(symlink_path) else: - logger.warning(( - f'Failed to create symlink to latest logs at \'{symlink_path}\'.' - 'Please remove the existing file/directory.')) + logger.warning( + (f'Failed to symlink to latest logs at \'{symlink_path}\'.' + 'Please remove the existing file/directory.')) return os.symlink(log_dir, symlink_path) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index bca18777235..e038bf51e0b 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -1320,7 +1320,7 @@ def test_scp_logs(): @pytest.mark.kubernetes def test_symlink_latest_logs(): test = Test( - 'symlink_latest_logs', + 'symlink_latest_logs', [ 'sky local up || true', 'eval symlink_path="~/sky_logs/sky_latest"; [ -L "$symlink_path" ] || exit 1 ;' @@ -1329,7 +1329,7 @@ def test_symlink_latest_logs(): '[ -L "$symlink_path" ] || exit 1 ;' 'target_file2=$(readlink -f "$symlink_path") ;' '[ "$target_file" != "$target_file2" ] || exit 1' - ], + ], 'sky local down', ) run_one_test(test) From 39cd385ca362b81dc14c9cd7c08e7a95ad0c292a Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Thu, 25 Jul 2024 17:56:26 -0700 Subject: [PATCH 05/12] Fix: Error handling and smoke test update --- sky/utils/log_utils.py | 26 +++++++++++++++++++------- tests/test_smoke.py | 2 +- 2 files changed, 20 insertions(+), 8 deletions(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index 04ec07a3a80..4527c4063ec 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -196,21 +196,33 @@ def readable_time_duration(start: Optional[float], def create_and_symlink_log_dir(log_dir: str): - """Creates a new log directory and symlinks to it from parent directory. + """Create a log directory and symlink to it from parent directory. + + If file operations fail, will log a warning and return without error. Args: log_dir (str): Expanded log directory path. """ - if log_dir == os.devnull: + try: + os.makedirs(log_dir, exist_ok=True) + except OSError: + logger.warning(f'Failed to create log directory at {log_dir!r}.' + 'Check if there is a file with the same name.') return - os.makedirs(log_dir, exist_ok=True) - symlink_path = os.path.join(log_dir, os.pardir, constants.SKY_LATEST_LINK) + symlink_path = os.path.join(os.path.dirname(os.path.abspath(log_dir)), + constants.SKY_LATEST_LINK) if os.path.exists(symlink_path): if os.path.islink(symlink_path): - os.remove(symlink_path) + try: + os.remove(symlink_path) + except OSError: + return else: logger.warning( - (f'Failed to symlink to latest logs at \'{symlink_path}\'.' + (f'Failed to symlink to latest logs at {symlink_path!r}.' 'Please remove the existing file/directory.')) return - os.symlink(log_dir, symlink_path) + try: + os.symlink(log_dir, symlink_path) + except OSError: + return diff --git a/tests/test_smoke.py b/tests/test_smoke.py index e038bf51e0b..1bdf07e3d47 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -1322,7 +1322,7 @@ def test_symlink_latest_logs(): test = Test( 'symlink_latest_logs', [ - 'sky local up || true', + 'sky launch -c test2 --cloud kubernetes -- echo hi || true', 'eval symlink_path="~/sky_logs/sky_latest"; [ -L "$symlink_path" ] || exit 1 ;' 'target_file=$(readlink -f "$symlink_path") || exit 1 ;' 'sky local down || true ;' From 384d3d85fde7b30ca7c40be3c6f68bf5c7b0def6 Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Thu, 25 Jul 2024 19:13:17 -0700 Subject: [PATCH 06/12] fix: error if makedirs fails and update smoke test --- sky/utils/log_utils.py | 7 +------ tests/test_smoke.py | 10 ++++++---- 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index 4527c4063ec..ac48e940aae 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -203,12 +203,7 @@ def create_and_symlink_log_dir(log_dir: str): Args: log_dir (str): Expanded log directory path. """ - try: - os.makedirs(log_dir, exist_ok=True) - except OSError: - logger.warning(f'Failed to create log directory at {log_dir!r}.' - 'Check if there is a file with the same name.') - return + os.makedirs(log_dir, exist_ok=True) symlink_path = os.path.join(os.path.dirname(os.path.abspath(log_dir)), constants.SKY_LATEST_LINK) if os.path.exists(symlink_path): diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 1bdf07e3d47..4377593c560 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -1322,15 +1322,17 @@ def test_symlink_latest_logs(): test = Test( 'symlink_latest_logs', [ - 'sky launch -c test2 --cloud kubernetes -- echo hi || true', + 'sky launch -c test1 --cloud kubernetes -- echo hi > log1.txt || true', 'eval symlink_path="~/sky_logs/sky_latest"; [ -L "$symlink_path" ] || exit 1 ;' 'target_file=$(readlink -f "$symlink_path") || exit 1 ;' - 'sky local down || true ;' + 'grep -E "To view detailed progress: .+ $target_file" log1.txt || exit 1;' + 'sky launch -c test1 --cloud kubernetes -- echo hi > log2.txt || true ;' '[ -L "$symlink_path" ] || exit 1 ;' - 'target_file2=$(readlink -f "$symlink_path") ;' + 'target_file2=$(readlink -f "$symlink_path") || exit 1 ;' + 'grep -E "To view detailed progress: .+ $target_file2" log2.txt || exit 1;' '[ "$target_file" != "$target_file2" ] || exit 1' ], - 'sky local down', + 'rm log1.txt log2.txt' ) run_one_test(test) From c2d6fd7e4ed3712e91024f485b32390ba9ef9f78 Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Thu, 25 Jul 2024 19:19:11 -0700 Subject: [PATCH 07/12] fix formatting --- tests/test_smoke.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index 4377593c560..f29ae039356 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -1332,7 +1332,7 @@ def test_symlink_latest_logs(): 'grep -E "To view detailed progress: .+ $target_file2" log2.txt || exit 1;' '[ "$target_file" != "$target_file2" ] || exit 1' ], - 'rm log1.txt log2.txt' + 'rm log1.txt log2.txt', ) run_one_test(test) From ba3f1649cd44622372850691ac02037cf84d859b Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Thu, 25 Jul 2024 23:18:43 -0700 Subject: [PATCH 08/12] add logging and generify smoke test --- sky/utils/log_utils.py | 6 ++++-- tests/test_smoke.py | 10 +++++----- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index ac48e940aae..ea0afa96a64 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -211,11 +211,13 @@ def create_and_symlink_log_dir(log_dir: str): try: os.remove(symlink_path) except OSError: + logger.warning('Failed to remove old symlink to latest logs' + f'at {symlink_path!r}.') return else: logger.warning( - (f'Failed to symlink to latest logs at {symlink_path!r}.' - 'Please remove the existing file/directory.')) + f'Failed to symlink to latest logs at {symlink_path!r}.' + 'Please remove the existing file/directory.') return try: os.symlink(log_dir, symlink_path) diff --git a/tests/test_smoke.py b/tests/test_smoke.py index f29ae039356..ea236193556 100644 --- a/tests/test_smoke.py +++ b/tests/test_smoke.py @@ -1317,22 +1317,22 @@ def test_scp_logs(): run_one_test(test) -@pytest.mark.kubernetes -def test_symlink_latest_logs(): +def test_symlink_latest_logs(generic_cloud: str): + name = _get_cluster_name() test = Test( 'symlink_latest_logs', [ - 'sky launch -c test1 --cloud kubernetes -- echo hi > log1.txt || true', + f'sky launch -y -c {name} --cloud {generic_cloud} -- echo hi > log1.txt || true', 'eval symlink_path="~/sky_logs/sky_latest"; [ -L "$symlink_path" ] || exit 1 ;' 'target_file=$(readlink -f "$symlink_path") || exit 1 ;' 'grep -E "To view detailed progress: .+ $target_file" log1.txt || exit 1;' - 'sky launch -c test1 --cloud kubernetes -- echo hi > log2.txt || true ;' + f'sky launch -y -c {name} --cloud {generic_cloud} -- echo hi > log2.txt || true ;' '[ -L "$symlink_path" ] || exit 1 ;' 'target_file2=$(readlink -f "$symlink_path") || exit 1 ;' 'grep -E "To view detailed progress: .+ $target_file2" log2.txt || exit 1;' '[ "$target_file" != "$target_file2" ] || exit 1' ], - 'rm log1.txt log2.txt', + 'rm log1.txt log2.txt 2> /dev/null || true', ) run_one_test(test) From 35002d3b9712968e8087b600efa585bb2d3c02bb Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Thu, 25 Jul 2024 23:24:49 -0700 Subject: [PATCH 09/12] fix whitespace --- sky/utils/log_utils.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index ea0afa96a64..51c4b10f99f 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -212,7 +212,7 @@ def create_and_symlink_log_dir(log_dir: str): os.remove(symlink_path) except OSError: logger.warning('Failed to remove old symlink to latest logs' - f'at {symlink_path!r}.') + f' at {symlink_path!r}.') return else: logger.warning( From 221b194f8a814d94556b78d8eaed974862c02b8d Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Sat, 27 Jul 2024 10:22:34 -0700 Subject: [PATCH 10/12] add warning for failed symlink --- sky/utils/log_utils.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index 51c4b10f99f..0b08eaa19ca 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -222,4 +222,7 @@ def create_and_symlink_log_dir(log_dir: str): try: os.symlink(log_dir, symlink_path) except OSError: + # No warning if logging to /dev/null + if log_dir != os.path.dirname(os.devnull): + logger.warning(f'Failed to symlink to latest logs at {symlink_path!r}.') return From 1410a8db1a547c62f36cdc7853d6b5defd7bce2f Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Sat, 27 Jul 2024 10:24:53 -0700 Subject: [PATCH 11/12] fix formatting (again) --- sky/utils/log_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index 0b08eaa19ca..0a2943ee239 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -224,5 +224,6 @@ def create_and_symlink_log_dir(log_dir: str): except OSError: # No warning if logging to /dev/null if log_dir != os.path.dirname(os.devnull): - logger.warning(f'Failed to symlink to latest logs at {symlink_path!r}.') + logger.warning( + f'Failed to symlink to latest logs at {symlink_path!r}.') return From c441905ba297ccbf8317c1890e618831002c5083 Mon Sep 17 00:00:00 2001 From: Shreyas Kallingal Date: Sat, 27 Jul 2024 20:14:17 -0700 Subject: [PATCH 12/12] fix TOCTTU bug --- sky/utils/log_utils.py | 54 +++++++++++++++++++++++++++++------------- 1 file changed, 37 insertions(+), 17 deletions(-) diff --git a/sky/utils/log_utils.py b/sky/utils/log_utils.py index 0a2943ee239..70cc3c10671 100644 --- a/sky/utils/log_utils.py +++ b/sky/utils/log_utils.py @@ -4,6 +4,7 @@ from typing import List, Optional import colorama +import filelock import pendulum import prettytable @@ -13,6 +14,11 @@ logger = sky_logging.init_logger(__name__) +# Filelock for updating log directory symlink. +_SYMLINK_LOCK_PATH = os.path.expanduser( + os.path.join(constants.SKY_LOGS_DIRECTORY, '.symlink_latest.lock')) +_SYMLINK_LOCK_TIMEOUT_SECONDS = 10 + class LineProcessor(object): """A processor for log lines.""" @@ -203,27 +209,41 @@ def create_and_symlink_log_dir(log_dir: str): Args: log_dir (str): Expanded log directory path. """ + # Fast path for logging to /dev/null + if log_dir == os.path.dirname(os.devnull): + return os.makedirs(log_dir, exist_ok=True) symlink_path = os.path.join(os.path.dirname(os.path.abspath(log_dir)), constants.SKY_LATEST_LINK) - if os.path.exists(symlink_path): - if os.path.islink(symlink_path): + try: + with filelock.FileLock(_SYMLINK_LOCK_PATH, + _SYMLINK_LOCK_TIMEOUT_SECONDS): + if os.path.exists(symlink_path): + if os.path.islink(symlink_path): + try: + os.remove(symlink_path) + except FileNotFoundError: + pass + except OSError: + logger.warning( + 'Failed to remove old symlink to latest logs ' + f'at {symlink_path!r}.') + return + else: + logger.warning( + f'Failed to symlink to latest logs at {symlink_path!r}.' + 'Please remove the existing file/directory.') + return try: - os.remove(symlink_path) + os.symlink(log_dir, symlink_path) except OSError: - logger.warning('Failed to remove old symlink to latest logs' - f' at {symlink_path!r}.') + logger.warning(f'Failed to symlink to latest logs from ' + f'{symlink_path!r} to {log_dir!r}.') return - else: - logger.warning( - f'Failed to symlink to latest logs at {symlink_path!r}.' - 'Please remove the existing file/directory.') - return - try: - os.symlink(log_dir, symlink_path) - except OSError: - # No warning if logging to /dev/null - if log_dir != os.path.dirname(os.devnull): - logger.warning( - f'Failed to symlink to latest logs at {symlink_path!r}.') + except filelock.Timeout: + logger.warning( + f'Failed to symlink to latest logs at {symlink_path!r} ' + 'due to a timeout when trying to access the log directory lock. ' + 'Please try again or manually remove the lock ' + f'at {_SYMLINK_LOCK_PATH!r}.') return