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.

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)

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}),
         }