Merge pull request #1020 from YunoHost/do-not-compress-backups

Backup to uncompressed tar by default

locales/en.json

@@ -71,7 +71,6 @@
     "backup_abstract_method": "This backup method has yet to be implemented",
     "backup_actually_backuping": "Creating a backup archive from the collected files...",
     "backup_app_failed": "Could not back up {app:s}",
-    "backup_applying_method_borg": "Sending all files to backup into borg-backup repository...",
     "backup_applying_method_copy": "Copying all files to backup...",
     "backup_applying_method_custom": "Calling the custom backup method '{method:s}'...",
     "backup_applying_method_tar": "Creating the backup TAR archive...",
@@ -85,7 +84,6 @@
     "backup_archive_system_part_not_available": "System part '{part:s}' unavailable in this backup",
     "backup_archive_writing_error": "Could not add the files '{source:s}' (named in the archive '{dest:s}') to be backed up into the compressed archive '{archive:s}'",
     "backup_ask_for_copying_if_needed": "Do you want to perform the backup using {size:s}MB temporarily? (This way is used since some files could not be prepared using a more efficient method.)",
-    "backup_borg_not_implemented": "The Borg backup method is not yet implemented",
     "backup_cant_mount_uncompress_archive": "Could not mount the uncompressed archive as write protected",
     "backup_cleaning_failed": "Could not clean up the temporary backup folder",
     "backup_copying_to_organize_the_archive": "Copying {size:s}MB to organize the archive",
@@ -99,7 +97,6 @@
     "backup_delete_error": "Could not delete '{path:s}'",
     "backup_deleted": "Backup deleted",
     "backup_hook_unknown": "The backup hook '{hook:s}' is unknown",
-    "backup_method_borg_finished": "Backup into Borg finished",
     "backup_method_copy_finished": "Backup copy finalized",
     "backup_method_custom_finished": "Custom backup method '{method:s}' finished",
     "backup_method_tar_finished": "TAR backup archive created",
@@ -327,6 +324,7 @@
     "global_settings_unknown_setting_from_settings_file": "Unknown key in settings: '{setting_key:s}', discard it and save it in /etc/yunohost/settings-unknown.json",
     "global_settings_setting_service_ssh_allow_deprecated_dsa_hostkey": "Allow the use of (deprecated) DSA hostkey for the SSH daemon configuration",
     "global_settings_setting_smtp_allow_ipv6": "Allow the use of IPv6 to receive and send mail",
+    "global_settings_setting_backup_compress_tar_archives": "When creating new backups, compress the archives (.tar.gz) instead of uncompressed archives (.tar). N.B. : enabling this option means create lighter backup archives, but the initial backup procedure will be significantly longer and heavy on CPU.",
     "global_settings_unknown_type": "Unexpected situation, the setting {setting:s} appears to have the type {unknown_type:s} but it is not a type supported by the system.",
     "good_practices_about_admin_password": "You are now about to define a new administration password. The password should be at least 8 characters long—though it is good practice to use a longer password (i.e. a passphrase) and/or to use a variation of characters (uppercase, lowercase, digits and special characters).",
     "good_practices_about_user_password": "You are now about to define a new user password. The password should be at least 8 characters long—though it is good practice to use a longer password (i.e. a passphrase) and/or to a variation of characters (uppercase, lowercase, digits and special characters).",

src/yunohost/backup.py

@@ -59,6 +59,7 @@ from yunohost.regenconf import regen_conf
 from yunohost.log import OperationLogger
 from yunohost.utils.error import YunohostError
 from yunohost.utils.packages import ynh_packages_version
+from yunohost.settings import settings_get
 
 BACKUP_PATH = '/home/yunohost.backup'
 ARCHIVES_PATH = '%s/archives' % BACKUP_PATH
@@ -225,8 +226,8 @@ class BackupManager():
         backup_manager = BackupManager(name="mybackup", description="bkp things")
 
         # Add backup method to apply
-        backup_manager.add(BackupMethod.create('copy', backup_manager, '/mnt/local_fs'))
+        backup_manager.add('copy', output_directory='/mnt/local_fs')
-        backup_manager.add(BackupMethod.create('tar', backup_manager, '/mnt/remote_fs'))
+        backup_manager.add('tar', output_directory='/mnt/remote_fs')
 
         # Define targets to be backuped
         backup_manager.set_system_targets(["data"])
@@ -315,17 +316,6 @@ class BackupManager():
         """Initialize preparation directory
 
         Ensure the working directory exists and is empty
-
-        exception:
-        backup_output_directory_not_empty -- (YunohostError) Raised if the
-            directory was given by the user and isn't empty
-
-        (TODO) backup_cant_clean_tmp_working_directory -- (YunohostError)
-            Raised if the working directory isn't empty, is temporary and can't
-            be automaticcaly cleaned
-
-        (TODO) backup_cant_create_working_directory -- (YunohostError) Raised
-            if iyunohost can't create the working directory
         """
 
         # FIXME replace isdir by exists ? manage better the case where the path
@@ -509,10 +499,6 @@ class BackupManager():
                       files to backup
         hooks/     -- restore scripts associated to system backup scripts are
                       copied here
-
-        Exceptions:
-        "backup_nothings_done" -- (YunohostError) This exception is raised if
-        nothing has been listed.
         """
 
         self._collect_system_files()
@@ -679,10 +665,6 @@ class BackupManager():
 
         Args:
         app -- (string) an app instance name (already installed) to backup
-
-        Exceptions:
-        backup_app_failed -- Raised at the end if the app backup script
-                             execution failed
         """
         from yunohost.permission import user_permission_list
 
@@ -741,17 +723,11 @@ class BackupManager():
     # Actual backup archive creation / method management                    #
     #
 
-    def add(self, method):
+    def add(self, method, output_directory=None):
         """
         Add a backup method that will be applied after the files collection step
-
-        Args:
-        method -- (BackupMethod) A backup method. Currently, you can use those:
-                  TarBackupMethod
-                  CopyBackupMethod
-                  CustomBackupMethod
         """
-        self.methods.append(method)
+        self.methods.append(BackupMethod.create(method, self, output_directory=output_directory))
 
     def backup(self):
         """Apply backup methods"""
@@ -815,7 +791,7 @@ class RestoreManager():
     """
     RestoreManager allow to restore a past backup archive
 
-    Currently it's a tar.gz file, but it could be another kind of archive
+    Currently it's a tar file, but it could be another kind of archive
 
     Public properties:
         info (getter)i # FIXME
@@ -841,14 +817,12 @@ class RestoreManager():
         return restore_manager.result
     """
 
-    def __init__(self, name, repo=None, method='tar'):
+    def __init__(self, name, method='tar'):
         """
         RestoreManager constructor
 
         Args:
         name -- (string) Archive name
-        repo -- (string|None) Repository where is this archive, it could be a
-                path (default: /home/yunohost.backup/archives)
         method -- (string) Method name to use to mount the archive
         """
         # Retrieve and open the archive
@@ -876,9 +850,6 @@ class RestoreManager():
     def _read_info_files(self):
         """
         Read the info file from inside an archive
-
-        Exceptions:
-        backup_archive_cant_retrieve_info_json -- Raised if we can't read the info
         """
         # Retrieve backup info
         info_file = os.path.join(self.work_dir, "info.json")
@@ -1036,10 +1007,6 @@ class RestoreManager():
 
         Use the mount method from the BackupMethod instance and read info about
         this archive
-
-        Exceptions:
-        restore_removing_tmp_dir_failed -- Raised if it's not possible to remove
-        the working directory
         """
 
         self.work_dir = os.path.join(BACKUP_PATH, "tmp", self.name)
@@ -1113,11 +1080,6 @@ class RestoreManager():
     def assert_enough_free_space(self):
         """
         Check available disk space
-
-        Exceptions:
-        restore_may_be_not_enough_disk_space -- Raised if there isn't enough
-        space to cover the security margin space
-        restore_not_enough_disk_space -- Raised if there isn't enough space
         """
 
         free_space = free_space_in_directory(BACKUP_PATH)
@@ -1293,11 +1255,6 @@ class RestoreManager():
         Args:
         app_instance_name -- (string) The app name to restore (no app with this
                              name should be already install)
-
-        Exceptions:
-        restore_already_installed_app -- Raised if an app with this app instance
-                                        name already exists
-        restore_app_failed -- Raised if the restore bash script failed
         """
         from yunohost.user import user_group_list
         from yunohost.permission import permission_create, permission_delete, user_permission_list, user_permission_update, permission_sync_to_user
@@ -1500,7 +1457,7 @@ class BackupMethod(object):
 
     TarBackupMethod
     ---------------
-    This method compresses all files to backup in a .tar.gz archive. When
+    This method compresses all files to backup in a .tar archive. When
     restoring, it untars the required parts.
 
     CustomBackupMethod
@@ -1525,7 +1482,24 @@ class BackupMethod(object):
         method.mount()
     """
 
-    def __init__(self, manager, repo=None):
+    @classmethod
+    def create(cls, method, manager, *args, **kwargs):
+        """
+        Factory method to create instance of BackupMethod
+
+        Args:
+        method -- (string) The method name of an existing BackupMethod. If the
+        name is unknown the CustomBackupMethod will be tried
+        *args    -- Specific args for the method, could be the repo target by the
+        method
+
+        Return a BackupMethod instance
+        """
+        known_methods = {c.method_name:c for c in BackupMethod.__subclasses__()}
+        backup_method = known_methods.get(method, CustomBackupMethod)
+        return backup_method(manager, method=method, *args, **kwargs)
+
+    def __init__(self, manager, repo=None, **kwargs):
         """
         BackupMethod constructors
 
@@ -1610,10 +1584,6 @@ class BackupMethod(object):
     def clean(self):
         """
         Umount sub directories of working dirextories and delete it if temporary
-
-        Exceptions:
-        backup_cleaning_failed -- Raise if we were not able to unmount sub
-                                  directories of the working directories
         """
         if self.need_mount():
             if not _recursive_umount(self.work_dir):
@@ -1625,9 +1595,6 @@ class BackupMethod(object):
     def _check_is_enough_free_space(self):
         """
         Check free space in repository or output directory before to backup
-
-        Exceptions:
-        not_enough_disk_space -- Raise if there isn't enough space.
         """
         # TODO How to do with distant repo or with deduplicated backup ?
         backup_size = self.manager.size
@@ -1649,9 +1616,6 @@ class BackupMethod(object):
 
         The usage of binding could be strange for a user because the du -sb
         command will return that the working directory is big.
-
-        Exceptions:
-        backup_unable_to_organize_files
         """
         paths_needed_to_be_copied = []
         for path in self.manager.paths_to_backup:
@@ -1749,36 +1713,6 @@ class BackupMethod(object):
             else:
                 shutil.copy(path['source'], dest)
 
-    @classmethod
-    def create(cls, method, manager, *args):
-        """
-        Factory method to create instance of BackupMethod
-
-        Args:
-        method -- (string) The method name of an existing BackupMethod. If the
-        name is unknown the CustomBackupMethod will be tried
-
-        ...    -- Specific args for the method, could be the repo target by the
-        method
-
-        Return a BackupMethod instance
-        """
-        if not isinstance(method, basestring):
-            methods = []
-            for m in method:
-                methods.append(BackupMethod.create(m, manager, *args))
-            return methods
-
-        bm_class = {
-            'copy': CopyBackupMethod,
-            'tar': TarBackupMethod,
-            'borg': BorgBackupMethod
-        }
-        if method in ["copy", "tar", "borg"]:
-            return bm_class[method](manager, *args)
-        else:
-            return CustomBackupMethod(manager, method=method, *args)
-
 
 class CopyBackupMethod(BackupMethod):
 
@@ -1787,12 +1721,7 @@ class CopyBackupMethod(BackupMethod):
     could be the inverse for restoring
     """
 
-    def __init__(self, manager, repo=None):
+    method_name = "copy"
-        super(CopyBackupMethod, self).__init__(manager, repo)
-
-    @property
-    def method_name(self):
-        return 'copy'
 
     def backup(self):
         """ Copy prepared files into a the repo """
@@ -1818,10 +1747,6 @@ class CopyBackupMethod(BackupMethod):
     def mount(self):
         """
         Mount the uncompress backup in readonly mode to the working directory
-
-        Exceptions:
-        backup_no_uncompress_archive_dir -- Raised if the repo doesn't exists
-        backup_cant_mount_uncompress_archive -- Raised if the binding failed
         """
         # FIXME: This code is untested because there is no way to run it from
         # the ynh cli
@@ -1848,21 +1773,18 @@ class CopyBackupMethod(BackupMethod):
 
 class TarBackupMethod(BackupMethod):
 
-    """
+    method_name = "tar"
-    This class compress all files to backup in archive.
-    """
-
-    def __init__(self, manager, repo=None):
-        super(TarBackupMethod, self).__init__(manager, repo)
-
-    @property
-    def method_name(self):
-        return 'tar'
 
     @property
     def _archive_file(self):
-        """Return the compress archive path"""
+
-        return os.path.join(self.repo, self.name + '.tar.gz')
+        if isinstance(self.manager, BackupManager) and settings_get("backup.compress_tar_archives"):
+            return os.path.join(self.repo, self.name + '.tar.gz')
+
+        f = os.path.join(self.repo, self.name + '.tar')
+        if os.path.exists(f + ".gz"):
+            f += ".gz"
+        return f
 
     def backup(self):
         """
@@ -1870,11 +1792,6 @@ class TarBackupMethod(BackupMethod):
 
         It adds the info.json in /home/yunohost.backup/archives and if the
         compress archive isn't located here, add a symlink to the archive to.
-
-        Exceptions:
-           backup_archive_open_failed -- Raised if we can't open the archive
-           backup_creation_failed     -- Raised if we can't write in the
-                                         compress archive
         """
 
         if not os.path.exists(self.repo):
@@ -1885,7 +1802,7 @@ class TarBackupMethod(BackupMethod):
 
         # Open archive file for writing
         try:
-            tar = tarfile.open(self._archive_file, "w:gz")
+            tar = tarfile.open(self._archive_file, "w:gz" if self._archive_file.endswith(".gz") else "w")
         except:
             logger.debug("unable to open '%s' for writing",
                          self._archive_file, exc_info=1)
@@ -1909,26 +1826,20 @@ class TarBackupMethod(BackupMethod):
 
         # If backuped to a non-default location, keep a symlink of the archive
         # to that location
-        link = os.path.join(ARCHIVES_PATH, self.name + '.tar.gz')
+        link = os.path.join(ARCHIVES_PATH, self.name + '.tar')
         if not os.path.isfile(link):
             os.symlink(self._archive_file, link)
 
     def mount(self):
         """
-        Mount the archive. We avoid copy to be able to restore on system without
+        Mount the archive. We avoid intermediate copies to be able to restore on system with low free space.
-        too many space.
-
-        Exceptions:
-        backup_archive_open_failed -- Raised if the archive can't be open
-        backup_archive_corrupted -- Raised if the archive appears corrupted
-        backup_archive_cant_retrieve_info_json -- If the info.json file can't be retrieved
         """
         super(TarBackupMethod, self).mount()
 
         # Mount the tarball
         logger.debug(m18n.n("restore_extracting"))
         try:
-            tar = tarfile.open(self._archive_file, "r:gz")
+            tar = tarfile.open(self._archive_file, "r:gz" if self._archive_file.endswith(".gz") else "r")
         except:
             logger.debug("cannot open backup archive '%s'",
                          self._archive_file, exc_info=1)
@@ -1997,7 +1908,7 @@ class TarBackupMethod(BackupMethod):
         tar.close()
 
     def copy(self, file, target):
-        tar = tarfile.open(self._archive_file, "r:gz")
+        tar = tarfile.open(self._archive_file, "r:gz" if self._archive_file.endswith(".gz") else "r")
         file_to_extract = tar.getmember(file)
         # Remove the path
         file_to_extract.name = os.path.basename(file_to_extract.name)
@@ -2005,26 +1916,6 @@ class TarBackupMethod(BackupMethod):
         tar.close()
 
 
-class BorgBackupMethod(BackupMethod):
-
-    @property
-    def method_name(self):
-        return 'borg'
-
-    def backup(self):
-        """ Backup prepared files with borg """
-        super(CopyBackupMethod, self).backup()
-
-        # TODO run borg create command
-        raise YunohostError('backup_borg_not_implemented')
-
-    def mount(self, mnt_path):
-        raise YunohostError('backup_borg_not_implemented')
-
-    def copy(self, file, target):
-        raise YunohostError('backup_borg_not_implemented')
-
-
 class CustomBackupMethod(BackupMethod):
 
     """
@@ -2033,21 +1924,16 @@ class CustomBackupMethod(BackupMethod):
     /etc/yunohost/hooks.d/backup_method/
     """
 
+    method_name = "custom"
+
     def __init__(self, manager, repo=None, method=None, **kwargs):
         super(CustomBackupMethod, self).__init__(manager, repo)
         self.args = kwargs
         self.method = method
         self._need_mount = None
 
-    @property
-    def method_name(self):
-        return 'borg'
-
     def need_mount(self):
         """Call the backup_method hook to know if we need to organize files
-
-        Exceptions:
-        backup_custom_need_mount_error -- Raised if the hook failed
         """
         if self._need_mount is not None:
             return self._need_mount
@@ -2062,9 +1948,6 @@ class CustomBackupMethod(BackupMethod):
     def backup(self):
         """
         Launch a custom script to backup
-
-        Exceptions:
-        backup_custom_backup_error -- Raised if the custom script failed
         """
 
         ret = hook_callback('backup_method', [self.method],
@@ -2078,9 +1961,6 @@ class CustomBackupMethod(BackupMethod):
     def mount(self):
         """
         Launch a custom script to mount the custom archive
-
-        Exceptions:
-        backup_custom_mount_error -- Raised if the custom script failed
         """
         super(CustomBackupMethod, self).mount()
         ret = hook_callback('backup_method', [self.method],
@@ -2150,7 +2030,7 @@ def backup_create(name=None, description=None, methods=[],
         if no_compress:
             methods = ['copy']
         else:
-            methods = ['tar']  # In future, borg will be the default actions
+            methods = ['tar']
 
     # If no --system or --apps given, backup everything
     if system is None and apps is None:
@@ -2171,14 +2051,8 @@ def backup_create(name=None, description=None, methods=[],
     else:
         backup_manager = BackupManager(name, description)
 
-    # Add backup methods
-    if output_directory:
-        methods = BackupMethod.create(methods, backup_manager, output_directory)
-    else:
-        methods = BackupMethod.create(methods, backup_manager)
-
     for method in methods:
-        backup_manager.add(method)
+        backup_manager.add(method, output_directory=output_directory)
 
     # Add backup targets (system and apps)
     backup_manager.set_system_targets(system)
@@ -2284,9 +2158,17 @@ def backup_list(with_info=False, human_readable=False):
 
     """
     # Get local archives sorted according to last modification time
-    archives = sorted(glob("%s/*.tar.gz" % ARCHIVES_PATH), key=lambda x: os.path.getctime(x))
+    # (we do a realpath() to resolve symlinks)
+    archives = glob("%s/*.tar.gz" % ARCHIVES_PATH) + glob("%s/*.tar" % ARCHIVES_PATH)
+    archives = set([os.path.realpath(archive) for archive in archives])
+    archives = sorted(archives, key=lambda x: os.path.getctime(x))
     # Extract only filename without the extension
-    archives = [os.path.basename(f)[:-len(".tar.gz")] for f in archives]
+    def remove_extension(f):
+        if f.endswith(".tar.gz"):
+            return os.path.basename(f)[:-len(".tar.gz")]
+        else:
+            return os.path.basename(f)[:-len(".tar")]
+    archives = [remove_extension(f) for f in archives]
 
     if with_info:
         d = OrderedDict()
@@ -2314,11 +2196,13 @@ def backup_info(name, with_details=False, human_readable=False):
         human_readable -- Print sizes in human readable format
 
     """
-    archive_file = '%s/%s.tar.gz' % (ARCHIVES_PATH, name)
+    archive_file = '%s/%s.tar' % (ARCHIVES_PATH, name)
 
     # Check file exist (even if it's a broken symlink)
     if not os.path.lexists(archive_file):
-        raise YunohostError('backup_archive_name_unknown', name=name)
+        archive_file += ".gz"
+        if not os.path.lexists(archive_file):
+            raise YunohostError('backup_archive_name_unknown', name=name)
 
     # If symlink, retrieve the real path
     if os.path.islink(archive_file):
@@ -2332,7 +2216,7 @@ def backup_info(name, with_details=False, human_readable=False):
     info_file = "%s/%s.info.json" % (ARCHIVES_PATH, name)
 
     if not os.path.exists(info_file):
-        tar = tarfile.open(archive_file, "r:gz")
+        tar = tarfile.open(archive_file, "r:gz" if archive_file.endswith(".gz") else "r")
         info_dir = info_file + '.d'
 
         try:
@@ -2368,7 +2252,7 @@ def backup_info(name, with_details=False, human_readable=False):
     # Retrieve backup size
     size = info.get('size', 0)
     if not size:
-        tar = tarfile.open(archive_file, "r:gz")
+        tar = tarfile.open(archive_file, "r:gz" if archive_file.endswith(".gz") else "r")
         size = reduce(lambda x, y: getattr(x, 'size', x) + getattr(y, 'size', y),
                       tar.getmembers())
         tar.close()
@@ -2428,7 +2312,9 @@ def backup_delete(name):
 
     hook_callback('pre_backup_delete', args=[name])
 
-    archive_file = '%s/%s.tar.gz' % (ARCHIVES_PATH, name)
+    archive_file = '%s/%s.tar' % (ARCHIVES_PATH, name)
+    if os.path.exists(archive_file + ".gz"):
+        archive_file += '.gz'
     info_file = "%s/%s.info.json" % (ARCHIVES_PATH, name)
 
     files_to_delete = [archive_file, info_file]

src/yunohost/settings.py

@@ -71,6 +71,7 @@ DEFAULTS = OrderedDict([
         "choices": ["intermediate", "modern"]}),
     ("pop3.enabled", {"type": "bool", "default": False}),
     ("smtp.allow_ipv6", {"type": "bool", "default": True}),
+    ("backup.compress_tar_archives", {"type": "bool", "default": False}),
 ])
 
 

src/yunohost/tests/test_backuprestore.py

@@ -375,7 +375,7 @@ def test_backup_with_different_output_directory(mocker):
                       output_directory="/opt/test_backup_output_directory",
                       name="backup")
 
-    assert os.path.exists("/opt/test_backup_output_directory/backup.tar.gz")
+    assert os.path.exists("/opt/test_backup_output_directory/backup.tar")
 
     archives = backup_list()["archives"]
     assert len(archives) == 1

tests/test_i18n_keys.py

@@ -116,7 +116,7 @@ def find_expected_string_keys():
     # Hardcoded expected keys ...
     yield "admin_password"  # Not sure that's actually used nowadays...
 
-    for method in ["tar", "copy", "borg", "custom"]:
+    for method in ["tar", "copy", "custom"]:
         yield "backup_applying_method_%s" % method
         yield "backup_method_%s_finished" % method