Skip to content

Commit

Permalink
mount: add not_change parameter to set_fstab and similars
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aplanas committed May 10, 2019
1 parent 2aa1832 commit 5e748e7
Show file tree
Hide file tree
Showing 4 changed files with 109 additions and 26 deletions.
23 changes: 19 additions & 4 deletions salt/modules/mount.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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'
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1672,13 +1683,17 @@ def set_filesystems(
config='/etc/filesystems',
test=False,
match_on='auto',
not_change=False,
**kwargs):
'''
.. versionadded:: 2018.3.3
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.
Expand Down Expand Up @@ -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'
Expand Down
31 changes: 22 additions & 9 deletions salt/states/mount.py
Original file line number Diff line number Diff line change
Expand Up @@ -1018,9 +1018,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.
Expand Down Expand Up @@ -1065,6 +1065,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,
Expand Down Expand Up @@ -1105,7 +1112,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,
Expand All @@ -1114,7 +1122,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,
Expand All @@ -1124,7 +1133,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 {}.'
Expand All @@ -1146,15 +1156,17 @@ 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,
fstype=fs_vfstype,
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,
Expand All @@ -1163,7 +1175,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':
Expand Down
45 changes: 44 additions & 1 deletion tests/unit/modules/test_mount.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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):
Expand Down
36 changes: 24 additions & 12 deletions tests/unit/states/test_mount.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand All @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down Expand Up @@ -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):
'''
Expand Down

0 comments on commit 5e748e7

Please sign in to comment.