From 63db8ee0a44a72635dc85ca7d8365d8bb473d6ba Mon Sep 17 00:00:00 2001 From: Francesco De Martino Date: Wed, 29 Jun 2022 14:47:57 +0200 Subject: [PATCH] Remove usage of temporary file to read computemgtd config Signed-off-by: Francesco De Martino --- CHANGELOG.md | 1 + src/slurm_plugin/computemgtd.py | 12 +++++------- tests/slurm_plugin/test_computemgtd.py | 3 +-- 3 files changed, 7 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 420a14148..e4def2e39 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ This file is used to list changes made in each version of the aws-parallelcluste **BUG FIXES** - Handle corner case in the scaling logic when instance is just launched and the describe instances API doesn't report yet all the EC2 info. +- Fix file handle leak in `computemgtd`. 3.1.4 ------ diff --git a/src/slurm_plugin/computemgtd.py b/src/slurm_plugin/computemgtd.py index 460a37158..dadbee287 100644 --- a/src/slurm_plugin/computemgtd.py +++ b/src/slurm_plugin/computemgtd.py @@ -15,14 +15,14 @@ import time from configparser import ConfigParser from datetime import datetime, timezone +from io import StringIO from logging.config import fileConfig from subprocess import CalledProcessError -from tempfile import NamedTemporaryFile from botocore.config import Config from common.schedulers.slurm_commands import get_nodes_info from common.time_utils import seconds -from common.utils import run_command, sleep_remaining_loop_time +from common.utils import check_command_output, run_command, sleep_remaining_loop_time from retrying import retry from slurm_plugin.common import ( DEFAULT_COMMAND_TIMEOUT, @@ -55,8 +55,6 @@ class ComputemgtdConfig: } def __init__(self, config_file_path): - tf = NamedTemporaryFile() - self._local_config_file = tf.name self._get_config(config_file_path) def __repr__(self): @@ -70,12 +68,12 @@ def _get_config(self, config_file_path): config = ConfigParser() try: # Use subprocess based method to copy shared file to local to prevent hanging when NFS is down - run_command( - f"cat {config_file_path} > {self._local_config_file}", + config_str = check_command_output( + f"cat {config_file_path}", timeout=DEFAULT_COMMAND_TIMEOUT, shell=True, # nosec ) - config.read_file(open(self._local_config_file, "r")) + config.read_file(StringIO(config_str)) except Exception: log.error(f"Cannot read computemgtd configuration file: {config_file_path}") raise diff --git a/tests/slurm_plugin/test_computemgtd.py b/tests/slurm_plugin/test_computemgtd.py index 89819496c..f68dc03e8 100644 --- a/tests/slurm_plugin/test_computemgtd.py +++ b/tests/slurm_plugin/test_computemgtd.py @@ -62,8 +62,7 @@ ) def test_computemgtd_config(config_file, expected_attributes, test_datadir, mocker): mocker.patch("slurm_plugin.computemgtd.ComputemgtdConfig._read_nodename_from_file", return_value="some_nodename") - mocker.patch("slurm_plugin.computemgtd.run_command") - mocker.patch("slurm_plugin.computemgtd.open", return_value=open(test_datadir / config_file, "r")) + mocker.patch("slurm_plugin.computemgtd.check_command_output", return_value=(test_datadir / config_file).read_text()) compute_config = ComputemgtdConfig("mocked_config_path") for key in expected_attributes: assert_that(compute_config.__dict__.get(key)).is_equal_to(expected_attributes.get(key))