From 1e885fff8eaf8c9e4e4587cb886f3a724445472b Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Fri, 16 Nov 2018 15:14:16 +0100 Subject: [PATCH 1/4] btrfs: add all subvolume commands (cherry picked from commit c476a2181d647c019872ae72ee46ce21faeeebc7) --- salt/modules/btrfs.py | 463 ++++++++++++++++++++++++++++++- tests/unit/modules/test_btrfs.py | 313 ++++++++++++++++++++- 2 files changed, 774 insertions(+), 2 deletions(-) diff --git a/salt/modules/btrfs.py b/salt/modules/btrfs.py index 36bfaeb12e7b..576eabd385e8 100644 --- a/salt/modules/btrfs.py +++ b/salt/modules/btrfs.py @@ -20,11 +20,11 @@ # Import Python libs from __future__ import absolute_import, print_function, unicode_literals +import itertools import os import re import uuid - # Import Salt libs import salt.utils.fsutils import salt.utils.platform @@ -673,3 +673,464 @@ def properties(obj, type=None, set=None): ret[prop]['value'] = value and value.split("=")[-1] or "N/A" return ret + + +def subvolume_create(name, dest=None, qgroupids=None): + ''' + Create subvolume `name` in `dest`. + + name + Name of the new subvolume + + dest + If not given, the subvolume will be created in the current + directory, if given will be in /dest/name + + qgroupids + Add the newly created subcolume to a qgroup. This parameter + is a list + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_create var + salt '*' btrfs.subvolume_create var dest=/mnt + salt '*' btrfs.subvolume_create var qgroupids='[200]' + + ''' + if qgroupids and type(qgroupids) is not list: + raise CommandExecutionError('Qgroupids parameter must be a list') + + cmd = ['btrfs', 'subvolume', 'create'] + + if type(qgroupids) is list: + cmd.append('-i') + cmd.extend(qgroupids) + if dest: + name = os.path.join(dest, name) + cmd.append(name) + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + return True + + +def subvolume_delete(name=None, names=None, commit=None): + ''' + Delete the subvolume(s) from the filesystem + + The user can remove one single subvolume (name) or multiple of + then at the same time (names). One of the two parameters needs to + specified. + + Please, refer to the documentation to understand the implication + on the transactions, and when the subvolume is really deleted. + + name + Name of the subvolume to remove + + names + List of names of subvolumes to remove + + commit + * 'after': Wait for transaction commit at the end + * 'each': Wait for transaction commit after each delete + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_delete /var/volumes/tmp + salt '*' btrfs.subvolume_delete /var/volumes/tmp commit=after + + ''' + if not name and not (names and type(names) is list): + raise CommandExecutionError('Provide a value for the name parameter') + + if commit and commit not in ('after', 'each'): + raise CommandExecutionError('Value for commit not recognized') + + cmd = ['btrfs', 'subvolume', 'delete'] + if commit == 'after': + cmd.append('--commit-after') + elif commit == 'each': + cmd.append('--commit-each') + if name: + cmd.append(name) + if type(names) is list: + cmd.extend(names) + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + return True + + +def subvolume_find_new(name, last_gen): + ''' + List the recently modified files in a subvolume + + name + Name of the subvolume + + last_gen + Last transid marker from where to compare + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_find_new /var/volumes/tmp 1024 + + ''' + cmd = ['btrfs', 'subvolume', 'find-new', name, last_gen] + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + + lines = res['stdout'].splitlines() + # Filenames are at the end of each inode line + files = [l.split()[-1] for l in lines if l.startswith('inode')] + # The last transid is in the last line + transid = lines[-1].split()[-1] + return { + 'files': files, + 'transid': transid, + } + + +def subvolume_get_default(path): + ''' + Get the default subvolume of the filesystem path + + path + Mount point for the subvolume + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_get_default /var/volumes/tmp + + ''' + cmd = ['btrfs', 'subvolume', 'get-default', path] + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + + line = res['stdout'].strip() + # The ID is the second parameter, and the name the last one, or + # '(FS_TREE)' + # + # When the default one is set: + # ID 5 (FS_TREE) + # + # When we manually set a different one (var): + # ID 257 gen 8 top level 5 path var + # + id_ = line.split()[1] + name = line.split()[-1] + return { + 'id': id_, + 'name': name, + } + + +def _pop(line, key, use_rest): + ''' + Helper for the line parser. + + If key is a prefix of line, will remove ir from the line and will + extract the value (space separation), and the rest of the line. + + If use_rest is True, the value will be the rest of the line. + + Return a tuple with the value and the rest of the line. + ''' + value = None + if line.startswith(key): + line = line[len(key):].strip() + if use_rest: + value = line + line = '' + else: + value, line = line.split(' ', 1) + return value, line.strip() + + +def subvolume_list(path, parent_id=False, absolute=False, + ogeneration=False, generation=False, + subvolumes=False, uuid=False, parent_uuid=False, + sent_subvolume_uuid=False, snapshots=False, + readonly=False, deleted=False, generation_cmp=None, + ogeneration_cmp=None, sort=None): + ''' + List the subvolumes present in the filesystem. + + path + Mount point for the subvolume + + parent_id + Print parent ID + + absolute + Print all the subvolumes in the filesystem and distinguish + between absolute and relative path with respect to the given + + + ogeneration + Print the ogeneration of the subvolume + + generation + Print the generation of the subvolume + + subvolumes + Print only subvolumes below specified + + uuid + Print the UUID of the subvolume + + parent_uuid + Print the parent uuid of subvolumes (and snapshots) + + sent_subvolume_uuid + Print the UUID of the sent subvolume, where the subvolume is + the result of a receive operation + + snapshots + Only snapshot subvolumes in the filesystem will be listed + + readonly + Only readonly subvolumes in the filesystem will be listed + + deleted + Only deleted subvolumens that are ye not cleaned + + generation_cmp + List subvolumes in the filesystem that its generation is >=, + <= or = value. '+' means >= value, '-' means <= value, If + there is neither '+' nor '-', it means = value + + ogeneration_cmp + List subvolumes in the filesystem that its ogeneration is >=, + <= or = value + + sort + List subvolumes in order by specified items. Possible values: + * rootid + * gen + * ogen + * path + You can add '+' or '-' in front of each items, '+' means + ascending, '-' means descending. The default is ascending. You + can combite it in a list. + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_list /var/volumes/tmp + salt '*' btrfs.subvolume_list /var/volumes/tmp path=True + salt '*' btrfs.subvolume_list /var/volumes/tmp sort='[-rootid]' + + ''' + if sort and type(sort) is not list: + raise CommandExecutionError('Sort parameter must be a list') + + valid_sorts = [ + ''.join((order, attrib)) for order, attrib in itertools.product( + ('-', '', '+'), ('rootid', 'gen', 'ogen', 'path')) + ] + if sort and not all(s in valid_sorts for s in sort): + raise CommandExecutionError('Value for sort not recognized') + + cmd = ['btrfs', 'subvolume', 'list'] + + params = ((parent_id, '-p'), + (absolute, '-a'), + (ogeneration, '-c'), + (generation, '-g'), + (subvolumes, '-o'), + (uuid, '-u'), + (parent_uuid, '-q'), + (sent_subvolume_uuid, '-R'), + (snapshots, '-s'), + (readonly, '-r'), + (deleted, '-d')) + cmd.extend(p[1] for p in params if p[0]) + + if generation_cmp: + cmd.extend(['-G', generation_cmp]) + + if ogeneration_cmp: + cmd.extend(['-C', ogeneration_cmp]) + + # We already validated the content of the list + if sort: + cmd.append('--sort={}'.format(','.join(sort))) + + cmd.append(path) + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + + # Parse the output. ID and gen are always at the begining, and + # path is always at the end. There is only one column that + # contains space (top level), and the path value can also have + # spaces. The issue is that we do not know how many spaces do we + # have in the path name, so any classic solution based on split + # will fail. + # + # This list is in order. + columns = ('ID', 'gen', 'cgen', 'parent', 'top level', 'otime', + 'parent_uuid', 'received_uuid', 'uuid', 'path') + result = [] + for line in res['stdout'].splitlines(): + table = {} + for key in columns: + value, line = _pop(line, key, key == 'path') + if value: + table[key.lower()] = value + # If line is not empty here, we are not able to parse it + if not line: + result.append(table) + + return result + + +def subvolume_set_default(subvolid, path): + ''' + Set the subvolume as default + + subvolid + ID of the new default subvolume + + path + Mount point for the filesystem + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_set_default 257 /var/volumes/tmp + + ''' + cmd = ['btrfs', 'subvolume', 'set-default', subvolid, path] + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + return True + + +def subvolume_show(path): + ''' + Show information of a given subvolume + + path + Mount point for the filesystem + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_show /var/volumes/tmp + + ''' + cmd = ['btrfs', 'subvolume', 'show', path] + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + + result = {} + table = {} + # The real name is the first line, later there is a table of + # values separated with colon. + stdout = res['stdout'].splitlines() + key = stdout.pop(0) + result[key.strip()] = table + + for line in stdout: + key, value = line.split(':', 1) + table[key.lower().strip()] = value.strip() + return result + + +def subvolume_snapshot(source, dest=None, name=None, read_only=False): + ''' + Create a snapshot of a source subvolume + + source + Source subvolume from where to create the snapshot + + dest + If only dest is given, the subvolume will be named as the + basename of the source + + name + Name of the snapshot + + read_only + Create a read only snapshot + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_snapshot /var/volumes/tmp dest=/.snapshots + salt '*' btrfs.subvolume_snapshot /var/volumes/tmp name=backup + + ''' + if not dest and not name: + raise CommandExecutionError('Provide parameter dest, name, or both') + + cmd = ['btrfs', 'subvolume', 'snapshot'] + if read_only: + cmd.append('-r') + if dest and not name: + cmd.append(dest) + if dest and name: + name = os.path.join(dest, name) + if name: + cmd.append(name) + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + return True + + +def subvolume_sync(path, subvolids=None, sleep=None): + ''' + Wait until given subvolume are completely removed from the + filesystem after deletion. + + path + Mount point for the filesystem + + subvolids + List of IDs of subvolumes to wait for + + sleep + Sleep N seconds betwenn checks (default: 1) + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_sync /var/volumes/tmp + salt '*' btrfs.subvolume_sync /var/volumes/tmp subvolids='[257]' + + ''' + if subvolids and type(subvolids) is not list: + raise CommandExecutionError('Subvolids parameter must be a list') + + cmd = ['btrfs', 'subvolume', 'sync'] + if sleep: + cmd.extend(['-s', sleep]) + + cmd.append(path) + if subvolids: + cmd.extend(subvolids) + + res = __salt__['cmd.run_all'](cmd) + salt.utils.fsutils._verify_run(res) + return True diff --git a/tests/unit/modules/test_btrfs.py b/tests/unit/modules/test_btrfs.py index ebd28a645184..ef31e68554a3 100644 --- a/tests/unit/modules/test_btrfs.py +++ b/tests/unit/modules/test_btrfs.py @@ -5,6 +5,8 @@ # Import python libs from __future__ import absolute_import, print_function, unicode_literals +import pytest + # Import Salt Testing Libs from tests.support.mixins import LoaderModuleMockMixin from tests.support.unit import TestCase, skipIf @@ -29,7 +31,7 @@ class BtrfsTestCase(TestCase, LoaderModuleMockMixin): Test cases for salt.modules.btrfs ''' def setup_loader_modules(self): - return {btrfs: {}} + return {btrfs: {'__salt__': {}}} # 'version' function tests: 1 def test_version(self): @@ -362,3 +364,312 @@ def test_properties_error(self): ''' self.assertRaises(CommandExecutionError, btrfs.properties, '/dev/sda1', 'subvol', True) + + def test_subvolume_create_fails_parameters(self): + ''' + Test btrfs subvolume create + ''' + # Fails when qgroupids is not a list + with pytest.raises(CommandExecutionError): + btrfs.subvolume_create('var', qgroupids='1') + + def test_subvolume_create(self): + ''' + Test btrfs subvolume create + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={'recode': 0}), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_create('var', dest='/mnt') + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'create', '/mnt/var']) + + def test_subvolume_delete_fails_parameters(self): + ''' + Test btrfs subvolume delete + ''' + # We need to provide name or names + with pytest.raises(CommandExecutionError): + btrfs.subvolume_delete() + + with pytest.raises(CommandExecutionError): + btrfs.subvolume_delete(names='var') + + def test_subvolume_delete_fails_parameter_commit(self): + ''' + Test btrfs subvolume delete + ''' + # Parameter commit can be 'after' or 'each' + with pytest.raises(CommandExecutionError): + btrfs.subvolume_delete(name='var', commit='maybe') + + def test_subvolume_delete(self): + ''' + Test btrfs subvolume delete + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={'recode': 0}), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_delete('var', names=['tmp']) + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'delete', 'var', 'tmp']) + + def test_subvolume_find_new_empty(self): + ''' + Test btrfs subvolume find-new + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={ + 'recode': 0, + 'stdout': 'transid marker was 1024' + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_find_new('var', '2000') == { + 'files': [], + 'transid': '1024' + } + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'find-new', 'var', '2000']) + + def test_subvolume_find_new(self): + ''' + Test btrfs subvolume find-new + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={ + 'recode': 0, + 'stdout': '''inode 185148 ... gen 2108 flags NONE var/log/audit/audit.log +inode 187390 ... INLINE etc/openvpn/openvpn-status.log +transid marker was 1024''' + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_find_new('var', '1023') == { + 'files': ['var/log/audit/audit.log', + 'etc/openvpn/openvpn-status.log'], + 'transid': '1024' + } + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'find-new', 'var', '1023']) + + def test_subvolume_get_default_free(self): + ''' + Test btrfs subvolume get-default + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={ + 'recode': 0, + 'stdout': 'ID 5 (FS_TREE)', + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_get_default('/mnt') == { + 'id': '5', + 'name': '(FS_TREE)', + } + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'get-default', '/mnt']) + + def test_subvolume_get_default(self): + ''' + Test btrfs subvolume get-default + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={ + 'recode': 0, + 'stdout': 'ID 257 gen 8 top level 5 path var', + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_get_default('/mnt') == { + 'id': '257', + 'name': 'var', + } + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'get-default', '/mnt']) + + def test_subvolume_list_fails_parameters(self): + ''' + Test btrfs subvolume list + ''' + # Fails when sort is not a list + with pytest.raises(CommandExecutionError): + btrfs.subvolume_list('/mnt', sort='-rootid') + + # Fails when sort is not recognized + with pytest.raises(CommandExecutionError): + btrfs.subvolume_list('/mnt', sort=['-root']) + + def test_subvolume_list_simple(self): + ''' + Test btrfs subvolume list + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={ + 'recode': 0, + 'stdout': '''ID 257 gen 8 top level 5 path one +ID 258 gen 10 top level 5 path another one +''', + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_list('/mnt') == [ + { + 'id': '257', + 'gen': '8', + 'top level': '5', + 'path': 'one', + }, + { + 'id': '258', + 'gen': '10', + 'top level': '5', + 'path': 'another one', + }, + ] + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'list', '/mnt']) + + def test_subvolume_list(self): + ''' + Test btrfs subvolume list + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={ + 'recode': 0, + 'stdout': '''\ +ID 257 gen 8 cgen 8 parent 5 top level 5 parent_uuid - received_uuid - \ + uuid 777...-..05 path one +ID 258 gen 10 cgen 10 parent 5 top level 5 parent_uuid - received_uuid - \ + uuid a90...-..01 path another one +''', + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_list('/mnt', parent_id=True, + absolute=True, + ogeneration=True, + generation=True, + subvolumes=True, uuid=True, + parent_uuid=True, + sent_subvolume_uuid=True, + generation_cmp='-100', + ogeneration_cmp='+5', + sort=['-rootid', 'gen']) == [ + { + 'id': '257', + 'gen': '8', + 'cgen': '8', + 'parent': '5', + 'top level': '5', + 'parent_uuid': '-', + 'received_uuid': '-', + 'uuid': '777...-..05', + 'path': 'one', + }, + { + 'id': '258', + 'gen': '10', + 'cgen': '10', + 'parent': '5', + 'top level': '5', + 'parent_uuid': '-', + 'received_uuid': '-', + 'uuid': 'a90...-..01', + 'path': 'another one', + }, + ] + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'list', '-p', '-a', '-c', '-g', + '-o', '-u', '-q', '-R', '-G', '-100', '-C', '+5', + '--sort=-rootid,gen', '/mnt']) + + def test_subvolume_set_default(self): + ''' + Test btrfs subvolume set-default + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={'recode': 0}), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_set_default('257', '/mnt') + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'set-default', '257', '/mnt']) + + def test_subvolume_show(self): + ''' + Test btrfs subvolume show + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={ + 'recode': 0, + 'stdout': '''@/var + Name: var + UUID: 7a14...-...04 + Parent UUID: - + Received UUID: - + Creation time: 2018-10-01 14:33:12 +0200 + Subvolume ID: 258 + Generation: 82479 + Gen at creation: 10 + Parent ID: 256 + Top level ID: 256 + Flags: - + Snapshot(s): +''', + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_show('/var') == { + '@/var': { + 'name': 'var', + 'uuid': '7a14...-...04', + 'parent uuid': '-', + 'received uuid': '-', + 'creation time': '2018-10-01 14:33:12 +0200', + 'subvolume id': '258', + 'generation': '82479', + 'gen at creation': '10', + 'parent id': '256', + 'top level id': '256', + 'flags': '-', + 'snapshot(s)': '', + }, + } + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'show', '/var']) + + def test_subvolume_sync_fail_parameters(self): + ''' + Test btrfs subvolume sync + ''' + # Fails when subvolids is not a list + with pytest.raises(CommandExecutionError): + btrfs.subvolume_sync('/mnt', subvolids='257') + + def test_subvolume_sync(self): + ''' + Test btrfs subvolume sync + ''' + salt_mock = { + 'cmd.run_all': MagicMock(return_value={'recode': 0}), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_sync('/mnt', subvolids=['257'], + sleep='1') + salt_mock['cmd.run_all'].assert_called_once() + salt_mock['cmd.run_all'].assert_called_with( + ['btrfs', 'subvolume', 'sync', '-s', '1', '/mnt', '257']) From 9b28bb0a247373c55946e41549906134a7030d7b Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Tue, 20 Nov 2018 11:48:42 +0100 Subject: [PATCH 2/4] btrfs: add subvolume_exists (cherry picked from commit 5de7ce4c09cf37ad4b20e03e8a77792a0a27e929) --- salt/modules/btrfs.py | 18 ++++++++++++++++++ tests/unit/modules/test_btrfs.py | 20 ++++++++++++++++++++ 2 files changed, 38 insertions(+) diff --git a/salt/modules/btrfs.py b/salt/modules/btrfs.py index 576eabd385e8..eb6e022e8999 100644 --- a/salt/modules/btrfs.py +++ b/salt/modules/btrfs.py @@ -675,6 +675,24 @@ def properties(obj, type=None, set=None): return ret +def subvolume_exists(path): + ''' + Check if a subvolume is present in the filesystem. + + path + Mount point for the subvolume (full path) + + CLI Example: + + .. code-block:: bash + + salt '*' btrfs.subvolume_exists /mnt/var + + ''' + cmd = ['btrfs', 'subvolume', 'show', path] + return __salt__['cmd.retcode'](cmd, ignore_retcode=True) == 0 + + def subvolume_create(name, dest=None, qgroupids=None): ''' Create subvolume `name` in `dest`. diff --git a/tests/unit/modules/test_btrfs.py b/tests/unit/modules/test_btrfs.py index ef31e68554a3..a56c3d8a198b 100644 --- a/tests/unit/modules/test_btrfs.py +++ b/tests/unit/modules/test_btrfs.py @@ -365,6 +365,26 @@ def test_properties_error(self): self.assertRaises(CommandExecutionError, btrfs.properties, '/dev/sda1', 'subvol', True) + def test_subvolume_exists(self): + ''' + Test subvolume_exists + ''' + salt_mock = { + 'cmd.retcode': MagicMock(return_value=0), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs.subvolume_exists('/mnt/one') + + def test_subvolume_not_exists(self): + ''' + Test subvolume_exists + ''' + salt_mock = { + 'cmd.retcode': MagicMock(return_value=1), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert not btrfs.subvolume_exists('/mnt/nowhere') + def test_subvolume_create_fails_parameters(self): ''' Test btrfs subvolume create From 107218abe44f7a907fff923aec8548cfd39307e9 Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Tue, 20 Nov 2018 13:50:44 +0100 Subject: [PATCH 3/4] btrfs: create and delete return False if noop If the create and delete actions are not needed, the function will return False. This indicate that the submodule was already there, or was missing on the first place. (cherry picked from commit 5feae48b4a727ad5d16779ed3d4e528698955cdc) --- salt/modules/btrfs.py | 29 ++++++++++++++++------ tests/unit/modules/test_btrfs.py | 41 ++++++++++++++++++++++++++++++-- 2 files changed, 61 insertions(+), 9 deletions(-) diff --git a/salt/modules/btrfs.py b/salt/modules/btrfs.py index eb6e022e8999..6fd2ac58a4cb 100644 --- a/salt/modules/btrfs.py +++ b/salt/modules/btrfs.py @@ -697,6 +697,9 @@ def subvolume_create(name, dest=None, qgroupids=None): ''' Create subvolume `name` in `dest`. + Return True if the subvolume is created, False is the subvolume is + already there. + name Name of the new subvolume @@ -720,13 +723,17 @@ def subvolume_create(name, dest=None, qgroupids=None): if qgroupids and type(qgroupids) is not list: raise CommandExecutionError('Qgroupids parameter must be a list') - cmd = ['btrfs', 'subvolume', 'create'] + if dest: + name = os.path.join(dest, name) + # If the subvolume is there, we are done + if subvolume_exists(name): + return False + + cmd = ['btrfs', 'subvolume', 'create'] if type(qgroupids) is list: cmd.append('-i') cmd.extend(qgroupids) - if dest: - name = os.path.join(dest, name) cmd.append(name) res = __salt__['cmd.run_all'](cmd) @@ -745,6 +752,9 @@ def subvolume_delete(name=None, names=None, commit=None): Please, refer to the documentation to understand the implication on the transactions, and when the subvolume is really deleted. + Return True if the subvolume is deleted, False is the subvolume + was already missing. + name Name of the subvolume to remove @@ -769,15 +779,20 @@ def subvolume_delete(name=None, names=None, commit=None): if commit and commit not in ('after', 'each'): raise CommandExecutionError('Value for commit not recognized') + # Filter the names and take the ones that are still there + names = [n for n in itertools.chain([name], names or []) + if n and subvolume_exists(n)] + + # If the subvolumes are gone, we are done + if not names: + return False + cmd = ['btrfs', 'subvolume', 'delete'] if commit == 'after': cmd.append('--commit-after') elif commit == 'each': cmd.append('--commit-each') - if name: - cmd.append(name) - if type(names) is list: - cmd.extend(names) + cmd.extend(names) res = __salt__['cmd.run_all'](cmd) salt.utils.fsutils._verify_run(res) diff --git a/tests/unit/modules/test_btrfs.py b/tests/unit/modules/test_btrfs.py index a56c3d8a198b..b5f934034d41 100644 --- a/tests/unit/modules/test_btrfs.py +++ b/tests/unit/modules/test_btrfs.py @@ -393,15 +393,26 @@ def test_subvolume_create_fails_parameters(self): with pytest.raises(CommandExecutionError): btrfs.subvolume_create('var', qgroupids='1') - def test_subvolume_create(self): + @patch('salt.modules.btrfs.subvolume_exists') + def test_subvolume_create_already_exists(self, subvolume_exists): ''' Test btrfs subvolume create ''' + subvolume_exists.return_value = True + assert not btrfs.subvolume_create('var', dest='/mnt') + + @patch('salt.modules.btrfs.subvolume_exists') + def test_subvolume_create(self, subvolume_exists): + ''' + Test btrfs subvolume create + ''' + subvolume_exists.return_value = False salt_mock = { 'cmd.run_all': MagicMock(return_value={'recode': 0}), } with patch.dict(btrfs.__salt__, salt_mock): assert btrfs.subvolume_create('var', dest='/mnt') + subvolume_exists.assert_called_once() salt_mock['cmd.run_all'].assert_called_once() salt_mock['cmd.run_all'].assert_called_with( ['btrfs', 'subvolume', 'create', '/mnt/var']) @@ -425,10 +436,36 @@ def test_subvolume_delete_fails_parameter_commit(self): with pytest.raises(CommandExecutionError): btrfs.subvolume_delete(name='var', commit='maybe') - def test_subvolume_delete(self): + @patch('salt.modules.btrfs.subvolume_exists') + def test_subvolume_delete_already_missing(self, subvolume_exists): + ''' + Test btrfs subvolume delete + ''' + subvolume_exists.return_value = False + assert not btrfs.subvolume_delete(name='var', names=['tmp']) + + @patch('salt.modules.btrfs.subvolume_exists') + def test_subvolume_delete_already_missing_name(self, subvolume_exists): + ''' + Test btrfs subvolume delete + ''' + subvolume_exists.return_value = False + assert not btrfs.subvolume_delete(name='var') + + @patch('salt.modules.btrfs.subvolume_exists') + def test_subvolume_delete_already_missing_names(self, subvolume_exists): + ''' + Test btrfs subvolume delete + ''' + subvolume_exists.return_value = False + assert not btrfs.subvolume_delete(names=['tmp']) + + @patch('salt.modules.btrfs.subvolume_exists') + def test_subvolume_delete(self, subvolume_exists): ''' Test btrfs subvolume delete ''' + subvolume_exists.return_value = True salt_mock = { 'cmd.run_all': MagicMock(return_value={'recode': 0}), } From 0a2efa8c9391d2f70a4fe7cc834de76aa85bd48e Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Tue, 3 Dec 2019 17:05:12 +0100 Subject: [PATCH 4/4] test_btrfs: skip test_subvolume_create on Windows --- tests/unit/modules/test_btrfs.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit/modules/test_btrfs.py b/tests/unit/modules/test_btrfs.py index b5f934034d41..5248cc564f45 100644 --- a/tests/unit/modules/test_btrfs.py +++ b/tests/unit/modules/test_btrfs.py @@ -21,6 +21,7 @@ # Import Salt Libs import salt.utils.files import salt.utils.fsutils +import salt.utils.platform import salt.modules.btrfs as btrfs from salt.exceptions import CommandExecutionError @@ -401,6 +402,7 @@ def test_subvolume_create_already_exists(self, subvolume_exists): subvolume_exists.return_value = True assert not btrfs.subvolume_create('var', dest='/mnt') + @skipIf(salt.utils.platform.is_windows(), 'Skip on Windows') @patch('salt.modules.btrfs.subvolume_exists') def test_subvolume_create(self, subvolume_exists): '''