From f8f9ad3d84d71ac0e0d72407d05c9d781aef1fbd Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Mon, 26 Nov 2018 14:33:54 +0100 Subject: [PATCH 1/2] btrfs: add new btrfs state (cherry picked from commit 237d40d4be291e6b6c49c0266d6629ae95cd2304) --- salt/states/btrfs.py | 250 +++++++++++++++++ tests/unit/states/test_btrfs.py | 476 ++++++++++++++++++++++++++++++++ 2 files changed, 726 insertions(+) create mode 100644 salt/states/btrfs.py create mode 100644 tests/unit/states/test_btrfs.py diff --git a/salt/states/btrfs.py b/salt/states/btrfs.py new file mode 100644 index 000000000000..36d9607890bb --- /dev/null +++ b/salt/states/btrfs.py @@ -0,0 +1,250 @@ +# -*- coding: utf-8 -*- +# +# Author: Alberto Planas +# +# Copyright 2018 SUSE LINUX GmbH, Nuernberg, Germany. +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +''' +:maintainer: Alberto Planas +:maturity: new +:depends: None +:platform: Linux +''' +from __future__ import absolute_import, print_function, unicode_literals +import functools +import logging +import os.path +import tempfile +import traceback + +from salt.exceptions import CommandExecutionError + +log = logging.getLogger(__name__) + +__virtualname__ = 'btrfs' + + +def _mount(device): + ''' + Mount the device in a temporary place. + ''' + dest = tempfile.mkdtemp() + res = __states__['mount.mounted'](dest, device=device, fstype='btrfs', + opts='subvol=/', persist=False) + if not res['result']: + log.error('Cannot mount device %s in %s', device, dest) + _umount(dest) + return None + return dest + + +def _umount(path): + ''' + Umount and clean the temporary place. + ''' + __states__['mount.unmounted'](path) + __utils__['files.rm_rf'](path) + + +def _is_default(path, dest, name): + ''' + Check if the subvolume is the current default. + ''' + subvol_id = __salt__['btrfs.subvolume_show'](path)[name]['subvolume id'] + def_id = __salt__['btrfs.subvolume_get_default'](dest)['id'] + return subvol_id == def_id + + +def _set_default(path, dest, name): + ''' + Set the subvolume as the current default. + ''' + subvol_id = __salt__['btrfs.subvolume_show'](path)[name]['subvolume id'] + return __salt__['btrfs.subvolume_set_default'](subvol_id, dest) + + +def _is_cow(path): + ''' + Check if the subvolume is copy on write + ''' + dirname = os.path.dirname(path) + return 'C' not in __salt__['file.lsattr'](dirname)[path] + + +def _unset_cow(path): + ''' + Disable the copy on write in a subvolume + ''' + return __salt__['file.chattr'](path, operator='add', attributes='C') + + +def __mount_device(action): + ''' + Small decorator to makes sure that the mount and umount happends in + a transactional way. + ''' + @functools.wraps(action) + def wrapper(*args, **kwargs): + name = kwargs['name'] + device = kwargs['device'] + + ret = { + 'name': name, + 'result': False, + 'changes': {}, + 'comment': ['Some error happends during the operation.'], + } + try: + dest = _mount(device) + if not dest: + msg = 'Device {} cannot be mounted'.format(device) + ret['comment'].append(msg) + kwargs['__dest'] = dest + ret = action(*args, **kwargs) + except Exception as e: + log.error('''Traceback: {}'''.format(traceback.format_exc())) + ret['comment'].append(e) + finally: + _umount(dest) + return ret + return wrapper + + +@__mount_device +def subvolume_created(name, device, qgroupids=None, set_default=False, + copy_on_write=True, __dest=None): + ''' + Makes sure that a btrfs subvolume is present. + + name + Name of the subvolume to add + + device + Device where to create the subvolume + + qgroupids + Add the newly created subcolume to a qgroup. This parameter + is a list + + set_default + If True, this new subvolume will be set as default when + mounted, unless subvol option in mount is used + + copy_on_write + If false, set the subvolume with chattr +C + + ''' + ret = { + 'name': name, + 'result': False, + 'changes': {}, + 'comment': [], + } + path = os.path.join(__dest, name) + + exists = __salt__['btrfs.subvolume_exists'](path) + if exists: + ret['comment'].append('Subvolume {} already present'.format(name)) + + # Resolve first the test case. The check is not complete, but at + # least we will report if a subvolume needs to be created. Can + # happend that the subvolume is there, but we also need to set it + # as default, or persist in fstab. + if __opts__['test']: + ret['result'] = None + if not exists: + ret['comment'].append('Subvolume {} will be created'.format(name)) + return ret + + if not exists: + # Create the directories where the subvolume lives + _path = os.path.dirname(path) + res = __states__['file.directory'](_path, makedirs=True) + if not res['result']: + ret['comment'].append('Error creating {} directory'.format(_path)) + return ret + + try: + __salt__['btrfs.subvolume_create'](name, dest=__dest, + qgroupids=qgroupids) + except CommandExecutionError: + ret['comment'].append('Error creating subvolume {}'.format(name)) + return ret + + ret['changes'][name] = 'Created subvolume {}'.format(name) + + if set_default and not _is_default(path, __dest, name): + ret['changes'][name + '_default'] = _set_default(path, __dest, name) + + if not copy_on_write and _is_cow(path): + ret['changes'][name + '_no_cow'] = _unset_cow(path) + + ret['result'] = True + return ret + + +@__mount_device +def subvolume_deleted(name, device, commit=False, __dest=None): + ''' + Makes sure that a btrfs subvolume is removed. + + name + Name of the subvolume to remove + + device + Device where to remove the subvolume + + commit + Wait until the transaction is over + + ''' + ret = { + 'name': name, + 'result': False, + 'changes': {}, + 'comment': [], + } + + path = os.path.join(__dest, name) + + exists = __salt__['btrfs.subvolume_exists'](path) + if not exists: + ret['comment'].append('Subvolume {} already missing'.format(name)) + + if __opts__['test']: + ret['result'] = None + if exists: + ret['comment'].append('Subvolume {} will be removed'.format(name)) + return ret + + # If commit is set, we wait until all is over + commit = 'after' if commit else None + + if not exists: + try: + __salt__['btrfs.subvolume_delete'](path, commit=commit) + except CommandExecutionError: + ret['comment'].append('Error removing subvolume {}'.format(name)) + return ret + + ret['changes'][name] = 'Removed subvolume {}'.format(name) + + ret['result'] = True + return ret diff --git a/tests/unit/states/test_btrfs.py b/tests/unit/states/test_btrfs.py new file mode 100644 index 000000000000..b07080ff6b49 --- /dev/null +++ b/tests/unit/states/test_btrfs.py @@ -0,0 +1,476 @@ +# -*- coding: utf-8 -*- +# +# Author: Alberto Planas +# +# Copyright 2018 SUSE LINUX GmbH, Nuernberg, Germany. +# +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +''' +:maintainer: Alberto Planas +:platform: Linux +''' +# Import Python Libs +from __future__ import absolute_import, print_function, unicode_literals +# Import Salt Testing Libs +from tests.support.mixins import LoaderModuleMockMixin +from tests.support.unit import skipIf, TestCase +from tests.support.mock import ( + MagicMock, + NO_MOCK, + NO_MOCK_REASON, + patch, +) + +from salt.exceptions import CommandExecutionError +import salt.states.btrfs as btrfs + + +@skipIf(NO_MOCK, NO_MOCK_REASON) +class BtrfsTestCase(TestCase, LoaderModuleMockMixin): + ''' + Test cases for salt.states.btrfs + ''' + + def setup_loader_modules(self): + return { + btrfs: { + '__salt__': {}, + '__states__': {}, + '__utils__': {}, + } + } + + @patch('salt.states.btrfs._umount') + @patch('tempfile.mkdtemp') + def test__mount_fails(self, mkdtemp, umount): + ''' + Test mounting a device in a temporary place. + ''' + mkdtemp.return_value = '/tmp/xxx' + states_mock = { + 'mount.mounted': MagicMock(return_value={'result': False}), + } + with patch.dict(btrfs.__states__, states_mock): + assert btrfs._mount('/dev/sda1') is None + mkdtemp.assert_called_once() + states_mock['mount.mounted'].assert_called_with('/tmp/xxx', + device='/dev/sda1', + fstype='btrfs', + opts='subvol=/', + persist=False) + umount.assert_called_with('/tmp/xxx') + + @patch('salt.states.btrfs._umount') + @patch('tempfile.mkdtemp') + def test__mount(self, mkdtemp, umount): + ''' + Test mounting a device in a temporary place. + ''' + mkdtemp.return_value = '/tmp/xxx' + states_mock = { + 'mount.mounted': MagicMock(return_value={'result': True}), + } + with patch.dict(btrfs.__states__, states_mock): + assert btrfs._mount('/dev/sda1') == '/tmp/xxx' + mkdtemp.assert_called_once() + states_mock['mount.mounted'].assert_called_with('/tmp/xxx', + device='/dev/sda1', + fstype='btrfs', + opts='subvol=/', + persist=False) + umount.assert_not_called() + + def test__umount(self): + ''' + Test umounting and cleanning temporary place. + ''' + states_mock = { + 'mount.unmounted': MagicMock(), + } + utils_mock = { + 'files.rm_rf': MagicMock(), + } + with patch.dict(btrfs.__states__, states_mock), \ + patch.dict(btrfs.__utils__, utils_mock): + btrfs._umount('/tmp/xxx') + states_mock['mount.unmounted'].assert_called_with('/tmp/xxx') + utils_mock['files.rm_rf'].assert_called_with('/tmp/xxx') + + def test__is_default_not_default(self): + ''' + Test if the subvolume is the current default. + ''' + salt_mock = { + 'btrfs.subvolume_show': MagicMock(return_value={ + '@/var': {'subvolume id': '256'}, + }), + 'btrfs.subvolume_get_default': MagicMock(return_value={ + 'id': '5', + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert not btrfs._is_default('/tmp/xxx/@/var', '/tmp/xxx', '@/var') + salt_mock['btrfs.subvolume_show'].assert_called_with('/tmp/xxx/@/var') + salt_mock['btrfs.subvolume_get_default'].assert_called_with('/tmp/xxx') + + def test__is_default(self): + ''' + Test if the subvolume is the current default. + ''' + salt_mock = { + 'btrfs.subvolume_show': MagicMock(return_value={ + '@/var': {'subvolume id': '256'}, + }), + 'btrfs.subvolume_get_default': MagicMock(return_value={ + 'id': '256', + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs._is_default('/tmp/xxx/@/var', '/tmp/xxx', '@/var') + salt_mock['btrfs.subvolume_show'].assert_called_with('/tmp/xxx/@/var') + salt_mock['btrfs.subvolume_get_default'].assert_called_with('/tmp/xxx') + + def test__set_default(self): + ''' + Test setting a subvolume as the current default. + ''' + salt_mock = { + 'btrfs.subvolume_show': MagicMock(return_value={ + '@/var': {'subvolume id': '256'}, + }), + 'btrfs.subvolume_set_default': MagicMock(return_value=True), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs._set_default('/tmp/xxx/@/var', '/tmp/xxx', '@/var') + salt_mock['btrfs.subvolume_show'].assert_called_with('/tmp/xxx/@/var') + salt_mock['btrfs.subvolume_set_default'].assert_called_with('256', '/tmp/xxx') + + def test__is_cow_not_cow(self): + ''' + Test if the subvolume is copy on write. + ''' + salt_mock = { + 'file.lsattr': MagicMock(return_value={ + '/tmp/xxx/@/var': ['C'], + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert not btrfs._is_cow('/tmp/xxx/@/var') + salt_mock['file.lsattr'].assert_called_with('/tmp/xxx/@') + + def test__is_cow(self): + ''' + Test if the subvolume is copy on write. + ''' + salt_mock = { + 'file.lsattr': MagicMock(return_value={ + '/tmp/xxx/@/var': [], + }), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs._is_cow('/tmp/xxx/@/var') + salt_mock['file.lsattr'].assert_called_with('/tmp/xxx/@') + + def test__unset_cow(self): + ''' + Test disabling the subvolume as copy on write. + ''' + salt_mock = { + 'file.chattr': MagicMock(return_value=True), + } + with patch.dict(btrfs.__salt__, salt_mock): + assert btrfs._unset_cow('/tmp/xxx/@/var') + salt_mock['file.chattr'].assert_called_with('/tmp/xxx/@/var', + operator='add', + attributes='C') + + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_exists(self, mount, umount): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=True), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1') == { + 'name': '@/var', + 'result': True, + 'changes': {}, + 'comment': ['Subvolume @/var already present'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + mount.assert_called_once() + umount.assert_called_once() + + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_exists_test(self, mount, umount): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=True), + } + opts_mock = { + 'test': True, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1') == { + 'name': '@/var', + 'result': None, + 'changes': {}, + 'comment': ['Subvolume @/var already present'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + mount.assert_called_once() + umount.assert_called_once() + + @patch('salt.states.btrfs._is_default') + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_exists_was_default(self, mount, umount, + is_default): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + is_default.return_value = True + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=True), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1', + set_default=True) == { + 'name': '@/var', + 'result': True, + 'changes': {}, + 'comment': ['Subvolume @/var already present'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + mount.assert_called_once() + umount.assert_called_once() + + @patch('salt.states.btrfs._set_default') + @patch('salt.states.btrfs._is_default') + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_exists_set_default(self, mount, umount, + is_default, set_default): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + is_default.return_value = False + set_default.return_value = True + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=True), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1', + set_default=True) == { + 'name': '@/var', + 'result': True, + 'changes': { + '@/var_default': True + }, + 'comment': ['Subvolume @/var already present'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + mount.assert_called_once() + umount.assert_called_once() + + @patch('salt.states.btrfs._is_cow') + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_exists_no_cow(self, mount, umount, is_cow): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + is_cow.return_value = False + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=True), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1', + copy_on_write=False) == { + 'name': '@/var', + 'result': True, + 'changes': {}, + 'comment': ['Subvolume @/var already present'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + mount.assert_called_once() + umount.assert_called_once() + + @patch('salt.states.btrfs._unset_cow') + @patch('salt.states.btrfs._is_cow') + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_exists_unset_cow(self, mount, umount, + is_cow, unset_cow): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + is_cow.return_value = True + unset_cow.return_value = True + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=True), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1', + copy_on_write=False) == { + 'name': '@/var', + 'result': True, + 'changes': { + '@/var_no_cow': True + }, + 'comment': ['Subvolume @/var already present'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + mount.assert_called_once() + umount.assert_called_once() + + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created(self, mount, umount): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=False), + 'btrfs.subvolume_create': MagicMock(), + } + states_mock = { + 'file.directory': MagicMock(return_value={'result': True}), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__states__, states_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1') == { + 'name': '@/var', + 'result': True, + 'changes': { + '@/var': 'Created subvolume @/var' + }, + 'comment': [], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + salt_mock['btrfs.subvolume_create'].assert_called_once() + mount.assert_called_once() + umount.assert_called_once() + + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_fails_directory(self, mount, umount): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=False), + } + states_mock = { + 'file.directory': MagicMock(return_value={'result': False}), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__states__, states_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1') == { + 'name': '@/var', + 'result': False, + 'changes': {}, + 'comment': ['Error creating /tmp/xxx/@ directory'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + mount.assert_called_once() + umount.assert_called_once() + + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_fails(self, mount, umount): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=False), + 'btrfs.subvolume_create': MagicMock(side_effect=CommandExecutionError), + } + states_mock = { + 'file.directory': MagicMock(return_value={'result': True}), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__states__, states_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1') == { + 'name': '@/var', + 'result': False, + 'changes': {}, + 'comment': ['Error creating subvolume @/var'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + salt_mock['btrfs.subvolume_create'].assert_called_once() + mount.assert_called_once() + umount.assert_called_once() From 4b2f93c2978e939e87dcc38bbb114015704ce717 Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Mon, 10 Dec 2018 10:22:19 +0100 Subject: [PATCH 2/2] btrfs: add option to not set subvolumes as default When we want to create a subvolume and set is as default, we can use the set_default parameter. The current code will check, for already created subvolumes, if it is actually marked as default, and if not it will be set inside the state. For composability with other states, we sometimes want to avoid the re-set of the default flag for already present subvolumes, as can be that this was change in a later action by another state. (cherry picked from commit 067482e8a2205257bf00423991ad84f9faa49a67) --- salt/states/btrfs.py | 12 +++++++++-- tests/unit/states/test_btrfs.py | 36 +++++++++++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/salt/states/btrfs.py b/salt/states/btrfs.py index 36d9607890bb..b2b939d53547 100644 --- a/salt/states/btrfs.py +++ b/salt/states/btrfs.py @@ -129,7 +129,8 @@ def wrapper(*args, **kwargs): @__mount_device def subvolume_created(name, device, qgroupids=None, set_default=False, - copy_on_write=True, __dest=None): + copy_on_write=True, force_set_default=True, + __dest=None): ''' Makes sure that a btrfs subvolume is present. @@ -150,6 +151,10 @@ def subvolume_created(name, device, qgroupids=None, set_default=False, copy_on_write If false, set the subvolume with chattr +C + force_set_default + If false and the subvolume is already present, it will not + force it as default if ``set_default`` is True + ''' ret = { 'name': name, @@ -190,7 +195,10 @@ def subvolume_created(name, device, qgroupids=None, set_default=False, ret['changes'][name] = 'Created subvolume {}'.format(name) - if set_default and not _is_default(path, __dest, name): + # If the volume was already present, we can opt-out the check for + # default subvolume. + if (not exists or (exists and force_set_default)) and \ + set_default and not _is_default(path, __dest, name): ret['changes'][name + '_default'] = _set_default(path, __dest, name) if not copy_on_write and _is_cow(path): diff --git a/tests/unit/states/test_btrfs.py b/tests/unit/states/test_btrfs.py index b07080ff6b49..f7edd1f92ba3 100644 --- a/tests/unit/states/test_btrfs.py +++ b/tests/unit/states/test_btrfs.py @@ -316,6 +316,42 @@ def test_subvolume_created_exists_set_default(self, mount, umount, mount.assert_called_once() umount.assert_called_once() + @patch('salt.states.btrfs._set_default') + @patch('salt.states.btrfs._is_default') + @patch('salt.states.btrfs._umount') + @patch('salt.states.btrfs._mount') + def test_subvolume_created_exists_set_default_no_force(self, + mount, + umount, + is_default, + set_default): + ''' + Test creating a subvolume. + ''' + mount.return_value = '/tmp/xxx' + is_default.return_value = False + set_default.return_value = True + salt_mock = { + 'btrfs.subvolume_exists': MagicMock(return_value=True), + } + opts_mock = { + 'test': False, + } + with patch.dict(btrfs.__salt__, salt_mock), \ + patch.dict(btrfs.__opts__, opts_mock): + assert btrfs.subvolume_created(name='@/var', + device='/dev/sda1', + set_default=True, + force_set_default=False) == { + 'name': '@/var', + 'result': True, + 'changes': {}, + 'comment': ['Subvolume @/var already present'], + } + salt_mock['btrfs.subvolume_exists'].assert_called_with('/tmp/xxx/@/var') + mount.assert_called_once() + umount.assert_called_once() + @patch('salt.states.btrfs._is_cow') @patch('salt.states.btrfs._umount') @patch('salt.states.btrfs._mount')