From 9b80b3efbaadb829c63923d1cdb45af3a0890f2c Mon Sep 17 00:00:00 2001 From: Brian Lin Date: Mon, 20 Mar 2023 14:51:39 -0500 Subject: [PATCH 1/3] Prep for sandbox refactoring with unit tests (SOFTWARE-5531) --- test/test_sandbox_mgmt.py | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 test/test_sandbox_mgmt.py diff --git a/test/test_sandbox_mgmt.py b/test/test_sandbox_mgmt.py new file mode 100644 index 00000000..26657557 --- /dev/null +++ b/test/test_sandbox_mgmt.py @@ -0,0 +1,33 @@ +#!/bin/env python + +import os +import unittest +from unittest.mock import patch, PropertyMock + +from common.gratia.common import sandbox_mgmt + +class SandboxMgmtTests(unittest.TestCase): + + @patch('gratia.common.config.ConfigProxy.get_GratiaExtension', create=True, return_value='test-extension') + def test_GenerateFilename(self, mock_config): + """GenerateFilename creates a temporary file and returns the path to the file + """ + prefix = 'test-prefix.' + temp_dir = '/tmp' + + try: + filename = sandbox_mgmt.GenerateFilename(prefix, temp_dir) + self.assertTrue(os.path.exists(filename), + f'Failed to create temporary file ({filename})') + self.assertEqual(temp_dir.rstrip('/'), + os.path.dirname(filename), + f'Temporary file {filename} placed in the wrong directory') + self.assertRegex(filename, + rf'{temp_dir}/*{prefix}\d+\.{mock_config.return_value}__\w+', + 'Unexpected file name format') + finally: + try: + os.remove(filename) + except FileNotFoundError: + # don't need to clean up what's not there + pass From 5ff42702164649e2f2144ea5c9813b0c037534ae Mon Sep 17 00:00:00 2001 From: Brian Lin Date: Mon, 20 Mar 2023 18:18:12 -0500 Subject: [PATCH 2/3] First attempt at cleaning up the tempfile creation (SOFTWARE-5540) --- common/gratia/common/sandbox_mgmt.py | 26 ++++++++------------------ common/gratia/common/xml_utils.py | 6 ++---- test/test_sandbox_mgmt.py | 25 +++++++++++++------------ 3 files changed, 23 insertions(+), 34 deletions(-) diff --git a/common/gratia/common/sandbox_mgmt.py b/common/gratia/common/sandbox_mgmt.py index 90906a9f..c907dfdc 100644 --- a/common/gratia/common/sandbox_mgmt.py +++ b/common/gratia/common/sandbox_mgmt.py @@ -5,6 +5,7 @@ import glob import time import shutil +import tempfile import tarfile from gratia.common.config import ConfigProxy @@ -409,18 +410,8 @@ def SearchOutstandingRecord(): def GenerateFilename(prefix, current_dir): '''Generate a filename of the for current_dir/prefix.$pid.ConfigFragment.gratia.xml__Unique''' - filename = prefix + str(global_state.RecordPid) + '.' + Config.get_GratiaExtension() \ - + '__XXXXXXXXXX' - filename = os.path.join(current_dir, filename) - mktemp_pipe = os.popen('mktemp -q "' + filename + '"') - if mktemp_pipe != None: - filename = mktemp_pipe.readline() - mktemp_pipe.close() - filename = filename.strip() - if filename != r'': - return filename - - raise IOError + fn_prefix = f'{prefix}.{global_state.RecordPid}.{Config.get_GratiaExtension()}__' + return tempfile.NamedTemporaryFile(prefix=fn_prefix, dir=current_dir, delete=False, mode='w') def UncompressOutbox(staging_name, target_dir): @@ -599,12 +590,11 @@ def OpenNewRecordFile(dirIndex): raise InternalError(msg) from exc try: - filename = GenerateFilename('r.', working_dir) - DebugPrint(3, 'Creating file:', filename) - outstandingRecordCount += 1 - f = open(filename, 'w') - dirIndex = index - return (f, dirIndex) + with GenerateFilename('r', working_dir) as recordfile: + DebugPrint(3, 'Creating file:', recordfile.name) + outstandingRecordCount += 1 + dirIndex = index + return (recordfile, dirIndex) except Exception as exc: msg = 'ERROR: Caught exception while creating file' DebugPrint(0, msg + ': ', exc) diff --git a/common/gratia/common/xml_utils.py b/common/gratia/common/xml_utils.py index c7c74868..d4e74599 100644 --- a/common/gratia/common/xml_utils.py +++ b/common/gratia/common/xml_utils.py @@ -280,10 +280,8 @@ def UsageCheckXmldoc(xmlDoc, external, resourceType=None): subdir = os.path.join(Config.get_DataFolder(), "quarantine", 'subdir.' + Config.getFilenameFragment()) if not os.path.exists(subdir): os.mkdir(subdir) - fn = sandbox_mgmt.GenerateFilename("r.", subdir) - writer = open(fn, 'w') - usageRecord.writexml(writer) - writer.close() + with sandbox_mgmt.GenerateFilename("r", subdir) as writer: + usageRecord.writexml(writer) usageRecord.unlink() continue diff --git a/test/test_sandbox_mgmt.py b/test/test_sandbox_mgmt.py index 26657557..d270bc8f 100644 --- a/test/test_sandbox_mgmt.py +++ b/test/test_sandbox_mgmt.py @@ -12,22 +12,23 @@ class SandboxMgmtTests(unittest.TestCase): def test_GenerateFilename(self, mock_config): """GenerateFilename creates a temporary file and returns the path to the file """ - prefix = 'test-prefix.' + prefix = 'test-prefix' temp_dir = '/tmp' try: - filename = sandbox_mgmt.GenerateFilename(prefix, temp_dir) - self.assertTrue(os.path.exists(filename), - f'Failed to create temporary file ({filename})') - self.assertEqual(temp_dir.rstrip('/'), - os.path.dirname(filename), - f'Temporary file {filename} placed in the wrong directory') - self.assertRegex(filename, - rf'{temp_dir}/*{prefix}\d+\.{mock_config.return_value}__\w+', - 'Unexpected file name format') + with sandbox_mgmt.GenerateFilename(prefix, temp_dir) as filename: + self.assertTrue(os.path.exists(filename.name), + f'Failed to create temporary file ({filename.name})') + self.assertEqual(temp_dir.rstrip('/'), + os.path.dirname(filename.name), + f'Temporary file {filename.name} placed in the wrong directory') + self.assertRegex(filename.name, + rf'{temp_dir}/*{prefix}\.\d+\.{mock_config.return_value}__\w+', + 'Unexpected file name format') finally: try: - os.remove(filename) - except FileNotFoundError: + filename.close() + os.remove(filename.name) + except (FileNotFoundError, NameError): # don't need to clean up what's not there pass From 948ac520a73c32e9be5e44146e8e8321ad423682 Mon Sep 17 00:00:00 2001 From: Brian Lin Date: Fri, 24 Mar 2023 17:08:06 -0500 Subject: [PATCH 3/3] First stab at updating temp tarball creation (SOFTWARE-5531) --- common/gratia/common/sandbox_mgmt.py | 27 ++++++++++++++------------- 1 file changed, 14 insertions(+), 13 deletions(-) diff --git a/common/gratia/common/sandbox_mgmt.py b/common/gratia/common/sandbox_mgmt.py index c907dfdc..753c8218 100644 --- a/common/gratia/common/sandbox_mgmt.py +++ b/common/gratia/common/sandbox_mgmt.py @@ -478,20 +478,21 @@ def CompressOutbox(probe_dir, outbox, outfiles): DebugPrint(0, msg + ':' + exc) raise InternalError(msg) from exc - staging_name = GenerateFilename('tz.', staged_store) - DebugPrint(1, 'Compressing outbox in tar.bz2 file: ' + staging_name) + with GenerateFilename('tz', staged_store) as temp_tarfile: + staging_name = temp_tarfile.name + DebugPrint(1, 'Compressing outbox in tar.bz2 file: ' + staging_name) - try: - tar = tarfile.open(staging_name, 'w:bz2') - except KeyboardInterrupt: - raise - except SystemExit: - raise - except Exception as e: - DebugPrint(0, 'Warning: Exception caught while opening tar.bz2 file: ' + staging_name + ':') - DebugPrint(0, 'Caught exception: ', e) - DebugPrintTraceback() - return False + try: + tar = tarfile.open(staging_name, 'w:bz2') + except KeyboardInterrupt: + raise + except SystemExit: + raise + except Exception as e: + DebugPrint(0, 'Warning: Exception caught while opening tar.bz2 file: ' + staging_name + ':') + DebugPrint(0, 'Caught exception: ', e) + DebugPrintTraceback() + return False try: for f in outfiles: