Fix for race condition for parallel multi-level backup restore

Change-Id: Ibf4840e9e1dafce57b54d3bb2d2bed8fe5666c5b
Resolves: bug 1500961

freezer/arguments.py

@@ -25,13 +25,13 @@ try:
     import configparser
 except ImportError:
     import ConfigParser as configparser
-from distutils import spawn as distspawn
 import logging
 import os
 from os.path import expanduser
 import socket
 import sys
 import utils
+from distutils import spawn as distspawn
 
 from oslo_utils import encodeutils
 
@@ -448,18 +448,10 @@ def backup_arguments(args_dict={}):
     arg_parser.set_defaults(**defaults)
     backup_args = arg_parser.parse_args()
 
-    # windows bin
-    path_to_binaries = os.path.dirname(os.path.abspath(__file__))
-
     # Intercept command line arguments if you are not using the CLIvss
     if args_dict:
         backup_args.__dict__.update(args_dict)
 
-    # Set additional namespace attributes
-    backup_args.__dict__['remote_match_backup'] = []
-    backup_args.__dict__['remote_obj_list'] = []
-    backup_args.__dict__['remote_newest_backup'] = u''
-
     # Set default working directory to ~/.freezer. If the directory
     # does not exists it is created
     work_dir = os.path.join(home, '.freezer')
@@ -488,47 +480,11 @@ def backup_arguments(args_dict={}):
     # If hostname is not set, hostname of the current node will be used
     if not backup_args.hostname:
         backup_args.__dict__['hostname'] = socket.gethostname()
-    backup_args.__dict__['manifest_meta_dict'] = {}
-    backup_args.__dict__['curr_backup_level'] = ''
-    backup_args.__dict__['manifest_meta_dict'] = ''
-    if winutils.is_windows():
-        backup_args.__dict__['tar_path'] = '{0}\\bin\\tar.exe'. \
-            format(path_to_binaries)
-    else:
-        backup_args.__dict__['tar_path'] = distspawn.find_executable('tar')
-    # If freezer is being used under OSX, please install gnutar and
-    # rename the executable as gnutar
-    if 'darwin' in sys.platform or 'bsd' in sys.platform:
-        if distspawn.find_executable('gtar'):
-            backup_args.__dict__['tar_path'] = \
-                distspawn.find_executable('gtar')
-        elif distspawn.find_executable('gnutar'):
-            backup_args.__dict__['tar_path'] = \
-                distspawn.find_executable('gnutar')
-        else:
-            raise Exception('Please install gnu tar (gtar) as it is a '
-                            'mandatory requirement to use freezer.')
 
     # If we have provided --proxy then overwrite the system HTTP_PROXY and
     # HTTPS_PROXY
     alter_proxy(backup_args.__dict__)
 
-    # Get absolute path of other commands used by freezer
-    backup_args.__dict__['lvcreate_path'] = distspawn.find_executable(
-        'lvcreate')
-    backup_args.__dict__['lvremove_path'] = distspawn.find_executable(
-        'lvremove')
-    backup_args.__dict__['bash_path'] = distspawn.find_executable('bash')
-    if winutils.is_windows():
-        backup_args.__dict__['openssl_path'] = 'openssl'
-    else:
-        backup_args.__dict__['openssl_path'] = \
-            distspawn.find_executable('openssl')
-    backup_args.__dict__['file_path'] = distspawn.find_executable('file')
-    backup_args.__dict__['mount_path'] = distspawn.find_executable('mount')
-    backup_args.__dict__['umount_path'] = distspawn.find_executable('umount')
-    backup_args.__dict__['ionice'] = distspawn.find_executable('ionice')
-
     # MySQLdb object
     backup_args.__dict__['mysql_db_inst'] = ''
 
@@ -547,10 +503,6 @@ def backup_arguments(args_dict={}):
         if backup_args.vssadmin == 'False' or backup_args.vssadmin == 'false':
             backup_args.vssadmin = False
 
-    backup_args.__dict__['meta_data'] = {}
-    backup_args.__dict__['meta_data_file'] = ''
-    backup_args.__dict__['absolute_path'] = ''
-
     # Freezer version
     backup_args.__dict__['__version__'] = '1.1.3'
 

freezer/engine/engine.py

@@ -21,6 +21,8 @@ Hudson (tjh@cryptsoft.com).
 Freezer general utils functions
 """
 import logging
+import multiprocessing
+import time
 
 from freezer import streaming
 from freezer import utils
@@ -57,8 +59,6 @@ class BackupEngine(object):
             Restore stream is a consumer, that is actually does restore (for
             tar it is a thread that creates gnutar subprocess and feeds chunks
             to stdin of this thread.
-
-    author: Eldar Nugaev
     """
     @property
     def main_storage(self):
@@ -69,7 +69,7 @@ class BackupEngine(object):
 
         PS. Should be changed to select the most up-to-date storage from
         existing ones
-        :rtype: freezer.storage.Storage
+        :rtype: freezer.storage.storage.Storage
         :return:
         """
         raise NotImplementedError("Should have implemented this")
@@ -103,6 +103,23 @@ class BackupEngine(object):
         """
         raise NotImplementedError("Should have implemented this")
 
+    def read_blocks(self, backup, write_pipe, read_pipe):
+        # Close the read pipe in this child as it is unneeded
+        # and download the objects from swift in chunks. The
+        # Chunk size is set by RESP_CHUNK_SIZE and sent to che write
+        # pipe
+        read_pipe.close()
+        for block in self.main_storage.backup_blocks(backup):
+            write_pipe.send_bytes(block)
+
+        # Closing the pipe after checking no data
+        # is still available in the pipe.
+        while True:
+            if not write_pipe.poll():
+                write_pipe.close()
+                break
+            time.sleep(1)
+
     def restore(self, backup, restore_path):
         """
         :type backup: freezer.storage.Backup
@@ -113,13 +130,34 @@ class BackupEngine(object):
         for level in range(0, backup.level + 1):
             b = backup.full_backup.increments[level]
             logging.info("Restore backup {0}".format(b))
-            streaming.stream(
-                self.main_storage.read_backup, {"backup": b},
-                self.restore_stream, {"restore_path": restore_path})
+            read_pipe, write_pipe = multiprocessing.Pipe()
+            process_stream = multiprocessing.Process(
+                target=self.read_blocks,
+                args=(b, write_pipe, read_pipe))
+            process_stream.daemon = True
+            process_stream.start()
+            write_pipe.close()
+
+            # Start the tar pipe consumer process
+            tar_stream = multiprocessing.Process(
+                target=self.restore_level, args=(restore_path, read_pipe))
+            tar_stream.daemon = True
+            tar_stream.start()
+            read_pipe.close()
+            write_pipe.close()
+            process_stream.join()
+            tar_stream.join()
+
+            if tar_stream.exitcode:
+                raise Exception('failed to restore file')
+
         logging.info(
             '[*] Restore execution successfully executed \
              for backup name {0}'.format(backup))
 
+    def restore_level(self, restore_path, read_pipe):
+        raise NotImplementedError("Should have implemented this")
+
     def backup_data(self, backup_path, manifest_path):
         """
         :param backup_path:
@@ -127,13 +165,3 @@ class BackupEngine(object):
         :return:
         """
         raise NotImplementedError("Should have implemented this")
-
-    def restore_stream(self, restore_path, rich_queue):
-        """
-        :param restore_path:
-        :type restore_path: str
-        :param rich_queue:
-        :type rich_queue: freezer.streaming.RichQueue
-        :return:
-        """
-        raise NotImplementedError("Should have implemented this")

freezer/engine/tar/tar_builders.py

@@ -20,6 +20,7 @@ Hudson (tjh@cryptsoft.com).
 
 Freezer Tar related functions
 """
+from freezer import utils
 
 
 class TarCommandBuilder:
@@ -43,8 +44,8 @@ class TarCommandBuilder:
                         'hard': '--hard-dereference',
                         'all': '--hard-dereference --dereference'}
 
-    def __init__(self, gnutar_path, filepath, compression_algo, is_windows):
-        self.gnutar_path = gnutar_path
+    def __init__(self, filepath, compression_algo, is_windows, tar_path=None):
+        self.tar_path = tar_path or utils.tar_path()
         self.dereference = ''
         self.listed_incremental = None
         self.exclude = ''
@@ -71,17 +72,17 @@ class TarCommandBuilder:
         """
         self.dereference = self.DEREFERENCE_MODE[mode]
 
-    def set_encryption(self, openssl_path, encrypt_pass_file):
-        self.openssl_path = openssl_path
+    def set_encryption(self, encrypt_pass_file, openssl_path=None):
+        self.openssl_path = openssl_path or utils.openssl_path()
         self.encrypt_pass_file = encrypt_pass_file
 
     def build(self):
         if self.is_windows:
             tar_command = self.WINDOWS_TEMPLATE.format(
-                gnutar_path=self.gnutar_path, algo=self.compression_algo)
+                gnutar_path=self.tar_path, algo=self.compression_algo)
         else:
             tar_command = self.UNIX_TEMPLATE.format(
-                gnutar_path=self.gnutar_path, algo=self.compression_algo)
+                gnutar_path=self.tar_path, algo=self.compression_algo)
 
         if self.dereference:
             tar_command = "{0} {1}".format(tar_command, self.dereference)
@@ -114,12 +115,13 @@ class TarCommandRestoreBuilder:
     UNIX_TEMPLATE = '{0} {1} --incremental --extract --unlink-first ' \
         '--ignore-zeros --warning=none --overwrite --directory {2}'
 
-    def __init__(self, tar_path, restore_path, compression_algo, is_windows):
+    def __init__(self, restore_path, compression_algo, is_windows,
+                 tar_path=None):
         self.dry_run = False
         self.is_windows = False
         self.openssl_path = None
         self.encrypt_pass_file = None
-        self.tar_path = tar_path
+        self.tar_path = tar_path or utils.tar_path()
         self.restore_path = restore_path
         self.compression_algo = get_tar_flag_from_algo(compression_algo)
         self.is_windows = is_windows
@@ -127,8 +129,8 @@ class TarCommandRestoreBuilder:
     def set_dry_run(self):
         self.dry_run = True
 
-    def set_encryption(self, openssl_path, encrypt_pass_file):
-        self.openssl_path = openssl_path
+    def set_encryption(self, encrypt_pass_file, openssl_path=None):
+        self.openssl_path = openssl_path or utils.openssl_path()
         self.encrypt_pass_file = encrypt_pass_file
 
     def build(self):

freezer/engine/tar/tar_engine.py

@@ -23,25 +23,23 @@ Freezer general utils functions
 import logging
 import os
 import subprocess
+import sys
 import threading
-import time
 
 from freezer.engine import engine
-from freezer import tar
+from freezer.engine.tar import tar_builders
 from freezer import streaming
+from freezer import winutils
 
 
 class TarBackupEngine(engine.BackupEngine):
     DEFAULT_CHUNK_SIZE = 20000000
 
     def __init__(
-            self, gnutar_path, compression_algo, dereference_symlink,
-            exclude, main_storage, is_windows, open_ssl_path=None,
-            encrypt_pass_file=None, dry_run=False,
+            self, compression_algo, dereference_symlink, exclude, main_storage,
+            is_windows, encrypt_pass_file=None, dry_run=False,
             chunk_size=DEFAULT_CHUNK_SIZE):
-        self.gnutar_path = gnutar_path
         self.compression_algo = compression_algo
-        self.open_ssl_path = open_ssl_path
         self.encrypt_pass_file = encrypt_pass_file
         self.dereference_symlink = dereference_symlink
         self.exclude = exclude
@@ -72,7 +70,6 @@ class TarBackupEngine(engine.BackupEngine):
                 break
             if tar_chunk:
                 rich_queue.put(tar_chunk)
-        logging.info("reader finished")
         rich_queue.finish()
 
     @staticmethod
@@ -92,12 +89,10 @@ class TarBackupEngine(engine.BackupEngine):
 
     def backup_data(self, backup_path, manifest_path):
         logging.info("Tar engine backup stream enter")
-        tar_command = tar.TarCommandBuilder(
-            self.gnutar_path, backup_path, self.compression_algo,
-            self.is_windows)
-        if self.open_ssl_path:
-            tar_command.set_encryption(self.open_ssl_path,
-                                       self.encrypt_pass_file)
+        tar_command = tar_builders.TarCommandBuilder(
+            backup_path, self.compression_algo, self.is_windows)
+        if self.encrypt_pass_file:
+            tar_command.set_encryption(self.encrypt_pass_file)
         if self.dereference_symlink:
             tar_command.set_dereference(self.dereference_symlink)
         tar_command.set_exclude(self.exclude)
@@ -122,60 +117,43 @@ class TarBackupEngine(engine.BackupEngine):
                 pass
         logging.info("Tar engine streaming end")
 
-    def restore_stream(self, restore_path, rich_queue):
+    def restore_level(self, restore_path, read_pipe):
         """
-        :param restore_path:
-        :type restore_path: str
-        :param rich_queue:
-        :type rich_queue: freezer.streaming.RichQueue
-        :return:
+        Restore the provided file into backup_opt_dict.restore_abs_path
+        Decrypt the file if backup_opt_dict.encrypt_pass_file key is provided
         """
 
-        tar_command = tar.TarCommandRestoreBuilder(
-            self.gnutar_path, restore_path, self.compression_algo,
-            self.is_windows)
+        tar_command = tar_builders.TarCommandRestoreBuilder(
+            restore_path, self.compression_algo, self.is_windows)
 
-        if self.open_ssl_path:
-            tar_command.set_encryption(self.open_ssl_path,
-                                       self.encrypt_pass_file)
+        if self.encrypt_pass_file:
+            tar_command.set_encryption(self.encrypt_pass_file)
 
         if self.dry_run:
             tar_command.set_dry_run()
 
         command = tar_command.build()
-        logging.info("Execution restore command: \n{}".format(command))
 
-        tar_process = subprocess.Popen(
-            command, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
-            stderr=subprocess.PIPE, shell=True, close_fds=True)
-        if self.is_windows:
+        if winutils.is_windows():
             # on windows, chdir to restore path.
             os.chdir(restore_path)
 
-        writer = threading.Thread(target=self.writer,
-                                  args=(rich_queue, tar_process.stdin))
-        writer.daemon = True
-        writer.start()
-        error_queue = streaming.RichQueue(size=2000)
-        # error buffer size should be small to detect error
-        reader = threading.Thread(target=self.reader,
-                                  args=(error_queue, tar_process.stderr, 10))
-        reader.daemon = True
-        reader.start()
-
-        while writer.is_alive() and not tar_process.poll() \
-                and error_queue.empty():
-            # here I know that tar_process is still running and I have no
-            # exceptions so far. So I need just wait
-            # I understand that sleep here means block of main thread, and
-            # may make it irresponsible. So I provide here very small timeout
-            time.sleep(1)
-
-        res = []
-        while not error_queue.empty():
-            res.append(error_queue.get())
-        if res:
-            tar_err = "".join(res)
+        tar_cmd_proc = subprocess.Popen(
+            command, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE, shell=True)
+        # Start loop reading the pipe and pass the data to the tar std input.
+        # If EOFError exception is raised, the loop end the std err will be
+        # checked for errors.
+        try:
+            while True:
+                t = read_pipe.recv_bytes()
+                tar_cmd_proc.stdin.write(t)
+        except EOFError:
+            logging.info('[*] Pipe closed as EOF reached. '
+                         'Data transmitted successfully')
+
+        tar_err = tar_cmd_proc.communicate()[1]
+
+        if 'error' in tar_err.lower():
             logging.exception('[*] Restore error: {0}'.format(tar_err))
-            rich_queue.force_stop()
-            raise Exception('[*] Restore error: {0}'.format(tar_err))
+            sys.exit(1)

freezer/job.py

@@ -27,9 +27,6 @@ from freezer import utils
 from freezer import backup
 from freezer import exec_cmd
 from freezer import restore
-from freezer import tar
-from freezer import winutils
-import os
 
 import logging
 
@@ -99,8 +96,7 @@ class BackupJob(Job):
 
     def get_metadata(self):
         metadata = {
-            'curr_backup_level': self.conf.curr_backup_level
-            if self.conf.curr_backup_level != '' else 0,
+            'curr_backup_level': 0,
             'fs_real_path': (self.conf.lvm_auto_snap or
                              self.conf.path_to_backup),
             'vol_snap_path':
@@ -130,17 +126,6 @@ class RestoreJob(Job):
         if conf.restore_from_date:
             restore_timestamp = utils.date_to_timestamp(conf.restore_from_date)
         if conf.backup_media == 'fs':
-            builder = tar.TarCommandRestoreBuilder(
-                conf.tar_path, restore_abs_path, conf.compression,
-                winutils.is_windows())
-            if conf.dry_run:
-                builder.set_dry_run()
-            if winutils.is_windows():
-                os.chdir(conf.restore_abs_path)
-            if conf.encrypt_pass_file:
-                builder.set_encryption(conf.openssl_path,
-                                       conf.encrypt_pass_file)
-
             backup = self.storage.find_one(conf.hostname_backup_name,
                                            restore_timestamp)
 

freezer/lvm.py

@@ -21,8 +21,7 @@ Hudson (tjh@cryptsoft.com).
 Freezer LVM related functions
 """
 
-from freezer.utils import (
-    create_dir, get_vol_fs_type, get_mount_from_path)
+from freezer import utils
 
 import re
 import os
@@ -54,7 +53,7 @@ def lvm_eval(backup_opt_dict):
         return False
 
     # Create lvm_dirmount dir if it doesn't exists and write action in logs
-    create_dir(backup_opt_dict.lvm_dirmount)
+    utils.create_dir(backup_opt_dict.lvm_dirmount)
 
     return True
 
@@ -79,10 +78,10 @@ def lvm_snap_remove(backup_opt_dict):
             logging.warning('[*] Found lvm snapshot {0} mounted on {1}\
             '.format(dev_vol, mount_point))
             umount_proc = subprocess.Popen('{0} -l -f {1}'.format(
-                backup_opt_dict.umount_path, mount_point),
+                utils.find_executable("umount"), mount_point),
                 stdin=subprocess.PIPE,
                 stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                shell=True, executable=backup_opt_dict.bash_path)
+                shell=True, executable=utils.find_executable("bash"))
             (umount_out, mount_err) = umount_proc.communicate()
             if re.search(r'\S+', umount_out):
                 raise Exception('impossible to umount {0} {1}'
@@ -94,10 +93,10 @@ def lvm_snap_remove(backup_opt_dict):
                     mapper_snap_vol))
                 snap_rm_proc = subprocess.Popen(
                     '{0} -f {1}'.format(
-                        backup_opt_dict.lvremove_path, mapper_snap_vol),
+                        utils.find_executable("lvremove"), mapper_snap_vol),
                     stdin=subprocess.PIPE,
                     stdout=subprocess.PIPE, stderr=subprocess.PIPE,
-                    shell=True, executable=backup_opt_dict.bash_path)
+                    shell=True, executable=utils.find_executable("bash"))
                 (lvm_rm_out, lvm_rm_err) = snap_rm_proc.communicate()
                 if 'successfully removed' in lvm_rm_out:
                     logging.info('[*] {0}'.format(lvm_rm_out))
@@ -143,7 +142,7 @@ def lvm_snap(backup_opt_dict):
     # Create the snapshot according the values passed from command line
     lvm_create_snap = '{0} --size {1} --snapshot --permission {2} --name {3} {4}\
     '.format(
-        backup_opt_dict.lvcreate_path,
+        utils.find_executable("lvcreate"),
         backup_opt_dict.lvm_snapsize,
         ('r' if backup_opt_dict.lvm_snapperm == 'ro'
          else backup_opt_dict.lvm_snapperm),
@@ -160,7 +159,7 @@ def lvm_snap(backup_opt_dict):
     lvm_process = subprocess.Popen(
         lvm_create_snap, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
         stderr=subprocess.PIPE, shell=True,
-        executable=backup_opt_dict.bash_path)
+        executable=utils.find_executable("bash"))
     (lvm_out, lvm_err) = lvm_process.communicate()
     if lvm_err is False:
         raise Exception('lvm snapshot creation error: {0}'.format(lvm_err))
@@ -176,7 +175,7 @@ def lvm_snap(backup_opt_dict):
 
     # Guess the file system of the provided source volume and st mount
     # options accordingly
-    filesys_type = get_vol_fs_type(backup_opt_dict)
+    filesys_type = utils.get_vol_fs_type(backup_opt_dict)
     mount_options = '-o {}'.format(backup_opt_dict.lvm_snapperm)
     if 'xfs' == filesys_type:
         mount_options = ' -onouuid '
@@ -185,14 +184,14 @@ def lvm_snap(backup_opt_dict):
         backup_opt_dict.lvm_volgroup,
         backup_opt_dict.lvm_snapname)
     mount_snap = '{0} {1} {2} {3}'.format(
-        backup_opt_dict.mount_path,
+        utils.find_executable("mount"),
         mount_options,
         abs_snap_name,
         backup_opt_dict.lvm_dirmount)
     mount_process = subprocess.Popen(
         mount_snap, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
         stderr=subprocess.PIPE, shell=True,
-        executable=backup_opt_dict.bash_path)
+        executable=utils.find_executable("bash"))
     mount_err = mount_process.communicate()[1]
     if 'already mounted' in mount_err:
         logging.warning('[*] Volume {0} already mounted on {1}\
@@ -218,7 +217,7 @@ def get_lvm_info(lvm_auto_snap):
     :returns: a list containing the items lvm_volgroup, lvm_srcvol, lvm_device
     """
 
-    mount_point_path = get_mount_from_path(lvm_auto_snap)
+    mount_point_path = utils.get_mount_from_path(lvm_auto_snap)
     with open('/proc/mounts', 'r') as mount_fd:
         mount_points = mount_fd.readlines()
         lvm_volgroup, lvm_srcvol, lvm_device = lvm_guess(

freezer/main.py

@@ -29,11 +29,11 @@ import json
 from freezer.bandwidth import monkeypatch_socket_bandwidth
 from freezer import job
 from freezer.osclients import ClientManager
-from freezer import swift
-from freezer import local
-from freezer import ssh
+from freezer.storage import swift
+from freezer.storage import local
+from freezer.storage import ssh
 from freezer import utils
-from freezer.engine import tar_engine
+from freezer.engine.tar import tar_engine
 from freezer import winutils
 
 # Initialize backup options
@@ -84,7 +84,7 @@ def freezer_main(backup_args, arg_parse):
             os.nice(-19)
             # Set I/O Priority to Real Time class with level 0
             subprocess.call([
-                u'{0}'.format(backup_args.ionice),
+                u'{0}'.format(utils.find_executable("ionice")),
                 u'-c', u'1', u'-n', u'0', u'-t',
                 u'-p', u'{0}'.format(PID)
             ])
@@ -145,13 +145,11 @@ def freezer_main(backup_args, arg_parse):
 
     backup_args.__dict__['storage'] = storage
     backup_args.__dict__['engine'] = tar_engine.TarBackupEngine(
-        backup_args.tar_path,
         backup_args.compression,
         backup_args.dereference_symlink,
         backup_args.exclude,
         storage,
         winutils.is_windows(),
-        backup_args.openssl_path,
         backup_args.encrypt_pass_file,
         backup_args.dry_run)
 

freezer/storage/local.py

@@ -23,7 +23,7 @@ import shutil
 import io
 import logging
 
-from freezer import storage
+from freezer.storage import storage
 from freezer import utils
 
 

freezer/storage/ssh.py

@@ -24,7 +24,7 @@ import logging
 
 import paramiko
 
-from freezer import storage
+from freezer.storage import storage
 from freezer import utils
 
 
@@ -48,12 +48,19 @@ class SshStorage(storage.Storage):
         self.storage_directory = storage_directory
         self.work_dir = work_dir
         self.chunk_size = chunk_size
-        ssh = paramiko.SSHClient()
+        self.port = port
         # automatically add keys without requiring human intervention
+        self.ssh = None
+        self.ftp = None
+        self.init()
+
+    def init(self):
+        ssh = paramiko.SSHClient()
         ssh.set_missing_host_key_policy(paramiko.AutoAddPolicy())
 
-        ssh.connect(remote_ip, username=remote_username,
-                    key_filename=ssh_key_path, port=port)
+        ssh.connect(self.remote_ip, username=self.remote_username,
+                    key_filename=self.ssh_key_path, port=self.port)
+
         # we should keep link to ssh to prevent garbage collection
         self.ssh = ssh
         self.ftp = self.ssh.open_sftp()
@@ -173,9 +180,9 @@ class SshStorage(storage.Storage):
         self.rm(self._zero_backup_dir(backup))
 
     def backup_blocks(self, backup):
+        self.init()
         filename = self.backup_dir(backup)
-        with self.ftp.open(filename, mode='rb',
-                           bufsize=self.chunk_size) as backup_file:
+        with self.ftp.open(filename, mode='rb') as backup_file:
             while True:
                 chunk = backup_file.read(self.chunk_size)
                 if chunk == '':

freezer/storage/storage.py

@@ -134,7 +134,7 @@ class Storage(object):
         prev_backup = self._find_previous_backup(
             backups, no_incremental, max_level, always_level,
             restart_always_level)
-        if prev_backup:
+        if prev_backup and prev_backup.tar_meta:
             return Backup(
                 hostname_backup_name,
                 time_stamp or utils.DateTime.now().timestamp,
@@ -154,6 +154,7 @@ class Storage(object):
         :param max_level:
         :param always_level:
         :param restart_always_level:
+        :rtype: freezer.storage.storage.Backup
         :return:
         """
         if no_incremental or not backups:

freezer/storage/swift.py

@@ -24,7 +24,7 @@ import logging
 import os
 
 from freezer import utils
-from freezer import storage
+from freezer.storage import storage
 
 
 class SwiftStorage(storage.Storage):

freezer/utils.py

@@ -29,6 +29,8 @@ import re
 import subprocess
 import errno
 from ConfigParser import ConfigParser
+from distutils import spawn as distspawn
+import sys
 
 
 class OpenstackOptions:
@@ -178,11 +180,11 @@ def get_vol_fs_type(backup_opt_dict):
         raise Exception(err)
 
     file_cmd = '{0} -0 -bLs --no-pad --no-buffer --preserve-date \
-    {1}'.format(backup_opt_dict.file_path, vol_name)
+    {1}'.format(find_executable("file"), vol_name)
     file_process = subprocess.Popen(
         file_cmd, stdin=subprocess.PIPE, stdout=subprocess.PIPE,
         stderr=subprocess.PIPE, shell=True,
-        executable=backup_opt_dict.bash_path)
+        executable=find_executable("bash"))
     (file_out, file_err) = file_process.communicate()
     file_match = re.search(r'(\S+?) filesystem data', file_out, re.I)
     if file_match is None:
@@ -354,3 +356,35 @@ def dequote(s):
     if (s[0] == s[-1]) and s.startswith(("'", '"')):
         return s[1:-1]
     return s
+
+
+def find_executable(name):
+    return distspawn.find_executable(name)
+
+
+def openssl_path():
+    import winutils
+    if winutils.is_windows():
+        return 'openssl'
+    else:
+        return find_executable('openssl')
+
+
+def tar_path():
+    import winutils
+    if winutils.is_windows():
+        # windows bin
+        path_to_binaries = os.path.dirname(os.path.abspath(__file__))
+        return '{0}\\bin\\tar.exe'.format(path_to_binaries)
+    elif 'darwin' in sys.platform or 'bsd' in sys.platform:
+        # If freezer is being used under OSX, please install gnutar and
+        # rename the executable as gnutar
+        if distspawn.find_executable('gtar'):
+            return find_executable('gtar')
+        elif distspawn.find_executable('gnutar'):
+            return find_executable('gnutar')
+        else:
+            raise Exception('Please install gnu tar (gtar) as it is a '
+                            'mandatory requirement to use freezer.')
+    else:
+        return find_executable('tar')

tests/commons.py

@@ -11,9 +11,9 @@ import pymysql as MySQLdb
 import pymongo
 import re
 from glanceclient.common.utils import IterableWithLength
-from freezer import swift
+from freezer.storage import swift
 from freezer.utils import OpenstackOptions
-from freezer.engine import tar_engine
+from freezer.engine.tar import tar_engine
 
 os.environ['OS_REGION_NAME'] = 'testregion'
 os.environ['OS_TENANT_ID'] = '0123456789'

tests/engines/__init__.py

@@ -0,0 +1 @@
+__author__ = 'reldan'

tests/engines/tar/test_tar_builders.py

@@ -1,11 +1,12 @@
 import unittest
-from freezer import tar
+from freezer.engine.tar import tar_builders
 
 
 class TestTarCommandBuilder(unittest.TestCase):
 
     def setUp(self):
-        self.builder = tar.TarCommandBuilder("gnutar", ".", "gzip", False)
+        self.builder = tar_builders\
+            .TarCommandBuilder(".", "gzip", False, "gnutar")
 
     def test_build(self):
         self.assertEquals(
@@ -24,7 +25,7 @@ class TestTarCommandBuilder(unittest.TestCase):
 
     def test_build_every_arg(self):
         self.builder.set_listed_incremental("listed-file.tar")
-        self.builder.set_encryption("openssl", "encrypt_pass_file")
+        self.builder.set_encryption("encrypt_pass_file", "openssl")
         self.builder.set_dereference("hard")
         self.builder.set_exclude("excluded_files")
         self.assertEquals(
@@ -38,11 +39,16 @@ class TestTarCommandBuilder(unittest.TestCase):
 
 class TestTarCommandRestoreBuilder(unittest.TestCase):
     def setUp(self):
-        self.builder = tar.TarCommandRestoreBuilder(
-            "gnutar", "restore_path", "gzip", False)
+        self.builder = tar_builders.TarCommandRestoreBuilder(
+            "restore_path", "gzip", False, "gnutar")
 
     def test(self):
         self.assertEquals(
             self.builder.build(),
             "gnutar -z --incremental --extract --unlink-first --ignore-zeros "
             "--warning=none --overwrite --directory restore_path")
+
+    def test_get_tar_flag_from_algo(self):
+        assert tar_builders.get_tar_flag_from_algo('gzip') == '-z'
+        assert tar_builders.get_tar_flag_from_algo('bzip2') == '-j'
+        assert tar_builders.get_tar_flag_from_algo('xz') == '-J'

tests/storages/test_local.py

@@ -2,11 +2,8 @@ import tempfile
 import shutil
 import pytest
 
-from freezer import local
-from freezer import tar
+from freezer.storage import local
 from freezer import utils
-import commons
-import os
 
 @pytest.mark.incremental
 class TestLocalStorage(object):

tests/storages/test_storage.py

@@ -1,6 +1,5 @@
 import unittest
-from freezer import storage
-from freezer import tar
+from freezer.storage import storage
 import mock
 
 

tests/storages/test_swift.py

@@ -1,8 +1,8 @@
 import unittest
 from freezer import osclients
 from freezer import utils
-from freezer import swift
-from freezer import storage
+from freezer.storage import swift
+from freezer.storage import storage
 
 
 class TestSwiftStorage(unittest.TestCase):

tests/test_arguments.py

@@ -1,12 +1,8 @@
 #!/usr/bin/env python
 
-from freezer.arguments import backup_arguments, alter_proxy
-import argparse
-from commons import *
-import sys
+from freezer.arguments import alter_proxy
 import os
 import pytest
-import distutils.spawn as distspawn
 
 
 class TestArguments(object):
@@ -39,24 +35,3 @@ class TestArguments(object):
         alter_proxy(test_dict)
         assert os.environ["HTTP_PROXY"] == test_proxy
         assert os.environ["HTTPS_PROXY"] == test_proxy
-
-    def test_arguments(self, monkeypatch):
-        fakeargparse = FakeArgparse()
-        fakeargparse = fakeargparse.ArgumentParser()
-        fakedistutils = FakeDistutils()
-        fakedistutilsspawn = fakedistutils.spawn()
-
-        monkeypatch.setattr(
-            argparse, 'ArgumentParser', fakeargparse)
-
-        platform = sys.platform
-        assert backup_arguments() is not False
-
-        if sys.__dict__['platform'] != 'darwin':
-            sys.__dict__['platform'] = 'darwin'
-            pytest.raises(Exception, backup_arguments)
-        sys.__dict__['platform'] = 'darwin'
-        monkeypatch.setattr(
-            distspawn, 'find_executable', fakedistutilsspawn.find_executable)
-        assert backup_arguments() is not False
-        sys.__dict__['platform'] = platform

tests/test_lvm.py

@@ -80,6 +80,7 @@ class TestLvm:
         backup_opt.lvm_snapsize = False
         backup_opt.lvm_snapname = False
         monkeypatch.setattr(os, 'path', fakeos)
+        monkeypatch.setattr(utils, 'find_executable', lambda x: "123")
         monkeypatch.setattr(subprocess, 'Popen', fakesubprocess.Popen)
         pytest.raises(Exception, lvm_snap, backup_opt)
 

tests/test_tar.py

@@ -1,54 +0,0 @@
-"""Freezer Tar related tests
-
-(c) Copyright 2014,2015 Hewlett-Packard Development Company, L.P.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-
-This product includes cryptographic software written by Eric Young
-(eay@cryptsoft.com). This product includes software written by Tim
-Hudson (tjh@cryptsoft.com).
-========================================================================
-
-"""
-
-from commons import *
-from freezer.tar import get_tar_flag_from_algo
-
-import os
-import logging
-
-
-class TestTar:
-
-    def test_tar_restore_args_valid(self, monkeypatch):
-
-        backup_opt = BackupOpt1()
-        fakelogging = FakeLogging()
-        monkeypatch.setattr(logging, 'critical', fakelogging.critical)
-        monkeypatch.setattr(logging, 'warning', fakelogging.warning)
-        monkeypatch.setattr(logging, 'exception', fakelogging.exception)
-        monkeypatch.setattr(logging, 'error', fakelogging.error)
-
-        fakeos = Os()
-        monkeypatch.setattr(os.path, 'exists', fakeos.exists)
-
-        backup_opt.dry_run = True
-
-        fakeos1 = Os1()
-        monkeypatch.setattr(os.path, 'exists', fakeos1.exists)
-        backup_opt.dry_run = False
-
-    def test_get_tar_flag_from_algo(self):
-        assert get_tar_flag_from_algo('gzip') == '-z'
-        assert get_tar_flag_from_algo('bzip2') == '-j'
-        assert get_tar_flag_from_algo('xz') == '-J'