Skip to content

NAS-137223 / 26.04 / remove most internal calls to zfs.snapshot.query #17022

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Aug 19, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions src/middlewared/middlewared/plugins/apps/rollback.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,10 +71,13 @@ def rollback(self, job, app_name, options):
if options['rollback_snapshot'] and (
app_volume_ds := self.middleware.call_sync('app.get_app_volume_ds', app_name)
):
ds = self.middleware.call_sync(
'zfs.resource.query_impl',
{'paths': [app_volume_ds], 'properties': None, 'get_snapshots': True}
)
snap_name = f'{app_volume_ds}@{options["app_version"]}'
if self.middleware.call_sync('zfs.snapshot.query', [['id', '=', snap_name]]):
if ds and snap_name in ds[0]['snapshots']:
job.set_progress(40, f'Rolling back {app_name!r} app to {options["app_version"]!r} version')

self.middleware.call_sync(
'zfs.snapshot.rollback', snap_name, {
'force': True,
Expand Down
12 changes: 10 additions & 2 deletions src/middlewared/middlewared/plugins/apps/upgrade.py
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,12 @@ def take_snapshot_of_hostpath_and_stop_app(self, app, snapshot_hostpath):

continue

ds = self.middleware.call_sync(
'zfs.resource.query_impl',
{'paths': [dataset], 'properties': None, 'get_snapshots': True}
)
snap_name = f'{dataset}@{get_upgrade_snap_name(app_info["name"], app_info["version"])}'
if self.middleware.call_sync('zfs.snapshot.query', [['id', '=', snap_name]]):
if ds and snap_name in ds[0]['snapshots']:
logger.debug('Snapshot %r already exists for %r app', snap_name, app_info['name'])
continue

Expand Down Expand Up @@ -138,8 +142,12 @@ def upgrade(self, job, app_name, options):
job.set_progress(40, f'Configuration updated for {app_name!r}, upgrading app')

if app_volume_ds := self.middleware.call_sync('app.get_app_volume_ds', app_name):
ds = self.middleware.call_sync(
'zfs.resource.query_impl',
{'paths': [app_volume_ds], 'properties': None, 'get_snapshots': True}
)
snap_name = f'{app_volume_ds}@{app["version"]}'
if self.middleware.call_sync('zfs.snapshot.query', [['id', '=', snap_name]]):
if ds and snap_name in ds[0]['snapshots']:
self.middleware.call_sync('zfs.snapshot.delete', snap_name, {'recursive': True})

self.middleware.call_sync(
Expand Down
44 changes: 30 additions & 14 deletions src/middlewared/middlewared/plugins/docker/backup.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import datetime
import errno
import logging
import os
import shutil

import yaml
from datetime import datetime

from middlewared.api import api_method
from middlewared.api.current import (
Expand Down Expand Up @@ -42,14 +43,18 @@ def backup(self, job, backup_name):
"""
self.middleware.call_sync('docker.state.validate')
docker_config = self.middleware.call_sync('docker.config')
name = backup_name or datetime.now().strftime('%F_%T')
name = backup_name or datetime.datetime.now().strftime('%F_%T')
if not validate_snapshot_name(f'a@{name}'):
# The a@ added is just cosmetic as the function requires a complete snapshot name
# with the dataset name included in it
raise CallError(f'{name!r} is not a valid snapshot name. It should be a valid ZFS snapshot name')

snap_name = BACKUP_NAME_PREFIX + name
if self.middleware.call_sync('zfs.snapshot.query', [['id', '=', f'{docker_config["dataset"]}@{snap_name}']]):
ds = self.middleware.call_sync(
'zfs.resource.query_impl',
{'paths': [docker_config['dataset']], 'properties': None, 'get_snapshots': True}
)
if ds and snap_name in ds[0]['snapshots']:
raise CallError(f'{snap_name!r} snapshot already exists', errno=errno.EEXIST)

if name in self.list_backups():
Expand Down Expand Up @@ -95,13 +100,21 @@ def list_backups(self):

backups_base_dir = backup_ds_path()
backups = {}
snapshots = self.middleware.call_sync(
'zfs.snapshot.query', [
['name', '^', f'{docker_config["dataset"]}@{BACKUP_NAME_PREFIX}']
], {'select': ['name']}
ds = self.middleware.call_sync(
'zfs.resource.query_impl',
{'paths': [docker_config['dataset']], 'properties': None, 'get_snapshots': True}
)
for snapshot in snapshots:
backup_name = snapshot['name'].split('@', 1)[-1].split(BACKUP_NAME_PREFIX, 1)[-1]
if not ds:
return backups
elif not ds[0]["snapshots"]:
return backups

prefix = f'{docker_config["dataset"]}@{BACKUP_NAME_PREFIX}'
for snap_name, snap_info in ds[0]['snapshots'].items():
if not snap_name.startswith(prefix):
continue

backup_name = snap_name.split('@', 1)[-1].split(BACKUP_NAME_PREFIX, 1)[-1]
backup_path = os.path.join(backups_base_dir, backup_name)
if not os.path.exists(backup_path):
continue
Expand All @@ -115,10 +128,13 @@ def list_backups(self):
backups[backup_name] = {
'name': backup_name,
'apps': [{k: app[k] for k in ('id', 'name', 'state')} for app in apps.values()],
'snapshot_name': snapshot['name'],
'created_on': str(self.middleware.call_sync(
'zfs.snapshot.get_instance', snapshot['name']
)['properties']['creation']['parsed']),
'snapshot_name': snap_name,
'created_on': str(
datetime.datetime.fromtimestamp(
snap_info["properties"]["creation"]["value"],
datetime.UTC
)
),
'backup_path': backup_path,
}

Expand Down Expand Up @@ -168,7 +184,7 @@ async def post_system_update_hook(middleware):
break

backup_job = await middleware.call(
'docker.backup', f'{UPDATE_BACKUP_PREFIX}-{datetime.now().strftime("%F_%T")}'
'docker.backup', f'{UPDATE_BACKUP_PREFIX}-{datetime.datetime.now().strftime("%F_%T")}'
)
await backup_job.wait()
if backup_job.error:
Expand Down
6 changes: 5 additions & 1 deletion src/middlewared/middlewared/plugins/docker/migrate.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,5 +84,9 @@ async def replicate_apps_dataset(self, new_pool, old_pool, migration_options):
finally:
await self.middleware.call('zfs.snapshot.delete', snap_details['id'], {'recursive': True})
snap_name = f'{applications_ds_name(new_pool)}@{snap_details["snapshot_name"]}'
if await self.middleware.call('zfs.snapshot.query', [['id', '=', snap_name]]):
ds = await self.middleware.call(
'zfs.resource.query_impl',
{'paths': [applications_ds_name(new_pool)], 'properties': None, 'get_snapshots': True}
)
if ds and snap_name in ds[0]['snapshots']:
await self.middleware.call('zfs.snapshot.delete', snap_name, {'recursive': True})
10 changes: 6 additions & 4 deletions src/middlewared/middlewared/plugins/vm/clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,18 @@ async def __next_clone_name(self, name):
return clone_name

async def __clone_zvol(self, name, zvol, created_snaps, created_clones):
if not await self.middleware.call(
'zfs.resource.query_impl', {'paths': [zvol], 'properties': None}
):
zz = await self.middleware.call(
'zfs.resource.query_impl',
{'paths': [zvol], 'properties': None, 'get_snapshots': True}
)
if not zz:
raise CallError(f'zvol {zvol} does not exist.', errno.ENOENT)

snapshot_name = name
i = 0
while True:
zvol_snapshot = f'{zvol}@{snapshot_name}'
if await self.middleware.call('zfs.snapshot.query', [('id', '=', zvol_snapshot)]):
if zvol_snapshot in zz[0]['snapshots']:
if ZVOL_CLONE_RE.search(snapshot_name):
snapshot_name = ZVOL_CLONE_RE.sub(rf'\1{ZVOL_CLONE_SUFFIX}{i}', snapshot_name)
else:
Expand Down
6 changes: 5 additions & 1 deletion tests/api2/test_account_privilege_role.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,11 @@ def test_can_read_with_read_or_write_role(role):
with dataset("test_snapshot_read") as ds:
with snapshot(ds, "test"):
with unprivileged_user_client([role]) as c:
assert len(c.call("pool.snapshot.query", [["dataset", "=", ds]])) == 1
snaps = c.call(
"zfs.resource.query",
{"paths": [ds], "properties": None, "get_snapshots": True}
)[0]["snapshots"]
assert len(snaps) == 1


def test_can_not_write_with_read_role():
Expand Down
6 changes: 5 additions & 1 deletion tests/api2/test_cloud_backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,7 +451,11 @@ def test_snapshot(s3_credential):
]) == ["blob", "blob2"]

time.sleep(1)
assert call("pool.snapshot.query", [["dataset", "=", ds]]) == []
snaps = call(
"zfs.resource.query",
{"paths": [ds], "properties": None, "get_snapshots": True}
)[0]["snapshots"]
assert len(snaps) == 0


@pytest.mark.parametrize("cloud_backup_task, expected", [(
Expand Down
6 changes: 5 additions & 1 deletion tests/api2/test_cloud_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,11 @@ def test_snapshot(has_zvol_sibling):

time.sleep(1)

assert call("pool.snapshot.query", [["dataset", "=", ds]]) == []
snaps = call(
"zfs.resource.query",
{"paths": [ds], "properties": None, "get_snapshots": True}
)[0]["snapshots"]
assert len(snaps) == 0
finally:
if has_zvol_sibling:
ssh(f"zfs destroy -r {pool}/zvol")
Expand Down