diff --git a/Atomic/util.py b/Atomic/util.py index fff87179..7cd9dc60 100644 --- a/Atomic/util.py +++ b/Atomic/util.py @@ -6,6 +6,7 @@ import json import subprocess import collections +from contextlib import contextmanager from fnmatch import fnmatch as matches import os import selinux @@ -33,8 +34,6 @@ ATOMIC_CONFD = os.environ.get('ATOMIC_CONFD', '/etc/atomic.d/') ATOMIC_LIBEXEC = os.environ.get('ATOMIC_LIBEXEC', '/usr/libexec/atomic') ATOMIC_VAR_LIB = os.environ.get('ATOMIC_VAR_LIB', '/var/lib/atomic') -if not os.path.exists(ATOMIC_VAR_LIB): - os.makedirs(ATOMIC_VAR_LIB) ATOMIC_INSTALL_JSON = os.environ.get('ATOMIC_INSTALL_JSON', os.path.join(ATOMIC_VAR_LIB, 'install.json')) GOMTREE_PATH = "/usr/bin/gomtree" @@ -802,94 +801,71 @@ def load_scan_result_file(file_name): return json.loads(open(os.path.join(file_name), "r").read()) -def file_lock(func): - lock_file_name = "{}.lock".format(os.path.join(os.path.dirname(ATOMIC_INSTALL_JSON), "." + os.path.basename(ATOMIC_INSTALL_JSON))) - - # Create the temporary lockfile if it doesn't exist - if not os.path.exists(lock_file_name): - open(lock_file_name, 'a').close() - - install_data_file = open(lock_file_name, "r") - - def get_lock(): - ''' - Obtains a read-only file lock on the install data - :return: - ''' - time_out = 0 - f_lock = False +@contextmanager +def file_lock(path): + lock_file_name = "{}.lock".format(path) + time_out = 0 + f_lock = False + with open(lock_file_name, "a") as f: while time_out < 10.5: # Ten second attempt to get a lock try: - fcntl.flock(install_data_file, fcntl.LOCK_EX | fcntl.LOCK_NB) + fcntl.flock(f, fcntl.LOCK_EX | fcntl.LOCK_NB) f_lock = True break except IOError: time.sleep(.5) time_out += .5 if not f_lock: - raise ValueError("Unable to get file lock for {}".format(ATOMIC_INSTALL_JSON)) - - def release_lock(): - fcntl.flock(install_data_file, fcntl.LOCK_UN) - - def wrapper(*args, **kwargs): - get_lock() - ret = func(*args, **kwargs) - release_lock() - return ret - - return wrapper + raise ValueError("Unable to get file lock for {}".format(lock_file_name)) + # Call the user code + yield + # Now unlock + fcntl.flock(f, fcntl.LOCK_UN) +# Records additional data for containers outside of the native storage (docker/ostree) class InstallData(object): - if not os.path.exists(ATOMIC_INSTALL_JSON): - open(ATOMIC_INSTALL_JSON, 'a').close() - - install_file_handle = open(ATOMIC_INSTALL_JSON, 'r') - - @staticmethod - def _read_install_data(file_handle): - try: - return json.loads(file_handle.read()) - except ValueError: - return {} - - @classmethod - def _write_install_data(cls, new_data): - install_data = cls._read_install_data(cls.install_file_handle) - for x in new_data: - install_data[x] = new_data[x] - temp_file = tempfile.NamedTemporaryFile(mode='w', delete=False) - json.dump(install_data, temp_file) - temp_file.close() - shutil.move(temp_file.name, ATOMIC_INSTALL_JSON) @classmethod - @file_lock def read_install_data(cls): - if os.path.exists(ATOMIC_INSTALL_JSON): - read_data = cls._read_install_data(cls.install_file_handle) - return read_data - return {} + with file_lock(ATOMIC_INSTALL_JSON): + try: + with open(ATOMIC_INSTALL_JSON, 'r') as f: + # Backwards compatibilty - we previously created an empty file explicitly; + # see https://github.com/projectatomic/atomic/pull/966 + if os.fstat(f.fileno()).st_size == 0: + return {} + return json.load(f) + except IOError as e: + if e.errno == errno.ENOENT: + return {} + raise e @classmethod - @file_lock def write_install_data(cls, new_data): - cls._write_install_data(new_data) + install_data = cls.read_install_data() + with file_lock(ATOMIC_INSTALL_JSON): + for x in new_data: + install_data[x] = new_data[x] + temp_file = tempfile.NamedTemporaryFile(mode='w', delete=False) + json.dump(install_data, temp_file) + temp_file.close() + if not os.path.exists(ATOMIC_VAR_LIB): + os.makedirs(ATOMIC_VAR_LIB) + shutil.move(temp_file.name, ATOMIC_INSTALL_JSON) @classmethod def get_install_name_by_id(cls, iid, install_data=None): if not install_data: - install_data = cls._read_install_data(cls.install_file_handle) + install_data = cls.read_install_data() for installed_image in install_data: if install_data[installed_image]['id'] == iid: return installed_image raise ValueError("Unable to find {} in installed image data ({}). Re-run command with -i to ignore".format(id, ATOMIC_INSTALL_JSON)) @classmethod - @file_lock def delete_by_id(cls, iid, ignore=False): - install_data = cls._read_install_data(cls.install_file_handle) + install_data = cls.read_install_data() try: id_key = InstallData.get_install_name_by_id(iid, install_data=install_data) except ValueError as e: @@ -897,7 +873,7 @@ def delete_by_id(cls, iid, ignore=False): raise ValueError(str(e)) return del install_data[id_key] - return cls._write_install_data(install_data) + return cls.write_install_data(install_data) @classmethod def image_installed(cls, img_object): diff --git a/tests/unit/test_util.py b/tests/unit/test_util.py index ea40a8b7..ad8126df 100644 --- a/tests/unit/test_util.py +++ b/tests/unit/test_util.py @@ -170,14 +170,12 @@ def __init__(self): self.image = None @patch('Atomic.util.InstallData.read_install_data', new=MockIO.read_mock) - @patch('Atomic.util.InstallData._read_install_data', new=MockIO.read_mock) @patch('Atomic.util.InstallData.write_install_data', new=MockIO.write_mock) def test_read(self): MockIO.reset_data() self.assertEqual(util.InstallData.read_install_data(), MockIO.install_data) @patch('Atomic.util.InstallData.read_install_data', new=MockIO.read_mock) - @patch('Atomic.util.InstallData._read_install_data', new=MockIO.read_mock) @patch('Atomic.util.InstallData.write_install_data', new=MockIO.write_mock) def test_write(self): MockIO.reset_data() @@ -187,7 +185,6 @@ def test_write(self): self.assertTrue('docker.io/library/centos:latest' in util.InstallData.read_install_data()) @patch('Atomic.util.InstallData.read_install_data', new=MockIO.read_mock) - @patch('Atomic.util.InstallData._read_install_data', new=MockIO.read_mock) @patch('Atomic.util.InstallData.write_install_data', new=MockIO.write_mock) def test_get_install_name_by_id(self): MockIO.reset_data() @@ -195,14 +192,12 @@ def test_get_install_name_by_id(self): self.assertEqual(util.InstallData.get_install_name_by_id('16e9fdecc1febc87fb1ca09271009cf5f28eb8d4aec5515922ef298c145a6726', install_data=MockIO.install_data), 'docker.io/library/centos:latest') @patch('Atomic.util.InstallData.read_install_data', new=MockIO.read_mock) - @patch('Atomic.util.InstallData._read_install_data', new=MockIO.read_mock) @patch('Atomic.util.InstallData.write_install_data', new=MockIO.write_mock) def test_fail_get_install_name_by_id(self): MockIO.reset_data() self.assertRaises(ValueError, util.InstallData.get_install_name_by_id, 1, MockIO.install_data) @patch('Atomic.util.InstallData.read_install_data', new=MockIO.read_mock) - @patch('Atomic.util.InstallData._read_install_data', new=MockIO.read_mock) @patch('Atomic.util.InstallData.write_install_data', new=MockIO.write_mock) def test_image_installed_name(self): MockIO.reset_data() @@ -216,7 +211,6 @@ def test_image_installed_name(self): self.assertTrue(util.InstallData.image_installed(local_image_object)) @patch('Atomic.util.InstallData.read_install_data', new=MockIO.read_mock) - @patch('Atomic.util.InstallData._read_install_data', new=MockIO.read_mock) @patch('Atomic.util.InstallData.write_install_data', new=MockIO.write_mock) def test_image_installed_id(self): MockIO.reset_data() @@ -230,7 +224,6 @@ def test_image_installed_id(self): self.assertTrue(util.InstallData.image_installed(local_image_object)) @patch('Atomic.util.InstallData.read_install_data', new=MockIO.read_mock) - @patch('Atomic.util.InstallData._read_install_data', new=MockIO.read_mock) @patch('Atomic.util.InstallData.write_install_data', new=MockIO.write_mock) def test_image_not_installed(self): MockIO.reset_data()