From 77881bb15aaa47aee7e2d5a1a5bd9fd56fdc425e Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Mon, 3 Dec 2018 17:56:55 +0100 Subject: [PATCH 1/2] mount: add fstab_{present,absent} states (cherry picked from commit 12ae2db12264f8632ccdd1231450699648f4042a) --- salt/states/mount.py | 292 ++++++++++++++++ tests/unit/states/test_mount.py | 593 ++++++++++++++++++++++++++++++++ 2 files changed, 885 insertions(+) diff --git a/salt/states/mount.py b/salt/states/mount.py index 162da1ca621e..2f5946b647ad 100644 --- a/salt/states/mount.py +++ b/salt/states/mount.py @@ -956,3 +956,295 @@ def mod_watch(name, user=None, **kwargs): else: ret['comment'] = 'Watch not supported in {0} at this time'.format(kwargs['sfun']) return ret + + +def _convert_to(maybe_device, convert_to): + ''' + Convert a device name, UUID or LABEL to a device name, UUID or + LABEL. + + Return the fs_spec required for fstab. + + ''' + + # Fast path. If we already have the information required, we can + # save one blkid call + if not convert_to or \ + (convert_to == 'device' and maybe_device.startswith('/')) or \ + maybe_device.startswith('{}='.format(convert_to.upper())): + return maybe_device + + # Get the device information + if maybe_device.startswith('/'): + blkid = __salt__['disk.blkid'](maybe_device) + else: + blkid = __salt__['disk.blkid'](token=maybe_device) + + result = None + if len(blkid) == 1: + if convert_to == 'device': + result = list(blkid.keys())[0] + else: + key = convert_to.upper() + result = '{}={}'.format(key, list(blkid.values())[0][key]) + + return result + + +def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', + fs_freq=0, fs_passno=0, mount_by=None, + config='/etc/fstab', mount=True, match_on='auto'): + ''' + Makes sure that a fstab mount point is pressent. + + name + The name of block device. Can be any valid fs_spec value. + + fs_file + Mount point (target) for the filesystem. + + fs_vfstype + The type of the filesystem (e.g. ext4, xfs, btrfs, ...) + + fs_mntops + The mount options associated with the filesystem. Default is + ``defaults``. + + fs_freq + Field is used by dump to determine which fs need to be + dumped. Default is ``0`` + + fs_passno + Field is used by fsck to determine the order in which + filesystem checks are done at boot time. Default is ``0`` + + mount_by + Select the final value for fs_spec. Can be [``None``, + ``device``, ``label``, ``uuid``, ``partlabel``, + ``partuuid``]. If ``None``, the value for fs_spect will be the + parameter ``name``, in other case will search the correct + value based on the device name. For example, for ``uuid``, the + value for fs_spec will be of type 'UUID=xxx' instead of the + device name set in ``name``. + + config + Place where the fstab file lives. Default is ``/etc/fstab`` + + mount + Set if the mount should be mounted immediately. Default is + ``True`` + + match_on + A name or list of fstab properties on which this state should + be applied. Default is ``auto``, a special value indicating + to guess based on fstype. In general, ``auto`` matches on + name for recognized special devices and device otherwise. + + ''' + ret = { + 'name': name, + 'result': False, + 'changes': {}, + 'comment': [], + } + + # Adjust fs_mntops based on the OS + if fs_mntops == 'defaults': + if __grains__['os'] in ['MacOS', 'Darwin']: + fs_mntops = 'noowners' + elif __grains__['os'] == 'AIX': + fs_mntops = '' + + # Adjust the config file based on the OS + if config == '/etc/fstab': + if __grains__['os'] in ['MacOS', 'Darwin']: + config = '/etc/auto_salt' + elif __grains__['os'] == 'AIX': + config = '/etc/filesystems' + + if not fs_file == '/': + fs_file = fs_file.rstrip('/') + + fs_spec = _convert_to(name, mount_by) + + # Validate that the device is valid after the conversion + if not fs_spec: + msg = 'Device {} cannot be converted to {}' + ret['comment'].append(msg.format(name, mount_by)) + return ret + + if __opts__['test']: + if __grains__['os'] in ['MacOS', 'Darwin']: + out = __salt__['mount.set_automaster'](name=fs_file, + device=fs_spec, + fstype=fs_vfstype, + opts=fs_mntops, + config=config, + test=True) + elif __grains__['os'] == 'AIX': + out = __salt__['mount.set_filesystems'](name=fs_file, + device=fs_spec, + fstype=fs_vfstype, + opts=fs_mntops, + mount=mount, + config=config, + test=True, + match_on=match_on) + else: + out = __salt__['mount.set_fstab'](name=fs_file, + device=fs_spec, + fstype=fs_vfstype, + opts=fs_mntops, + dump=fs_freq, + pass_num=fs_passno, + config=config, + test=True, + match_on=match_on) + ret['result'] = None + if out == 'present': + msg = '{} entry is already in {}.' + ret['comment'].append(msg.format(fs_file, config)) + elif out == 'new': + msg = '{} entry will be written in {}.' + ret['comment'].append(msg.format(fs_file, config)) + elif out == 'change': + msg = '{} entry will be updated in {}.' + ret['comment'].append(msg.format(fs_file, config)) + else: + ret['result'] = False + msg = '{} entry cannot be created in {}: {}.' + ret['comment'].append(msg.format(fs_file, config, out)) + return ret + + if __grains__['os'] in ['MacOS', 'Darwin']: + out = __salt__['mount.set_automaster'](name=fs_file, + device=fs_spec, + fstype=fs_vfstype, + opts=fs_mntops, + config=config) + elif __grains__['os'] == 'AIX': + out = __salt__['mount.set_filesystems'](name=fs_file, + device=fs_spec, + fstype=fs_vfstype, + opts=fs_mntops, + mount=mount, + config=config, + match_on=match_on) + else: + out = __salt__['mount.set_fstab'](name=fs_file, + device=fs_spec, + fstype=fs_vfstype, + opts=fs_mntops, + dump=fs_freq, + pass_num=fs_passno, + config=config, + match_on=match_on) + + ret['result'] = True + if out == 'present': + msg = '{} entry was already in {}.' + ret['comment'].append(msg.format(fs_file, config)) + elif out == 'new': + ret['changes']['persist'] = out + msg = '{} entry added in {}.' + ret['comment'].append(msg.format(fs_file, config)) + elif out == 'change': + ret['changes']['persist'] = out + msg = '{} entry updated in {}.' + ret['comment'].append(msg.format(fs_file, config)) + else: + ret['result'] = False + msg = '{} entry cannot be changed in {}: {}.' + ret['comment'].append(msg.format(fs_file, config, out)) + + return ret + + +def fstab_absent(name, fs_file, mount_by=None, config='/etc/fstab'): + ''' + Makes sure that a fstab mount point is absent. + + name + The name of block device. Can be any valid fs_spec value. + + fs_file + Mount point (target) for the filesystem. + + mount_by + Select the final value for fs_spec. Can be [``None``, + ``device``, ``label``, ``uuid``, ``partlabel``, + ``partuuid``]. If ``None``, the value for fs_spect will be the + parameter ``name``, in other case will search the correct + value based on the device name. For example, for ``uuid``, the + value for fs_spec will be of type 'UUID=xxx' instead of the + device name set in ``name``. + + config + Place where the fstab file lives + + ''' + ret = { + 'name': name, + 'result': False, + 'changes': {}, + 'comment': [], + } + + # Adjust the config file based on the OS + if config == '/etc/fstab': + if __grains__['os'] in ['MacOS', 'Darwin']: + config = '/etc/auto_salt' + elif __grains__['os'] == 'AIX': + config = '/etc/filesystems' + + if not fs_file == '/': + fs_file = fs_file.rstrip('/') + + fs_spec = _convert_to(name, mount_by) + + if __grains__['os'] in ['MacOS', 'Darwin']: + fstab_data = __salt__['mount.automaster'](config) + elif __grains__['os'] == 'AIX': + fstab_data = __salt__['mount.filesystems'](config) + else: + fstab_data = __salt__['mount.fstab'](config) + + if __opts__['test']: + ret['result'] = None + if fs_file not in fstab_data: + msg = '{} entry is already missing in {}.' + ret['comment'].append(msg.format(fs_file, config)) + else: + msg = '{} entry will be removed from {}.' + ret['comment'].append(msg.format(fs_file, config)) + return ret + + if fs_file in fstab_data: + if __grains__['os'] in ['MacOS', 'Darwin']: + out = __salt__['mount.rm_automaster'](name=fs_file, + device=fs_spec, + config=config) + elif __grains__['os'] == 'AIX': + out = __salt__['mount.rm_filesystems'](name=fs_file, + device=fs_spec, + config=config) + else: + out = __salt__['mount.rm_fstab'](name=fs_file, + device=fs_spec, + config=config) + + if out is not True: + ret['result'] = False + msg = '{} entry failed when removing from {}.' + ret['comment'].append(msg.format(fs_file, config)) + else: + ret['result'] = True + ret['changes']['persist'] = 'removed' + msg = '{} entry removed from {}.' + ret['comment'].append(msg.format(fs_file, config)) + else: + ret['result'] = True + msg = '{} entry is already missing in {}.' + ret['comment'].append(msg.format(fs_file, config)) + + return ret diff --git a/tests/unit/states/test_mount.py b/tests/unit/states/test_mount.py index 3e3a75d3cd8b..2b35626b8263 100644 --- a/tests/unit/states/test_mount.py +++ b/tests/unit/states/test_mount.py @@ -449,3 +449,596 @@ def test_mod_watch(self): 'changes': {}} self.assertDictEqual(mount.mod_watch(name, sfun='unmount'), ret) + + def test__convert_to_fast_none(self): + ''' + Test the device name conversor + ''' + assert mount._convert_to('/dev/sda1', None) == '/dev/sda1' + + def test__convert_to_fast_device(self): + ''' + Test the device name conversor + ''' + assert mount._convert_to('/dev/sda1', 'device') == '/dev/sda1' + + def test__convert_to_fast_token(self): + ''' + Test the device name conversor + ''' + assert mount._convert_to('LABEL=home', 'label') == 'LABEL=home' + + def test__convert_to_device_none(self): + ''' + Test the device name conversor + ''' + salt_mock = { + 'disk.blkid': MagicMock(return_value={}), + } + with patch.dict(mount.__salt__, salt_mock): + assert mount._convert_to('/dev/sda1', 'uuid') is None + salt_mock['disk.blkid'].assert_called_with('/dev/sda1') + + def test__convert_to_device_token(self): + ''' + Test the device name conversor + ''' + uuid = '988c663d-74a2-432b-ba52-3eea34015f22' + salt_mock = { + 'disk.blkid': MagicMock(return_value={ + '/dev/sda1': {'UUID': uuid} + }), + } + with patch.dict(mount.__salt__, salt_mock): + uuid = 'UUID={}'.format(uuid) + assert mount._convert_to('/dev/sda1', 'uuid') == uuid + salt_mock['disk.blkid'].assert_called_with('/dev/sda1') + + def test__convert_to_token_device(self): + ''' + Test the device name conversor + ''' + uuid = '988c663d-74a2-432b-ba52-3eea34015f22' + salt_mock = { + 'disk.blkid': MagicMock(return_value={ + '/dev/sda1': {'UUID': uuid} + }), + } + with patch.dict(mount.__salt__, salt_mock): + uuid = 'UUID={}'.format(uuid) + assert mount._convert_to(uuid, 'device') == '/dev/sda1' + salt_mock['disk.blkid'].assert_called_with(token=uuid) + + def test_fstab_present_macos_test_present(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry is already in /etc/auto_salt.'], + } + + grains_mock = {'os': 'MacOS'} + opts_mock = {'test': True} + salt_mock = { + 'mount.set_automaster': MagicMock(return_value='present') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_automaster'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='noowners', + config='/etc/auto_salt', + test=True) + + def test_fstab_present_aix_test_present(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry is already in /etc/filesystems.'], + } + + grains_mock = {'os': 'AIX'} + opts_mock = {'test': True} + salt_mock = { + 'mount.set_filesystems': MagicMock(return_value='present') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_filesystems'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + mount=True, + opts='', + config='/etc/filesystems', + test=True, + match_on='auto') + + def test_fstab_present_test_present(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry is already in /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': True} + salt_mock = { + 'mount.set_fstab': MagicMock(return_value='present') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='defaults', + dump=0, + pass_num=0, + config='/etc/fstab', + test=True, + match_on='auto') + + def test_fstab_present_test_new(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry will be written in /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': True} + salt_mock = { + 'mount.set_fstab': MagicMock(return_value='new') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='defaults', + dump=0, + pass_num=0, + config='/etc/fstab', + test=True, + match_on='auto') + + def test_fstab_present_test_change(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry will be updated in /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': True} + salt_mock = { + 'mount.set_fstab': MagicMock(return_value='change') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='defaults', + dump=0, + pass_num=0, + config='/etc/fstab', + test=True, + match_on='auto') + + def test_fstab_present_test_error(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': False, + 'changes': {}, + 'comment': ['/home entry cannot be created in /etc/fstab: error.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': True} + salt_mock = { + 'mount.set_fstab': MagicMock(return_value='error') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='defaults', + dump=0, + pass_num=0, + config='/etc/fstab', + test=True, + match_on='auto') + + def test_fstab_present_macos_present(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {}, + 'comment': ['/home entry was already in /etc/auto_salt.'], + } + + grains_mock = {'os': 'MacOS'} + opts_mock = {'test': False} + salt_mock = { + 'mount.set_automaster': MagicMock(return_value='present') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_automaster'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='noowners', + config='/etc/auto_salt') + + def test_fstab_present_aix_present(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {}, + 'comment': ['/home entry was already in /etc/filesystems.'], + } + + grains_mock = {'os': 'AIX'} + opts_mock = {'test': False} + salt_mock = { + 'mount.set_filesystems': MagicMock(return_value='present') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_filesystems'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + mount=True, + opts='', + config='/etc/filesystems', + match_on='auto') + + def test_fstab_present_present(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {}, + 'comment': ['/home entry was already in /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': False} + salt_mock = { + 'mount.set_fstab': MagicMock(return_value='present') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='defaults', + dump=0, + pass_num=0, + config='/etc/fstab', + match_on='auto') + + def test_fstab_present_new(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {'persist': 'new'}, + 'comment': ['/home entry added in /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': False} + salt_mock = { + 'mount.set_fstab': MagicMock(return_value='new') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='defaults', + dump=0, + pass_num=0, + config='/etc/fstab', + match_on='auto') + + def test_fstab_present_change(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {'persist': 'change'}, + 'comment': ['/home entry updated in /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': False} + salt_mock = { + 'mount.set_fstab': MagicMock(return_value='change') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='defaults', + dump=0, + pass_num=0, + config='/etc/fstab', + match_on='auto') + + def test_fstab_present_fail(self): + ''' + Test fstab_present + ''' + ret = { + 'name': '/dev/sda1', + 'result': False, + 'changes': {}, + 'comment': ['/home entry cannot be changed in /etc/fstab: error.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': False} + salt_mock = { + 'mount.set_fstab': MagicMock(return_value='error') + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_present('/dev/sda1', '/home', 'ext2') == ret + salt_mock['mount.set_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + fstype='ext2', + opts='defaults', + dump=0, + pass_num=0, + config='/etc/fstab', + match_on='auto') + + def test_fstab_absent_macos_test_absent(self): + ''' + Test fstab_absent + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry is already missing in /etc/auto_salt.'], + } + + grains_mock = {'os': 'MacOS'} + opts_mock = {'test': True} + salt_mock = { + 'mount.automaster': MagicMock(return_value={}) + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_absent('/dev/sda1', '/home') == ret + salt_mock['mount.automaster'].assert_called_with('/etc/auto_salt') + + def test_fstab_absent_aix_test_absent(self): + ''' + Test fstab_absent + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry is already missing in /etc/filesystems.'], + } + + grains_mock = {'os': 'AIX'} + opts_mock = {'test': True} + salt_mock = { + 'mount.filesystems': MagicMock(return_value={}) + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_absent('/dev/sda1', '/home') == ret + salt_mock['mount.filesystems'].assert_called_with('/etc/filesystems') + + def test_fstab_absent_test_absent(self): + ''' + Test fstab_absent + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry is already missing in /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': True} + salt_mock = { + 'mount.fstab': MagicMock(return_value={}) + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_absent('/dev/sda1', '/home') == ret + salt_mock['mount.fstab'].assert_called_with('/etc/fstab') + + def test_fstab_absent_test_present(self): + ''' + Test fstab_absent + ''' + ret = { + 'name': '/dev/sda1', + 'result': None, + 'changes': {}, + 'comment': ['/home entry will be removed from /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': True} + salt_mock = { + 'mount.fstab': MagicMock(return_value={'/home': {}}) + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_absent('/dev/sda1', '/home') == ret + salt_mock['mount.fstab'].assert_called_with('/etc/fstab') + + def test_fstab_absent_macos_present(self): + ''' + Test fstab_absent + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {'persist': 'removed'}, + 'comment': ['/home entry removed from /etc/auto_salt.'], + } + + grains_mock = {'os': 'MacOS'} + opts_mock = {'test': False} + salt_mock = { + 'mount.automaster': MagicMock(return_value={'/home': {}}), + 'mount.rm_automaster': MagicMock(return_value=True) + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_absent('/dev/sda1', '/home') == ret + salt_mock['mount.automaster'].assert_called_with('/etc/auto_salt') + salt_mock['mount.rm_automaster'].assert_called_with(name='/home', + device='/dev/sda1', + config='/etc/auto_salt') + + def test_fstab_absent_aix_present(self): + ''' + Test fstab_absent + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {'persist': 'removed'}, + 'comment': ['/home entry removed from /etc/filesystems.'], + } + + grains_mock = {'os': 'AIX'} + opts_mock = {'test': False} + salt_mock = { + 'mount.filesystems': MagicMock(return_value={'/home': {}}), + 'mount.rm_filesystems': MagicMock(return_value=True) + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_absent('/dev/sda1', '/home') == ret + salt_mock['mount.filesystems'].assert_called_with('/etc/filesystems') + salt_mock['mount.rm_filesystems'].assert_called_with(name='/home', + device='/dev/sda1', + config='/etc/filesystems') + + def test_fstab_absent_present(self): + ''' + Test fstab_absent + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {'persist': 'removed'}, + 'comment': ['/home entry removed from /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': False} + salt_mock = { + 'mount.fstab': MagicMock(return_value={'/home': {}}), + 'mount.rm_fstab': MagicMock(return_value=True) + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_absent('/dev/sda1', '/home') == ret + salt_mock['mount.fstab'].assert_called_with('/etc/fstab') + salt_mock['mount.rm_fstab'].assert_called_with(name='/home', + device='/dev/sda1', + config='/etc/fstab') + + def test_fstab_absent_absent(self): + ''' + Test fstab_absent + ''' + ret = { + 'name': '/dev/sda1', + 'result': True, + 'changes': {}, + 'comment': ['/home entry is already missing in /etc/fstab.'], + } + + grains_mock = {'os': 'Linux'} + opts_mock = {'test': False} + salt_mock = { + 'mount.fstab': MagicMock(return_value={}) + } + with patch.dict(mount.__grains__, grains_mock), \ + patch.dict(mount.__opts__, opts_mock), \ + patch.dict(mount.__salt__, salt_mock): + assert mount.fstab_absent('/dev/sda1', '/home') == ret + salt_mock['mount.fstab'].assert_called_with('/etc/fstab') From 07be0baed918f5aa9e0c2da93e61c53a1001b3c5 Mon Sep 17 00:00:00 2001 From: Alberto Planas Date: Fri, 10 May 2019 13:58:52 +0200 Subject: [PATCH 2/2] mount: add not_change parameter to set_fstab and similars The current 'set_' family search the entry based on a 'match_on' parameter. If the line is found check that there is no difference between the expectations and the present line. If there are changes this line gets replaced. This default behaviour is the correct one in cases where the Salt code owns all the changes made on entries. But in some scenarios we do not want to overwrite the line if is already present, even if there are diferences in other parameters, like the option field. This patch add a new parameter, 'not_change', that if is True will not overwrite the present line in any case. Also update the `mount` state to transfer this new parameter. (cherry picked from commit 5e748e7929c93bcbce172fd2ff3a89780bf618ef) --- salt/modules/mount.py | 23 +++++++++++++--- salt/states/mount.py | 31 +++++++++++++++------- tests/unit/modules/test_mount.py | 45 +++++++++++++++++++++++++++++++- tests/unit/states/test_mount.py | 36 ++++++++++++++++--------- 4 files changed, 109 insertions(+), 26 deletions(-) diff --git a/salt/modules/mount.py b/salt/modules/mount.py index 541a2c9cc6ac..f2737b9a5ca4 100644 --- a/salt/modules/mount.py +++ b/salt/modules/mount.py @@ -711,11 +711,15 @@ def set_fstab( config='/etc/fstab', test=False, match_on='auto', + not_change=False, **kwargs): ''' Verify that this mount is represented in the fstab, change the mount to match the data passed, or add the mount if it is not present. + If the entry is found via `match_on` and `not_change` is True, the + current line will be preserved. + CLI Example: .. code-block:: bash @@ -793,7 +797,7 @@ def set_fstab( # Note: If ret isn't None here, # we've matched multiple lines ret = 'present' - if entry.match(line): + if entry.match(line) or not_change: lines.append(line) else: ret = 'change' @@ -837,12 +841,16 @@ def set_vfstab( config='/etc/vfstab', test=False, match_on='auto', + not_change=False, **kwargs): ''' ..verionadded:: 2016.3.2 Verify that this mount is represented in the fstab, change the mount to match the data passed, or add the mount if it is not present. + If the entry is found via `match_on` and `not_change` is True, the + current line will be preserved. + CLI Example: .. code-block:: bash @@ -922,7 +930,7 @@ def set_vfstab( # Note: If ret isn't None here, # we've matched multiple lines ret = 'present' - if entry.match(line): + if entry.match(line) or not_change: lines.append(line) else: ret = 'change' @@ -1023,6 +1031,7 @@ def set_automaster( opts='', config='/etc/auto_salt', test=False, + not_change=False, **kwargs): ''' Verify that this mount is represented in the auto_salt, change the mount @@ -1071,9 +1080,11 @@ def set_automaster( lines.append(line) continue if comps[0] == name or comps[2] == device_fmt: + present = True + if not_change: + continue # check to see if there are changes # and fix them if there are any - present = True if comps[0] != name: change = True comps[0] = name @@ -1672,6 +1683,7 @@ def set_filesystems( config='/etc/filesystems', test=False, match_on='auto', + not_change=False, **kwargs): ''' .. versionadded:: 2018.3.3 @@ -1679,6 +1691,9 @@ def set_filesystems( Verify that this mount is represented in the filesystems, change the mount to match the data passed, or add the mount if it is not present on AIX + If the entry is found via `match_on` and `not_change` is True, the + current line will be preserved. + Provide information if the path is mounted :param name: The name of the mount point where the device is mounted. @@ -1778,7 +1793,7 @@ def set_filesystems( for fsys_view in six.viewitems(fsys_filedict): if criteria.match(fsys_view): ret = 'present' - if entry_ip.match(fsys_view): + if entry_ip.match(fsys_view) or not_change: view_lines.append(fsys_view) else: ret = 'change' diff --git a/salt/states/mount.py b/salt/states/mount.py index 2f5946b647ad..0802bf438851 100644 --- a/salt/states/mount.py +++ b/salt/states/mount.py @@ -993,9 +993,9 @@ def _convert_to(maybe_device, convert_to): def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', fs_freq=0, fs_passno=0, mount_by=None, - config='/etc/fstab', mount=True, match_on='auto'): - ''' - Makes sure that a fstab mount point is pressent. + config='/etc/fstab', mount=True, match_on='auto', + not_change=False): + '''Makes sure that a fstab mount point is pressent. name The name of block device. Can be any valid fs_spec value. @@ -1040,6 +1040,13 @@ def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', to guess based on fstype. In general, ``auto`` matches on name for recognized special devices and device otherwise. + not_change + By default, if the entry is found in the fstab file but is + different from the expected content (like different options), + the entry will be replaced with the correct content. If this + parameter is set to ``True`` and the line is found, the + original content will be preserved. + ''' ret = { 'name': name, @@ -1080,7 +1087,8 @@ def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', fstype=fs_vfstype, opts=fs_mntops, config=config, - test=True) + test=True, + not_change=not_change) elif __grains__['os'] == 'AIX': out = __salt__['mount.set_filesystems'](name=fs_file, device=fs_spec, @@ -1089,7 +1097,8 @@ def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', mount=mount, config=config, test=True, - match_on=match_on) + match_on=match_on, + not_change=not_change) else: out = __salt__['mount.set_fstab'](name=fs_file, device=fs_spec, @@ -1099,7 +1108,8 @@ def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', pass_num=fs_passno, config=config, test=True, - match_on=match_on) + match_on=match_on, + not_change=not_change) ret['result'] = None if out == 'present': msg = '{} entry is already in {}.' @@ -1121,7 +1131,8 @@ def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', device=fs_spec, fstype=fs_vfstype, opts=fs_mntops, - config=config) + config=config, + not_change=not_change) elif __grains__['os'] == 'AIX': out = __salt__['mount.set_filesystems'](name=fs_file, device=fs_spec, @@ -1129,7 +1140,8 @@ def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', opts=fs_mntops, mount=mount, config=config, - match_on=match_on) + match_on=match_on, + not_change=not_change) else: out = __salt__['mount.set_fstab'](name=fs_file, device=fs_spec, @@ -1138,7 +1150,8 @@ def fstab_present(name, fs_file, fs_vfstype, fs_mntops='defaults', dump=fs_freq, pass_num=fs_passno, config=config, - match_on=match_on) + match_on=match_on, + not_change=not_change) ret['result'] = True if out == 'present': diff --git a/tests/unit/modules/test_mount.py b/tests/unit/modules/test_mount.py index 808cef90df46..fe9b067665aa 100644 --- a/tests/unit/modules/test_mount.py +++ b/tests/unit/modules/test_mount.py @@ -216,6 +216,21 @@ def test_set_fstab(self): mock_open(read_data=MOCK_SHELL_FILE)): self.assertEqual(mount.set_fstab('A', 'B', 'C'), 'new') + mock = MagicMock(return_value=True) + with patch.object(os.path, 'isfile', mock): + with patch('salt.utils.files.fopen', + mock_open(read_data=MOCK_SHELL_FILE)): + self.assertEqual(mount.set_fstab('B', 'A', 'C', 'D', 'F', 'G'), + 'present') + + mock = MagicMock(return_value=True) + with patch.object(os.path, 'isfile', mock): + with patch('salt.utils.files.fopen', + mock_open(read_data=MOCK_SHELL_FILE)): + self.assertEqual(mount.set_fstab('B', 'A', 'C', + not_change=True), + 'present') + def test_rm_automaster(self): ''' Remove the mount point from the auto_master @@ -239,6 +254,34 @@ def test_set_automaster(self): mount.set_automaster, 'A', 'B', 'C') + mock = MagicMock(return_value=True) + mock_read = MagicMock(side_effect=OSError) + with patch.object(os.path, 'isfile', mock): + with patch.object(salt.utils.files, 'fopen', mock_read): + self.assertRaises(CommandExecutionError, + mount.set_automaster, 'A', 'B', 'C') + + mock = MagicMock(return_value=True) + with patch.object(os.path, 'isfile', mock): + with patch('salt.utils.files.fopen', + mock_open(read_data=MOCK_SHELL_FILE)): + self.assertEqual(mount.set_automaster('A', 'B', 'C'), 'new') + + mock = MagicMock(return_value=True) + with patch.object(os.path, 'isfile', mock): + with patch('salt.utils.files.fopen', + mock_open(read_data='/..A -fstype=C,D C:B')): + self.assertEqual(mount.set_automaster('A', 'B', 'C', 'D'), + 'present') + + mock = MagicMock(return_value=True) + with patch.object(os.path, 'isfile', mock): + with patch('salt.utils.files.fopen', + mock_open(read_data='/..A -fstype=XX C:B')): + self.assertEqual(mount.set_automaster('A', 'B', 'C', 'D', + not_change=True), + 'present') + def test_automaster(self): ''' Test the list the contents of the fstab @@ -284,7 +327,7 @@ def test_set_filesystems(self): with patch.dict(mount.__grains__, {'os': 'AIX', 'kernel': 'AIX'}): with patch.object(os.path, 'isfile', mock): self.assertRaises(CommandExecutionError, - mount.set_filesystems, 'A', 'B', 'C') + mount.set_filesystems, 'A', 'B', 'C') mock_read = MagicMock(side_effect=OSError) with patch.object(os.path, 'isfile', mock): diff --git a/tests/unit/states/test_mount.py b/tests/unit/states/test_mount.py index 2b35626b8263..8f21db0cbb99 100644 --- a/tests/unit/states/test_mount.py +++ b/tests/unit/states/test_mount.py @@ -534,7 +534,8 @@ def test_fstab_present_macos_test_present(self): fstype='ext2', opts='noowners', config='/etc/auto_salt', - test=True) + test=True, + not_change=False) def test_fstab_present_aix_test_present(self): ''' @@ -563,7 +564,8 @@ def test_fstab_present_aix_test_present(self): opts='', config='/etc/filesystems', test=True, - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_test_present(self): ''' @@ -593,7 +595,8 @@ def test_fstab_present_test_present(self): pass_num=0, config='/etc/fstab', test=True, - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_test_new(self): ''' @@ -623,7 +626,8 @@ def test_fstab_present_test_new(self): pass_num=0, config='/etc/fstab', test=True, - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_test_change(self): ''' @@ -653,7 +657,8 @@ def test_fstab_present_test_change(self): pass_num=0, config='/etc/fstab', test=True, - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_test_error(self): ''' @@ -683,7 +688,8 @@ def test_fstab_present_test_error(self): pass_num=0, config='/etc/fstab', test=True, - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_macos_present(self): ''' @@ -709,7 +715,8 @@ def test_fstab_present_macos_present(self): device='/dev/sda1', fstype='ext2', opts='noowners', - config='/etc/auto_salt') + config='/etc/auto_salt', + not_change=False) def test_fstab_present_aix_present(self): ''' @@ -737,7 +744,8 @@ def test_fstab_present_aix_present(self): mount=True, opts='', config='/etc/filesystems', - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_present(self): ''' @@ -766,7 +774,8 @@ def test_fstab_present_present(self): dump=0, pass_num=0, config='/etc/fstab', - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_new(self): ''' @@ -795,7 +804,8 @@ def test_fstab_present_new(self): dump=0, pass_num=0, config='/etc/fstab', - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_change(self): ''' @@ -824,7 +834,8 @@ def test_fstab_present_change(self): dump=0, pass_num=0, config='/etc/fstab', - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_present_fail(self): ''' @@ -853,7 +864,8 @@ def test_fstab_present_fail(self): dump=0, pass_num=0, config='/etc/fstab', - match_on='auto') + match_on='auto', + not_change=False) def test_fstab_absent_macos_test_absent(self): '''