requires root privileges and touches /var during build

Atomic/util.py

@@ -33,8 +33,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"
@@ -805,32 +803,32 @@ def load_scan_result_file(file_name):
 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
-        while time_out < 10.5: # Ten second attempt to get a lock
-            try:
-                fcntl.flock(install_data_file, fcntl.LOCK_EX | fcntl.LOCK_NB)
-                f_lock = True
-                break
-            except IOError:
-                time.sleep(.5)
-                time_out += .5
+        # Create the temporary lockfile if it doesn't exist
+        if not os.path.exists(lock_file_name):
+            open(lock_file_name, 'a').close()
+
+        with open(lock_file_name, "r") as f:
+            while time_out < 10.5: # Ten second attempt to get a lock
+                try:
+                    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)
+        with open(lock_file_name, "r") as f:
+            fcntl.flock(f, fcntl.LOCK_UN)
 
     def wrapper(*args, **kwargs):
         get_lock()
@@ -841,34 +839,40 @@ def wrapper(*args, **kwargs):
     return wrapper
 
 
+# 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):
+    @classmethod
+    def _read_install_data(cls):
         try:
-            return json.loads(file_handle.read())
-        except ValueError:
-            return {}
+            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
     def _write_install_data(cls, new_data):
-        install_data = cls._read_install_data(cls.install_file_handle)
+        install_data = cls._read_install_data()
         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
     @file_lock
     def read_install_data(cls):
         if os.path.exists(ATOMIC_INSTALL_JSON):
-            read_data = cls._read_install_data(cls.install_file_handle)
+            read_data = cls._read_install_data()
             return read_data
         return {}
 
@@ -880,7 +884,7 @@ def write_install_data(cls, new_data):
     @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
@@ -889,7 +893,7 @@ def get_install_name_by_id(cls, iid, install_data=None):
     @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: