From 19976701c11bbee7a5f2aa47dd66b6d065eeaea9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20Th=C3=A9bault?= Date: Mon, 26 Aug 2024 03:18:35 +0900 Subject: [PATCH 1/2] Implement ALL target for snapshot, snaplist and snapremove --- doc/source/advanced-use.rst | 14 ++ iocage_cli/snaplist.py | 9 +- iocage_cli/snapremove.py | 14 +- iocage_cli/snapshot.py | 3 +- iocage_lib/iocage.py | 64 ++++++++- tests/conftest.py | 6 + tests/data_classes.py | 47 ++++++- tests/functional_tests/0018_snapshot_test.py | 21 +++ .../0019_list_snapshot_test.py | 46 ++++++- .../0020_remove_snapshot_test.py | 127 ++++++++++++++++++ 10 files changed, 335 insertions(+), 16 deletions(-) diff --git a/doc/source/advanced-use.rst b/doc/source/advanced-use.rst index 7f59f9a1..c71c8427 100644 --- a/doc/source/advanced-use.rst +++ b/doc/source/advanced-use.rst @@ -158,6 +158,8 @@ Snapshots are point-in-time copies of data, a safety point to which a jail can be reverted at any time. Initially, snapshots take up almost no space, as only changing data is recorded. +You may use **ALL** as a target jail name for these commands if you want to target all jails at once. + List snapshots for a jail: :command:`iocage snaplist [UUID|NAME]` @@ -168,6 +170,18 @@ Create a new snapshot: This creates a snapshot based on the current time. +:command:`iocage snapshot [UUID|NAME] -n [SNAPSHOT NAME]` + +This creates a snapshot with the given name. + +Delete a snapshot: + +:command:`iocage snapremove [UUID|NAME] -n [SNAPSHOT NAME]` + +Delete all snapshots from a jail (requires `-f / --force`): + +:command:`iocage snapremove [UUID|NAME] -n ALL -f` + .. index:: Resource Limits .. _Resource Limits: diff --git a/iocage_cli/snaplist.py b/iocage_cli/snaplist.py index 971b116e..80503ed5 100644 --- a/iocage_cli/snaplist.py +++ b/iocage_cli/snaplist.py @@ -43,9 +43,14 @@ def cli(header, jail, _long, _sort): snap_list = ioc.IOCage(jail=jail).snap_list(_long, _sort) if header: - table.header(["NAME", "CREATED", "RSIZE", "USED"]) + if jail == 'ALL': + cols = ["JAIL"] + else: + cols = [] + cols.extend(["NAME", "CREATED", "RSIZE", "USED"]) + table.header(cols) # We get an infinite float otherwise. - table.set_cols_dtype(["t", "t", "t", "t"]) + table.set_cols_dtype(["t"]*len(cols)) table.add_rows(snap_list, header=False) ioc_common.logit({ "level" : "INFO", diff --git a/iocage_cli/snapremove.py b/iocage_cli/snapremove.py index 3a19185d..1ab2fc8c 100644 --- a/iocage_cli/snapremove.py +++ b/iocage_cli/snapremove.py @@ -25,6 +25,7 @@ import click +import iocage_lib.ioc_common as ioc_common import iocage_lib.iocage as ioc @@ -32,6 +33,15 @@ @click.argument("jail") @click.option("--name", "-n", help="The snapshot name. This will be what comes" " after @", required=True) -def cli(jail, name): +@click.option("--force", "-f", is_flag=True, default=False, + help="Force removal (required for -n ALL)") +def cli(jail, name, force): """Removes a snapshot from a user supplied jail.""" - ioc.IOCage(jail=jail).snap_remove(name) + if name == 'ALL' and not force: + ioc_common.logit({ + "level": "EXCEPTION", + "message": 'Usage: iocage snapremove [OPTIONS] JAILS...\n' + '\nError: Mass snapshot deletion requires "force" (-f).' + }) + skip_jails = jail != 'ALL' + ioc.IOCage(jail=jail, skip_jails=skip_jails).snap_remove(name) diff --git a/iocage_cli/snapshot.py b/iocage_cli/snapshot.py index 09245b25..37db1939 100644 --- a/iocage_cli/snapshot.py +++ b/iocage_cli/snapshot.py @@ -36,4 +36,5 @@ " after @", required=False) def cli(jail, name): """Snapshot a jail.""" - ioc.IOCage(jail=jail, skip_jails=True).snapshot(name) + skip_jails = jail != 'ALL' + ioc.IOCage(jail=jail, skip_jails=skip_jails).snapshot(name) diff --git a/iocage_lib/iocage.py b/iocage_lib/iocage.py index f52c7d7e..6e1fef84 100644 --- a/iocage_lib/iocage.py +++ b/iocage_lib/iocage.py @@ -1648,8 +1648,22 @@ def set(self, prop, plugin=False, rename=False): rtsold_enable = "YES" if "accept_rtadv" in value else "NO" ioc_common.set_rcconf(path, "rtsold_enable", rtsold_enable) + def snap_list_all(self, long, _sort): + self._all = False + snap_list = [] + for jail in self.jails: + self.jail = jail + snap_list.extend( + [[jail, *snap] for snap in self.snap_list(long, _sort)] + ) + sort = ioc_common.ioc_sort("snaplist", _sort, data=snap_list) + snap_list.sort(key=sort) + return snap_list + def snap_list(self, long=True, _sort="created"): """Gathers a list of snapshots and returns it""" + if self._all: + return self.snap_list_all(long=long, _sort=_sort) uuid, path = self.__check_jail_existence__() conf = ioc_json.IOCJson(path, silent=self.silent).json_get_value('all') snap_list = [] @@ -1709,8 +1723,20 @@ def snap_list(self, long=True, _sort="created"): return snap_list + def snapshot_all(self, name): + # We want a consistent name across a snapshot batch. + if not name: + name = datetime.datetime.utcnow().strftime("%F_%T") + self._all = False + for jail in self.jails: + self.jail = jail + self.snapshot(name) + def snapshot(self, name): """Will create a snapshot for the given jail""" + if self._all: + self.snapshot_all(name) + return date = datetime.datetime.utcnow().strftime("%F_%T") uuid, path = self.__check_jail_existence__() @@ -2160,8 +2186,44 @@ def debug(self, directory): ioc_debug.IOCDebug(directory).run_debug() - def snap_remove(self, snapshot): + def _get_cloned_datasets(self): + return { + d.properties.get('origin', "").replace('/root@', '@') + for d in Dataset( + os.path.join(self.pool, 'iocage') + ).get_dependents(depth=3) + } + + def snap_remove_all(self, snapshot): + self._all = False + cloned_datasets=self._get_cloned_datasets() + + for jail in self.jails: + self.jail = jail + self.snap_remove(snapshot, cloned_datasets=cloned_datasets) + + def snap_remove(self, snapshot, cloned_datasets=None): """Removes user supplied snapshot from jail""" + if self._all: + self.snap_remove_all(snapshot) + return + if snapshot == 'ALL': + if cloned_datasets is None: + cloned_datasets = self._get_cloned_datasets() + for snapshot, *_ in reversed(self.snap_list()): + if snapshot in cloned_datasets: + ioc_common.logit({ + 'level': 'WARNING', + 'message': f"Skipped snapshot {snapshot}: used by clones." + }) + elif snapshot.rsplit('@', 1)[0].endswith('/root'): + # Deleting here would result in trying to delete + # the jail dataset-level snapshot twice since we construct + # the target based on the uuid, not path, below. + continue + else: + self.snap_remove(snapshot.rsplit('@', 1)[-1]) + return uuid, path = self.__check_jail_existence__() conf = ioc_json.IOCJson(path, silent=self.silent).json_get_value('all') diff --git a/tests/conftest.py b/tests/conftest.py index fbb84389..be54a33f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -315,6 +315,12 @@ def jail(): return Jail +@pytest.fixture +def snapshot(): + from tests.data_classes import Snapshot + return Snapshot + + @pytest.fixture def resource_selector(): from tests.data_classes import ResourceSelector diff --git a/tests/data_classes.py b/tests/data_classes.py index db12eead..6285a400 100644 --- a/tests/data_classes.py +++ b/tests/data_classes.py @@ -27,7 +27,7 @@ def __init__(self, raw_data, r_type=None): for attr in [ 'name', 'jid', 'state', 'release', 'ip4', 'ip6', 'orig_release', 'boot', 'type', 'template', 'basejail', 'crt', 'res', 'qta', - 'use', 'ava', 'created', 'rsize', 'used', 'orig_name' + 'use', 'ava', 'created', 'rsize', 'used', 'orig_name', 'jail' ]: setattr(self, attr, None) @@ -268,6 +268,10 @@ def snapshot_parse(self): self.name, self.created, self.rsize, self.used = self.standard_parse() + def snapall_parse(self): + self.jail, self.name, self.created, self.rsize, self.used = self.standard_parse() + + class ZFS: # TODO: Improve how we manage zfs object here pool = None @@ -361,13 +365,25 @@ class Resource: DEFAULT_JSON_FILE = 'config.json' def __init__(self, name, zfs=None): - self.name = name - self.zfs = ZFS() if not zfs else zfs + super().__setattr__('name', name) + super().__setattr__('zfs', ZFS() if not zfs else zfs) assert isinstance(self.zfs, ZFS) is True + def __eq__(self, other): + return self.name == other.name + + def __hash__(self): + return hash(self.name) + def __repr__(self): return self.name + def __setattr__(self, name, attr_value): + raise AttributeError(f"Resources are immutable. Cannot set attribute '{name}'.") + + def __delattr__(self, name): + raise AttributeError(f"Resources are immutable. Cannot delete attribute '{name}'.") + def convert_to_row(self, **kwargs): raise NotImplemented @@ -376,12 +392,12 @@ class Snapshot(Resource): def __init__(self, name, parent_dataset, zfs=None): super().__init__(name, zfs) - self.parent = parent_dataset + object.__setattr__(self, 'parent', parent_dataset) if isinstance(self.parent, str): - self.parent = Jail(self.parent) + object.__setattr__(self, 'parent', Jail(self.parent)) if self.exists: for k, v in self.zfs.get_snapshot_safely(self.name).items(): - setattr(self, k, v) + object.__setattr__(self, k, v) @property def exists(self): @@ -625,7 +641,7 @@ def is_rcjail(self): @property def is_cloned(self): return bool( - self.jail_dataset[ + self.root_dataset[ 'properties' ].get('origin', {}).get('value') ) @@ -795,3 +811,20 @@ def jails_with_prop(self, key, value): return [ j for j in self.all_jails if j.config.get(key, None) == value ] + + @property + def cloned_snapshots_set(self): + cloned_jails = self.cloned_jails + origins = { + jail.root_dataset['properties']['origin']['value'] + for jail in cloned_jails + } + origins |= { + jail.jail_dataset['properties']['origin']['value'] + for jail in cloned_jails + } + origins -= { "" } + return { + Snapshot(origin, origin.rsplit('@', 1)[0]) + for origin in origins + } diff --git a/tests/functional_tests/0018_snapshot_test.py b/tests/functional_tests/0018_snapshot_test.py index c305afe8..67df2ac3 100644 --- a/tests/functional_tests/0018_snapshot_test.py +++ b/tests/functional_tests/0018_snapshot_test.py @@ -30,6 +30,7 @@ SNAP_NAME = 'snaptest' +SNAPALL_NAME = 'snapalltest' def common_function(invoke_cli, jails, skip_test): @@ -46,6 +47,20 @@ def common_function(invoke_cli, jails, skip_test): ].count(SNAP_NAME) >= 2 +def all_jails_function(invoke_cli, jails, skip_test): + skip_test(not jails) + + invoke_cli( + ['snapshot', 'ALL', '-n', SNAPALL_NAME] + ) + + for jail in jails: + # We use count because of template and cloned jails + assert [ + s.id.split('@')[1] for s in jail.recursive_snapshots + ].count(SNAPALL_NAME) >= 2 + + @require_root @require_zpool def test_01_snapshot_of_jail(invoke_cli, resource_selector, skip_test): @@ -66,3 +81,9 @@ def test_02_snapshot_of_template_jail(invoke_cli, resource_selector, skip_test): @require_zpool def test_03_snapshot_of_cloned_jail(invoke_cli, resource_selector, skip_test): common_function(invoke_cli, resource_selector.cloned_jails, skip_test) + + +@require_root +@require_zpool +def test_04_snapshot_of_all_jails(invoke_cli, resource_selector, skip_test): + all_jails_function(invoke_cli, resource_selector.all_jails, skip_test) diff --git a/tests/functional_tests/0019_list_snapshot_test.py b/tests/functional_tests/0019_list_snapshot_test.py index 371191d8..033b4ce9 100644 --- a/tests/functional_tests/0019_list_snapshot_test.py +++ b/tests/functional_tests/0019_list_snapshot_test.py @@ -37,7 +37,10 @@ def common_function( jails_as_rows, full=False ): for flag in SORTING_FLAGS: - command = ['snaplist', jail.name, '-s', flag] + if type(jail) is list: + command = ['snaplist', 'ALL', '-s', flag] + else: + command = ['snaplist', jail.name, '-s', flag] if full: command.append('-l') @@ -45,8 +48,17 @@ def common_function( command ) - orig_list = parse_rows_output(result.output, 'snapshot') - verify_list = jails_as_rows(jail.recursive_snapshots, full=full) + if type(jail) is list: + jails = jail + orig_list = parse_rows_output(result.output, 'snapall') + verify_list = [] + for jail in jails: + for row in jails_as_rows(jail.recursive_snapshots, full=full): + row.jail = jail.name + verify_list.append(row) + else: + orig_list = parse_rows_output(result.output, 'snapshot') + verify_list = jails_as_rows(jail.recursive_snapshots, full=full) verify_list.sort(key=lambda r: r.sort_flag(flag)) @@ -89,3 +101,31 @@ def test_03_list_snapshots_of_jail_with_long_flag( common_function( invoke_cli, jails[0], parse_rows_output, jails_as_rows, True ) + + +@require_root +@require_zpool +def test_04_list_snapshots_of_all_jails( + invoke_cli, resource_selector, skip_test, + parse_rows_output, jails_as_rows +): + jails = resource_selector.all_jails_having_snapshots + skip_test(not jails) + + common_function( + invoke_cli, jails, parse_rows_output, jails_as_rows + ) + + +@require_root +@require_zpool +def test_05_list_snapshots_of_all_jails_with_long_flag( + invoke_cli, resource_selector, skip_test, + parse_rows_output, jails_as_rows +): + jails = resource_selector.all_jails_having_snapshots + skip_test(not jails) + + common_function( + invoke_cli, jails, parse_rows_output, jails_as_rows, True + ) diff --git a/tests/functional_tests/0020_remove_snapshot_test.py b/tests/functional_tests/0020_remove_snapshot_test.py index 1b863e26..bf9bbbb3 100644 --- a/tests/functional_tests/0020_remove_snapshot_test.py +++ b/tests/functional_tests/0020_remove_snapshot_test.py @@ -29,6 +29,7 @@ require_zpool = pytest.mark.require_zpool SNAP_NAME = 'snaptest' +SNAPALL_NAME = 'snapalltest' @require_root @@ -60,3 +61,129 @@ def test_01_remove_snapshot(invoke_cli, resource_selector, skip_test): ) assert remove_snap.exists is False + + +@require_root +@require_zpool +def test_02_remove_snapshot_of_all_jails( + invoke_cli, resource_selector, skip_test): + jails = resource_selector.all_jails + skip_test(not jails) + + snap_jails = [] + for jail in jails: + if any( + SNAPALL_NAME in snap.id for snap in jail.recursive_snapshots + ): + snap_jails.append(jail) + + skip_test(not snap_jails) + + remove_snaps = [] + for snap_jail in snap_jails: + for snap in snap_jail.recursive_snapshots: + if SNAPALL_NAME in snap.id: + remove_snaps.append(snap) + + assert all(snap.exists is True for snap in remove_snaps) + + invoke_cli( + ['snapremove', '-n', SNAPALL_NAME, 'ALL'] + ) + + assert all(snap.exists is False for snap in remove_snaps) + +@require_root +@require_zpool +def test_03_remove_all_snapshots_fail(invoke_cli, resource_selector, skip_test): + jails = resource_selector.all_jails_having_snapshots + skip_test(not jails) + + snap_jail = None + for jail in jails: + if (not jail.is_template and not jail.is_cloned and + len(jail.recursive_snapshots)>0): + snap_jail = jail + break + + skip_test(not snap_jail) + + failremove_snap = snap_jail.recursive_snapshots[0] + + assert failremove_snap.exists is True + + # Voluntarily forgetting the -f force flag + invoke_cli( + ['snapremove', '-n', 'ALL', snap_jail.name], + assert_returncode=False + ) + + assert failremove_snap.exists is True + + +@require_root +@require_zpool +def test_04_remove_all_snapshots_success(invoke_cli, resource_selector, + snapshot, skip_test): + jails = resource_selector.all_jails_having_snapshots + skip_test(not jails) + + snap_jail = None + for jail in jails: + if (not jail.is_template and not jail.is_cloned and + len(jail.recursive_snapshots)>1): + snap_jail = jail + break + + skip_test(not snap_jail) + + remove_snaps = set(snap_jail.recursive_snapshots) + assert all(snap.exists is True for snap in remove_snaps) + + cloned_snaps = resource_selector.cloned_snapshots_set + assert all(snap.exists is True for snap in cloned_snaps) + + filtered_remove_snaps = remove_snaps - { + snapshot(s, s.rsplit('@', 1)[0]) + for snap in cloned_snaps + for s in (snap.name.replace('/root@', '@'), snap.name) + } + + result = invoke_cli( + ['snapremove', '-n', 'ALL', snap_jail.name, '--force'] + ) + + assert all(snap.exists is False for snap in filtered_remove_snaps) + assert all(snap.exists is True for snap in cloned_snaps) + + +@require_root +@require_zpool +def test_05_remove_all_snapshots_all_jails(invoke_cli, resource_selector, + snapshot, skip_test): + jails = resource_selector.all_jails_having_snapshots + skip_test(not jails) + + cloned_snaps = resource_selector.cloned_snapshots_set + assert all(snap.exists is True for snap in cloned_snaps) + + remove_snaps = { + snap for jail in jails for snap in jail.recursive_snapshots + } + assert all(snap.exists is True for snap in remove_snaps) + + # We want to keep cloned jail datasets, cloned root datasets, + # and non-cloned jail datasets that contain cloned root datasets. + # This last case happens when creating jails from templates. + filtered_remove_snaps = remove_snaps - { + snapshot(s, s.rsplit('@', 1)[0]) + for snap in cloned_snaps + for s in (snap.name.replace('/root@', '@'), snap.name) + } + + result = invoke_cli( + ['snapremove', '-n', 'ALL', 'ALL', '--force'] + ) + + assert all(snap.exists is False for snap in filtered_remove_snaps) + assert all(snap.exists is True for snap in cloned_snaps) From 700f5d69c6a3f8f57ddfc4e485d1ac68bed6e4d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Quentin=20Th=C3=A9bault?= Date: Sat, 21 Sep 2024 04:49:50 +0900 Subject: [PATCH 2/2] Apply suggestions from code review Use isinstance() instead of type() Co-authored-by: Vincent Barbaresi --- tests/functional_tests/0019_list_snapshot_test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/functional_tests/0019_list_snapshot_test.py b/tests/functional_tests/0019_list_snapshot_test.py index 033b4ce9..dc88fb47 100644 --- a/tests/functional_tests/0019_list_snapshot_test.py +++ b/tests/functional_tests/0019_list_snapshot_test.py @@ -37,7 +37,7 @@ def common_function( jails_as_rows, full=False ): for flag in SORTING_FLAGS: - if type(jail) is list: + if isinstance(jail, list): command = ['snaplist', 'ALL', '-s', flag] else: command = ['snaplist', jail.name, '-s', flag] @@ -48,7 +48,7 @@ def common_function( command ) - if type(jail) is list: + if isinstance(jail, list): jails = jail orig_list = parse_rows_output(result.output, 'snapall') verify_list = []