Only update props when changed in zpool.present

We make sure the values conform to what zfs expects, this should
fix the unneeded updates as mentioned in #44404

Additional test for zpool.get with whitespaces in the result.

salt/modules/zpool.py

@@ -16,7 +16,7 @@ import salt.utils.decorators
 import salt.utils.decorators.path
 import salt.utils.path
 from salt.utils.odict import OrderedDict
-from salt.utils.stringutils import to_num as _conform_value
+from salt.utils.stringutils import to_num as str_to_num
 
 log = logging.getLogger(__name__)
 
@@ -70,6 +70,26 @@ def _check_mkfile():
     return salt.utils.path.which('mkfile')
 
 
+def _conform_value(value):
+    '''
+    Ensure value always conform to what zpool expects
+    '''
+    if isinstance(value, bool):  # NOTE: salt breaks the on/off/yes/no properties
+        return 'on' if value else 'off'
+
+    if isinstance(value, str) and ' ' in value:  # NOTE: handle whitespaces
+        # NOTE: quoting the string may be better
+        #       but it is hard to know if we already quoted it before
+        #       this can be improved in the future
+        return "'{0}'".format(value.strip("'"))
+
+    if isinstance(value, str):  # NOTE: convert to numeric if needed
+        return str_to_num(value)
+
+    # NOTE: passthrough
+    return value
+
+
 def healthy():
     '''
     .. versionadded:: 2016.3.0
@@ -458,10 +478,9 @@ def set(zpool, prop, value):
     '''
     ret = {}
     ret[zpool] = {}
-    if isinstance(value, bool):
-        value = 'on' if value else 'off'
-    elif ' ' in value:
-        value = "'{0}'".format(value)
+
+    # make sure value is what zfs expects
+    value = _conform_value(value)
 
     # get zpool list data
     zpool_cmd = _check_zpool()
@@ -686,34 +705,30 @@ def create(zpool, *vdevs, **kwargs):
     if properties and 'bootsize' in properties:
         createboot = True
 
+    # make sure values are in the format zfs expects
+    if properties:
+        for prop in properties:
+            properties[prop] = _conform_value(properties[prop])
+
+    if filesystem_properties:
+        for prop in filesystem_properties:
+            filesystem_properties[prop] = _conform_value(filesystem_properties[prop])
+
     # apply extra arguments from kwargs
     if force:  # force creation
         cmd = '{0} -f'.format(cmd)
     if createboot:  # create boot paritition
         cmd = '{0} -B'.format(cmd)
     if properties:  # create "-o property=value" pairs
-        optlist = []
+        proplist = []
         for prop in properties:
-            if isinstance(properties[prop], bool):
-                value = 'on' if properties[prop] else 'off'
-            else:
-                if ' ' in properties[prop]:
-                    value = "'{0}'".format(properties[prop])
-                else:
-                    value = properties[prop]
-            optlist.append('-o {0}={1}'.format(prop, value))
-        opts = ' '.join(optlist)
-        cmd = '{0} {1}'.format(cmd, opts)
+            proplist.append('-o {0}={1}'.format(prop, properties[prop]))
+        cmd = '{0} {1}'.format(cmd, ' '.join(proplist))
     if filesystem_properties:  # create "-O property=value" pairs
-        optlist = []
+        fsproplist = []
         for prop in filesystem_properties:
-            if ' ' in filesystem_properties[prop]:
-                value = "'{0}'".format(filesystem_properties[prop])
-            else:
-                value = filesystem_properties[prop]
-            optlist.append('-O {0}={1}'.format(prop, value))
-        opts = ' '.join(optlist)
-        cmd = '{0} {1}'.format(cmd, opts)
+            fsproplist.append('-O {0}={1}'.format(prop, filesystem_properties[prop]))
+        cmd = '{0} {1}'.format(cmd, ' '.join(fsproplist))
     if mountpoint:  # set mountpoint
         cmd = '{0} -m {1}'.format(cmd, mountpoint)
     if altroot:  # set altroot

salt/states/zpool.py

@@ -73,6 +73,7 @@ import logging
 
 # Import Salt libs
 from salt.utils.odict import OrderedDict
+from salt.modules.zpool import _conform_value
 
 log = logging.getLogger(__name__)
 
@@ -186,6 +187,10 @@ def present(name, properties=None, filesystem_properties=None, layout=None, conf
 
         log.debug('zpool.present::{0}::layout - {1}'.format(name, layout))
 
+    # ensure properties conform to the zfs parsable format
+    for prop in properties:
+        properties[prop] = _conform_value(properties[prop])
+
     # ensure the pool is present
     ret['result'] = False
     if __salt__['zpool.exists'](name):  # update
@@ -200,26 +205,15 @@ def present(name, properties=None, filesystem_properties=None, layout=None, conf
             if prop not in properties_current:
                 continue
 
-            value = properties[prop]
-            if isinstance(value, bool):
-                value = 'on' if value else 'off'
-
-            if properties_current[prop] != value:
+            if properties_current[prop] != properties[prop]:
                 properties_update.append(prop)
 
         # update properties
         for prop in properties_update:
-            value = properties[prop]
-            res = __salt__['zpool.set'](name, prop, value)
-
-            # also transform value so we match with the return
-            if isinstance(value, bool):
-                value = 'on' if value else 'off'
-            elif ' ' in value:
-                value = "'{0}'".format(value)
+            res = __salt__['zpool.set'](name, prop, properties[prop])
 
             # check return
-            if name in res and prop in res[name] and res[name][prop] == value:
+            if name in res and prop in res[name] and res[name][prop] == properties[prop]:
                 if name not in ret['changes']:
                     ret['changes'][name] = {}
                 ret['changes'][name].update(res[name])

tests/unit/modules/test_zpool.py

@@ -184,6 +184,20 @@ class ZpoolTestCase(TestCase, LoaderModuleMockMixin):
             res = OrderedDict([('mypool', OrderedDict([('size', 1992864825344)]))])
             self.assertEqual(res, ret)
 
+    def test_get_whitespace(self):
+        '''
+        Tests successful return of get function with a string with whitespaces
+        '''
+        ret = {}
+        ret['stdout'] = "comment\tmy testing pool\t-\n"
+        ret['stderr'] = ""
+        ret['retcode'] = 0
+        mock_cmd = MagicMock(return_value=ret)
+        with patch.dict(zpool.__salt__, {'cmd.run_all': mock_cmd}):
+            ret = zpool.get('mypool', 'comment')
+            res = OrderedDict([('mypool', OrderedDict([('comment', "'my testing pool'")]))])
+            self.assertEqual(res, ret)
+
     def test_scrub_start(self):
         '''
         Tests start of scrub