NAS-132072 / 25.04 / Add transfer setting to Truecloud Backup (by cre…

…atorcary) (#14882)

* remove bwlimit and transfers from cloud_backup

(cherry picked from commit 51b7d9a)

* add transfer_setting

(cherry picked from commit 4907c38)

* migration

(cherry picked from commit 2c533a0)

* add transfer_setting_choices endpoint

(cherry picked from commit 48961da)

* add tests

(cherry picked from commit 7fac9b2)

* fix new test

(cherry picked from commit d9dec14)

* fix new test

(cherry picked from commit ad8c2dd)

* fix circular import

* merge migration

* add import comment

---------

Co-authored-by: Logan Cary <[email protected]>

src/middlewared/middlewared/alembic/versions/24.10/2024-11-04_20-26_cloud_backup_transfer_setting.py

@@ -0,0 +1,36 @@
+"""Remove cloud_backup bwlimit, transfers; add transfer_setting
+
+Revision ID: 8eacd2bb3e18
+Revises: 92b98613c498
+Create Date: 2024-11-04 20:26:07.568238+00:00
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '8eacd2bb3e18'
+down_revision = '92b98613c498'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.batch_alter_table('tasks_cloud_backup', schema=None) as batch_op:
+        batch_op.add_column(sa.Column('transfer_setting', sa.String(length=16), server_default='DEFAULT', nullable=False))
+        batch_op.drop_column('transfers')
+        batch_op.drop_column('bwlimit')
+
+    # ### end Alembic commands ###
+
+
+def downgrade():
+    # ### commands auto generated by Alembic - please adjust! ###
+    with op.batch_alter_table('tasks_cloud_backup', schema=None) as batch_op:
+        batch_op.add_column(sa.Column('bwlimit', sa.TEXT(), nullable=False))
+        batch_op.add_column(sa.Column('transfers', sa.INTEGER(), nullable=True))
+        batch_op.drop_column('transfer_setting')
+
+    # ### end Alembic commands ###

src/middlewared/middlewared/alembic/versions/25.04/2024-11-07_15-49_merge.py

@@ -0,0 +1,24 @@
+"""Merge
+
+Revision ID: 0ec74f350c66
+Revises: 0e1cd4d5fcf0, 8eacd2bb3e18
+Create Date: 2024-11-07 15:49:27.544418+00:00
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '0ec74f350c66'
+down_revision = ('0e1cd4d5fcf0', '8eacd2bb3e18')
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+    pass
+
+
+def downgrade():
+    pass

src/middlewared/middlewared/plugins/cloud/crud.py

@@ -78,10 +78,6 @@ async def _validate(self, app, verrors, name, data):
 
             await provider.validate_task_full(data, credentials, verrors)
 
-        for i, (limit1, limit2) in enumerate(zip(data["bwlimit"], data["bwlimit"][1:])):
-            if limit1["time"] >= limit2["time"]:
-                verrors.add(f"{name}.bwlimit.{i + 1}.time", f"Invalid time order: {limit1['time']}, {limit2['time']}")
-
         if self.allow_zvol and (path := await self.get_path_field(data)).startswith("/dev/zvol/"):
             zvol = zvol_path_to_name(path)
             if not await self.middleware.call('pool.dataset.query', [['name', '=', zvol], ['type', '=', 'VOLUME']]):

src/middlewared/middlewared/plugins/cloud/model.py

@@ -2,7 +2,6 @@
 
 from middlewared.schema import Bool, Cron, Dict, Int, List, Str
 import middlewared.sqlalchemy as sa
-from middlewared.validators import Range, Time
 
 
 class CloudTaskModelMixin:
@@ -23,10 +22,8 @@ def credential_id(cls):
     pre_script = sa.Column(sa.Text())
     post_script = sa.Column(sa.Text())
     snapshot = sa.Column(sa.Boolean())
-    bwlimit = sa.Column(sa.JSON(list))
     include = sa.Column(sa.JSON(list))
     exclude = sa.Column(sa.JSON(list))
-    transfers = sa.Column(sa.Integer(), nullable=True)
     args = sa.Column(sa.Text())
     enabled = sa.Column(sa.Boolean(), default=True)
     job = sa.Column(sa.JSON(None))
@@ -45,12 +42,8 @@ def credential_id(cls):
     Str("pre_script", default="", max_length=None),
     Str("post_script", default="", max_length=None),
     Bool("snapshot", default=False),
-    List("bwlimit", items=[Dict("cloud_sync_bwlimit",
-                                Str("time", validators=[Time()]),
-                                Int("bandwidth", validators=[Range(min_=1)], null=True))]),
     List("include", items=[Str("path", empty=False)]),
     List("exclude", items=[Str("path", empty=False)]),
-    Int("transfers", null=True, default=None, validators=[Range(min_=1)]),
     Str("args", default="", max_length=None),
     Bool("enabled", default=True),
 ]

src/middlewared/middlewared/plugins/cloud_backup/crud.py

@@ -2,7 +2,7 @@
 from middlewared.common.attachment import LockableFSAttachmentDelegate
 from middlewared.plugins.cloud.crud import CloudTaskServiceMixin
 from middlewared.plugins.cloud.model import CloudTaskModelMixin, cloud_task_schema
-from middlewared.schema import accepts, Bool, Cron, Dict, Int, Password, Patch
+from middlewared.schema import accepts, Bool, Cron, Dict, Int, Password, Patch, Str
 from middlewared.service import pass_app, private, TaskPathService, ValidationErrors
 import middlewared.sqlalchemy as sa
 from middlewared.utils.path import FSLocation
@@ -16,6 +16,7 @@ class CloudBackupModel(CloudTaskModelMixin, sa.Model):
 
     password = sa.Column(sa.EncryptedText())
     keep_last = sa.Column(sa.Integer())
+    transfer_setting = sa.Column(sa.String(16), default="DEFAULT")
 
 
 class CloudBackupService(TaskPathService, CloudTaskServiceMixin, TaskStateMixin):
@@ -42,6 +43,14 @@ class Config:
         ("add", Bool("locked")),
     )
 
+    @private
+    def transfer_setting_args(self):
+        return {
+            "DEFAULT": [],
+            "PERFORMANCE": ["--pack-size", "29"],
+            "FAST_STORAGE": ["--pack-size", "58", "--read-concurrency", "100"]
+        }
+
     @private
     async def extend_context(self, rows, extra):
         return {
@@ -69,11 +78,17 @@ async def _compress(self, cloud_backup):
 
         return cloud_backup
 
+    @accepts()
+    def transfer_setting_choices(self):
+        args = self.transfer_setting_args()
+        return list(args.keys())
+
     @accepts(Dict(
         "cloud_backup_create",
         *cloud_task_schema,
         Password("password", required=True, empty=False),
         Int("keep_last", required=True, validators=[Range(min_=1)]),
+        Str("transfer_setting", enum=["DEFAULT", "PERFORMANCE", "FAST_STORAGE"], default="DEFAULT"),
         register=True,
     ))
     @pass_app(rest=True)

src/middlewared/middlewared/plugins/cloud_backup/sync.py

@@ -58,6 +58,9 @@ async def restic(middleware, job, cloud_backup, dry_run):
         if cmd is None:
             cmd = [local_path]
 
+        args = await middleware.call("cloud_backup.transfer_setting_args")
+        cmd.extend(args[cloud_backup["transfer_setting"]])
+
         if dry_run:
             cmd.append("-n")
 

src/middlewared/middlewared/plugins/cloud_sync.py

@@ -11,7 +11,7 @@
 from middlewared.plugins.cloud.path import get_remote_path, check_local_path
 from middlewared.plugins.cloud.remotes import REMOTES, remote_classes
 from middlewared.rclone.remote.storjix import StorjIxError
-from middlewared.schema import accepts, Bool, Cron, Dict, Int, Password, Patch, Str
+from middlewared.schema import accepts, Bool, Cron, Dict, Int, List, Password, Patch, Str
 from middlewared.service import (
     CallError, CRUDService, ValidationError, ValidationErrors, item_method, job, pass_app, private, TaskPathService,
 )
@@ -21,7 +21,7 @@
 from middlewared.utils.path import FSLocation
 from middlewared.utils.service.task_state import TaskStateMixin
 from middlewared.utils.time_utils import utc_now
-from middlewared.validators import validate_schema
+from middlewared.validators import Range, Time, validate_schema
 
 import aiorwlock
 import asyncio
@@ -683,6 +683,8 @@ class CloudSyncModel(CloudTaskModelMixin, sa.Model):
 
     direction = sa.Column(sa.String(10))
     transfer_mode = sa.Column(sa.String(20))
+    bwlimit = sa.Column(sa.JSON(list))
+    transfers = sa.Column(sa.Integer(), nullable=True)
 
     encryption = sa.Column(sa.Boolean())
     filename_encryption = sa.Column(sa.Boolean())
@@ -756,6 +758,10 @@ async def _basic_validate(self, verrors, name, data):
     async def _validate(self, app, verrors, name, data):
         await super()._validate(app, verrors, name, data)
 
+        for i, (limit1, limit2) in enumerate(zip(data["bwlimit"], data["bwlimit"][1:])):
+            if limit1["time"] >= limit2["time"]:
+                verrors.add(f"{name}.bwlimit.{i + 1}.time", f"Invalid time order: {limit1['time']}, {limit2['time']}")
+
         if data["snapshot"]:
             if data["direction"] != "PUSH":
                 verrors.add(f"{name}.snapshot", "This option can only be enabled for PUSH tasks")
@@ -800,6 +806,13 @@ async def _validate_folder(self, verrors, name, data):
         "cloud_sync_create",
         *cloud_task_schema,
 
+        List("bwlimit", items=[Dict(
+            "cloud_sync_bwlimit",
+            Str("time", validators=[Time()]),
+            Int("bandwidth", validators=[Range(min_=1)], null=True)
+        )]),
+        Int("transfers", null=True, default=None, validators=[Range(min_=1)]),
+
         Str("direction", enum=["PUSH", "PULL"], required=True),
         Str("transfer_mode", enum=["SYNC", "COPY", "MOVE"], required=True),
 

src/middlewared/middlewared/plugins/zfs_/validation_utils.py

@@ -1,9 +1,8 @@
 import libzfs
 
-from .utils import zvol_name_to_path
-
 
 def check_zvol_in_boot_pool_using_name(zvol_name: str) -> bool:
+    from .utils import zvol_name_to_path  # lazy import to avoid circular import when running `alembic merge heads`
     return check_zvol_in_boot_pool_using_path(zvol_name_to_path(zvol_name))
 
 

tests/api2/test_cloud_backup.py

@@ -62,7 +62,7 @@ def s3_credential():
 
 
 @pytest.fixture(scope="function")
-def cloud_backup_task(s3_credential):
+def cloud_backup_task(s3_credential, request):
     clean()
 
     with dataset("cloud_backup") as local_dataset:
@@ -75,6 +75,7 @@ def cloud_backup_task(s3_credential):
             },
             "password": "test",
             "keep_last": 100,
+            **getattr(request, "param", {})
         }) as t:
             yield types.SimpleNamespace(
                 local_dataset=local_dataset,
@@ -288,3 +289,23 @@ def test_sync_initializes_repo(cloud_backup_task):
     clean()
 
     call("cloud_backup.sync", cloud_backup_task.task["id"], job=True)
+
+
+def test_transfer_setting_choices():
+    assert call("cloud_backup.transfer_setting_choices") == ["DEFAULT", "PERFORMANCE", "FAST_STORAGE"]
+
+
+@pytest.mark.parametrize("cloud_backup_task, options", [
+    (
+        {"transfer_setting": "PERFORMANCE"},
+        "'--pack-size', '29'"
+    ),
+    (
+        {"transfer_setting": "FAST_STORAGE"},
+        "'--pack-size', '58', '--read-concurrency', '100'"
+    )
+], indirect=["cloud_backup_task"])
+def test_other_transfer_settings(cloud_backup_task, options):
+    run_task(cloud_backup_task.task)
+    result = ssh(f"grep '{options}' /var/log/middlewared.log")
+    assert result.strip() != ""