From c2ecfd11e8de3d7540b3765e3a78cfdb3bcde566 Mon Sep 17 00:00:00 2001 From: eldar nugaev Date: Thu, 24 Sep 2015 15:17:25 +0100 Subject: [PATCH] Fix for race condition for parallel multi-level backup restore Change-Id: Ibf4840e9e1dafce57b54d3bb2d2bed8fe5666c5b Resolves: bug 1500961 --- freezer/arguments.py | 50 +---------- freezer/engine/engine.py | 60 +++++++++---- freezer/engine/tar/__init__.py | 0 .../{tar.py => engine/tar/tar_builders.py} | 22 ++--- freezer/engine/{ => tar}/tar_engine.py | 88 +++++++------------ freezer/job.py | 17 +--- freezer/lvm.py | 25 +++--- freezer/main.py | 12 ++- freezer/storage/__init__.py | 0 freezer/{ => storage}/local.py | 2 +- freezer/{ => storage}/ssh.py | 19 ++-- freezer/{ => storage}/storage.py | 3 +- freezer/{ => storage}/swift.py | 2 +- freezer/utils.py | 38 +++++++- tests/commons.py | 4 +- tests/engines/__init__.py | 1 + tests/engines/tar/__init__.py | 0 tests/{ => engines/tar}/test_tar_builders.py | 16 ++-- tests/storages/__init__.py | 0 tests/{ => storages}/test_local.py | 5 +- tests/{ => storages}/test_storage.py | 3 +- tests/{ => storages}/test_swift.py | 4 +- tests/test_arguments.py | 27 +----- tests/test_lvm.py | 1 + tests/test_tar.py | 54 ------------ 25 files changed, 181 insertions(+), 272 deletions(-) create mode 100644 freezer/engine/tar/__init__.py rename freezer/{tar.py => engine/tar/tar_builders.py} (88%) rename freezer/engine/{ => tar}/tar_engine.py (60%) create mode 100644 freezer/storage/__init__.py rename freezer/{ => storage}/local.py (99%) rename freezer/{ => storage}/ssh.py (94%) rename freezer/{ => storage}/storage.py (99%) rename freezer/{ => storage}/swift.py (99%) create mode 100644 tests/engines/__init__.py create mode 100644 tests/engines/tar/__init__.py rename tests/{ => engines/tar}/test_tar_builders.py (74%) create mode 100644 tests/storages/__init__.py rename tests/{ => storages}/test_local.py (96%) rename tests/{ => storages}/test_storage.py (99%) rename tests/{ => storages}/test_swift.py (97%) delete mode 100644 tests/test_tar.py diff --git a/freezer/arguments.py b/freezer/arguments.py index 6f5d7601..0203a1fe 100644 --- a/freezer/arguments.py +++ b/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' diff --git a/freezer/engine/engine.py b/freezer/engine/engine.py index 9a853f23..b4300ee9 100644 --- a/freezer/engine/engine.py +++ b/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") diff --git a/freezer/engine/tar/__init__.py b/freezer/engine/tar/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/freezer/tar.py b/freezer/engine/tar/tar_builders.py similarity index 88% rename from freezer/tar.py rename to freezer/engine/tar/tar_builders.py index 4bd173c8..eab897a0 100644 --- a/freezer/tar.py +++ b/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): diff --git a/freezer/engine/tar_engine.py b/freezer/engine/tar/tar_engine.py similarity index 60% rename from freezer/engine/tar_engine.py rename to freezer/engine/tar/tar_engine.py index e6e2a0c4..9c830b12 100644 --- a/freezer/engine/tar_engine.py +++ b/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() + 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') - 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) + tar_err = tar_cmd_proc.communicate()[1] - res = [] - while not error_queue.empty(): - res.append(error_queue.get()) - if res: - tar_err = "".join(res) + 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) diff --git a/freezer/job.py b/freezer/job.py index ebd26a05..b1629f3e 100644 --- a/freezer/job.py +++ b/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) diff --git a/freezer/lvm.py b/freezer/lvm.py index 8b3ae97e..a7fc6023 100644 --- a/freezer/lvm.py +++ b/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( diff --git a/freezer/main.py b/freezer/main.py index f65bda1e..7e89c571 100644 --- a/freezer/main.py +++ b/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) diff --git a/freezer/storage/__init__.py b/freezer/storage/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/freezer/local.py b/freezer/storage/local.py similarity index 99% rename from freezer/local.py rename to freezer/storage/local.py index 56e667b7..c0b1a876 100644 --- a/freezer/local.py +++ b/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 diff --git a/freezer/ssh.py b/freezer/storage/ssh.py similarity index 94% rename from freezer/ssh.py rename to freezer/storage/ssh.py index b4eb3cbb..6f86b374 100644 --- a/freezer/ssh.py +++ b/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 == '': diff --git a/freezer/storage.py b/freezer/storage/storage.py similarity index 99% rename from freezer/storage.py rename to freezer/storage/storage.py index 606f19c3..f1012183 100644 --- a/freezer/storage.py +++ b/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: diff --git a/freezer/swift.py b/freezer/storage/swift.py similarity index 99% rename from freezer/swift.py rename to freezer/storage/swift.py index b46cf0af..8df5fe89 100644 --- a/freezer/swift.py +++ b/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): diff --git a/freezer/utils.py b/freezer/utils.py index 3d4623cf..70f3e01d 100644 --- a/freezer/utils.py +++ b/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') diff --git a/tests/commons.py b/tests/commons.py index 77081ab3..62b275e4 100644 --- a/tests/commons.py +++ b/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' diff --git a/tests/engines/__init__.py b/tests/engines/__init__.py new file mode 100644 index 00000000..4def9add --- /dev/null +++ b/tests/engines/__init__.py @@ -0,0 +1 @@ +__author__ = 'reldan' diff --git a/tests/engines/tar/__init__.py b/tests/engines/tar/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_tar_builders.py b/tests/engines/tar/test_tar_builders.py similarity index 74% rename from tests/test_tar_builders.py rename to tests/engines/tar/test_tar_builders.py index 6230f1b1..7df781a8 100644 --- a/tests/test_tar_builders.py +++ b/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' diff --git a/tests/storages/__init__.py b/tests/storages/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/test_local.py b/tests/storages/test_local.py similarity index 96% rename from tests/test_local.py rename to tests/storages/test_local.py index 713b9281..26c9c6a1 100644 --- a/tests/test_local.py +++ b/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): diff --git a/tests/test_storage.py b/tests/storages/test_storage.py similarity index 99% rename from tests/test_storage.py rename to tests/storages/test_storage.py index f3c8df45..27bd03e6 100644 --- a/tests/test_storage.py +++ b/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 diff --git a/tests/test_swift.py b/tests/storages/test_swift.py similarity index 97% rename from tests/test_swift.py rename to tests/storages/test_swift.py index 79101f0f..fc33ffc6 100644 --- a/tests/test_swift.py +++ b/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): diff --git a/tests/test_arguments.py b/tests/test_arguments.py index a4e5303b..576d939e 100644 --- a/tests/test_arguments.py +++ b/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 diff --git a/tests/test_lvm.py b/tests/test_lvm.py index 10cd81e2..577ac88c 100644 --- a/tests/test_lvm.py +++ b/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) diff --git a/tests/test_tar.py b/tests/test_tar.py deleted file mode 100644 index 78bd8a32..00000000 --- a/tests/test_tar.py +++ /dev/null @@ -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'