From fe8dbc2729ecfcbce551b9f612ecab44a22189a5 Mon Sep 17 00:00:00 2001 From: bugclerk <40872210+bugclerk@users.noreply.github.com> Date: Thu, 7 Nov 2024 08:12:13 -0800 Subject: [PATCH] NAS-132072 / 25.04 / Add transfer setting to Truecloud Backup (by creatorcary) (#14882) * remove bwlimit and transfers from cloud_backup (cherry picked from commit 51b7d9a82830cbfaf6fbf871a77b97788e400a9b) * add transfer_setting (cherry picked from commit 4907c38ecdb8788edba64c9b4eff262368a37139) * migration (cherry picked from commit 2c533a0667afc494764b9dc1b4cf1a52decf5f41) * add transfer_setting_choices endpoint (cherry picked from commit 48961daca8cb7b800054a1c3cf238a869644917c) * add tests (cherry picked from commit 7fac9b27717d8e00daea0294b058b72e99188c12) * fix new test (cherry picked from commit d9dec1429c3dfd6996b06c5207c1f1413d89179d) * fix new test (cherry picked from commit ad8c2dd0cc25dccda5332b2efa9690c3adc089ea) * fix circular import * merge migration * add import comment --------- Co-authored-by: Logan Cary --- ...-04_20-26_cloud_backup_transfer_setting.py | 36 +++++++++++++++++++ .../versions/25.04/2024-11-07_15-49_merge.py | 24 +++++++++++++ .../middlewared/plugins/cloud/crud.py | 4 --- .../middlewared/plugins/cloud/model.py | 7 ---- .../middlewared/plugins/cloud_backup/crud.py | 17 ++++++++- .../middlewared/plugins/cloud_backup/sync.py | 3 ++ .../middlewared/plugins/cloud_sync.py | 17 +++++++-- .../plugins/zfs_/validation_utils.py | 3 +- tests/api2/test_cloud_backup.py | 23 +++++++++++- 9 files changed, 117 insertions(+), 17 deletions(-) create mode 100644 src/middlewared/middlewared/alembic/versions/24.10/2024-11-04_20-26_cloud_backup_transfer_setting.py create mode 100644 src/middlewared/middlewared/alembic/versions/25.04/2024-11-07_15-49_merge.py diff --git a/src/middlewared/middlewared/alembic/versions/24.10/2024-11-04_20-26_cloud_backup_transfer_setting.py b/src/middlewared/middlewared/alembic/versions/24.10/2024-11-04_20-26_cloud_backup_transfer_setting.py new file mode 100644 index 000000000000..576c54e34995 --- /dev/null +++ b/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 ### diff --git a/src/middlewared/middlewared/alembic/versions/25.04/2024-11-07_15-49_merge.py b/src/middlewared/middlewared/alembic/versions/25.04/2024-11-07_15-49_merge.py new file mode 100644 index 000000000000..36d8fbb68399 --- /dev/null +++ b/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 diff --git a/src/middlewared/middlewared/plugins/cloud/crud.py b/src/middlewared/middlewared/plugins/cloud/crud.py index ac615931ac38..5dbfeb836822 100644 --- a/src/middlewared/middlewared/plugins/cloud/crud.py +++ b/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']]): diff --git a/src/middlewared/middlewared/plugins/cloud/model.py b/src/middlewared/middlewared/plugins/cloud/model.py index 7b05535d2ba6..fc23485f3719 100644 --- a/src/middlewared/middlewared/plugins/cloud/model.py +++ b/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), ] diff --git a/src/middlewared/middlewared/plugins/cloud_backup/crud.py b/src/middlewared/middlewared/plugins/cloud_backup/crud.py index b880c7ee6a03..8b5574680b8b 100644 --- a/src/middlewared/middlewared/plugins/cloud_backup/crud.py +++ b/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) diff --git a/src/middlewared/middlewared/plugins/cloud_backup/sync.py b/src/middlewared/middlewared/plugins/cloud_backup/sync.py index 0f790920b385..e1f3e33d211d 100644 --- a/src/middlewared/middlewared/plugins/cloud_backup/sync.py +++ b/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") diff --git a/src/middlewared/middlewared/plugins/cloud_sync.py b/src/middlewared/middlewared/plugins/cloud_sync.py index bb07595f9758..b0ef43a9b60d 100644 --- a/src/middlewared/middlewared/plugins/cloud_sync.py +++ b/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), diff --git a/src/middlewared/middlewared/plugins/zfs_/validation_utils.py b/src/middlewared/middlewared/plugins/zfs_/validation_utils.py index 32de8429b856..1bd683451ca1 100644 --- a/src/middlewared/middlewared/plugins/zfs_/validation_utils.py +++ b/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)) diff --git a/tests/api2/test_cloud_backup.py b/tests/api2/test_cloud_backup.py index 6f2c5971c8e0..504cb4935439 100644 --- a/tests/api2/test_cloud_backup.py +++ b/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() != ""