From ec0e4d328d11f7b754dbb4354bf58e8b19ef2e19 Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Sat, 17 Jan 2015 15:00:45 -0500
Subject: [PATCH 01/11] Adding rework for mount match-on criteria. Implimented
 core class in mount/modules to represent and match fstab entries.
 Re-implimented related methods in DRY compliance.

---
 salt/modules/mount.py | 268 +++++++++++++++++++++++-------------------
 salt/states/mount.py  |  14 ++-
 2 files changed, 159 insertions(+), 123 deletions(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index 7c1d1c0a92..7e3f242b97 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -188,6 +188,69 @@ def active(extended=False):
     return ret
 
 
+class _fstab_entry():
+	'''
+	Utility class for manipulating fstab entries. Primarily we're parsing,
+	formating, and comparing lines. Parsing emits dicts expected from
+	fstab() or raises a ValueError.
+	'''
+
+	class ParseError( ValueError ):
+		pass
+
+	fstab_keys = [ 'device', 'name', 'fstype', 'opts', 'dump', 'pass_num' ]
+
+	# preserve data format
+	compatibility_keys = [ 'device', 'name', 'fstype', 'opts', 'dump', 'pass' ]
+
+	fstab_format = '{device}\t\t{name}\t{fstype}\t{opts}\t{dump} {pass_num}\n'
+
+	@classmethod
+	def dict_from_line( cls, line, keys = fstab_keys ):
+		if len( keys ) != 6:
+			raise ValueError( 'Invalid key array: {0}'.format( keys ) )
+		if line.startswith('#'):
+			raise cls.ParseError( "Comment!" )
+
+		comps = line.split()
+		if len(comps) != 6:
+			raise cls.ParseError( "Invalid Entry!" )
+
+		return dict( zip( keys, comps ) )
+
+	@classmethod
+	def from_line( cls, *args, **kwargs ):
+		return cls( ** cls.dict_from_line( *args, **kwargs ) )
+
+	@classmethod
+	def dict_to_line( cls, entry ):
+		return cls.fstab_format.format( **entry )
+
+	def __str__( self ):
+		'''string value, only works for full repr'''
+		return self.dict_to_line( self.criteria )
+
+	def __repr__( self ):
+		'''always works'''
+		return str( self.criteria )
+
+	def pick( self, keys ):
+		'''returns an instance with just those keys'''
+		return self.__class__( **dict( map( lambda key: ( key, self.criteria[ key ] ), keys ) ) )
+
+	def __init__( self, **criteria ):
+		'''Store non-empty, non-null values to use as filter'''
+		self.criteria = dict( filter( lambda (key,value): value ), criteria.items )
+
+	def match( self, line ):
+		'''compare potentially partial criteria against line'''
+		entry = self.line_to_dict( line )
+		for key, value in self.criteria.items():
+			if entry[ key ] != value:
+				return False
+		return True
+
+
 def fstab(config='/etc/fstab'):
     '''
     List the contents of the fstab
@@ -203,25 +266,16 @@ def fstab(config='/etc/fstab'):
         return ret
     with salt.utils.fopen(config) as ifile:
         for line in ifile:
-            if line.startswith('#'):
-                # Commented
-                continue
-            if not line.strip():
-                # Blank line
-                continue
-            comps = line.split()
-            if len(comps) != 6:
-                # Invalid entry
-                continue
-            ret[comps[1]] = {'device': comps[0],
-                             'fstype': comps[2],
-                             'opts': comps[3].split(','),
-                             'dump': comps[4],
-                             'pass': comps[5]}
+	    try:
+                entry = _fstab_entry.line_to_dict( line, _fstab_entry.compatibility_keys )
+                ret[ entry.pop( 'name' ) ] = entry
+            except _fstab_entry.ParseError:
+                pass
+
     return ret
 
 
-def rm_fstab(name, device, config='/etc/fstab'):
+def rm_fstab(name, device, config='/etc/fstab' ):
     '''
     Remove the mount point from the fstab
 
@@ -231,45 +285,39 @@ def rm_fstab(name, device, config='/etc/fstab'):
 
         salt '*' mount.rm_fstab /mnt/foo
     '''
-    contents = fstab(config)
-    if name not in contents:
-        return True
-    # The entry is present, get rid of it
+    modified = False
+
+    if isinstance( opts, list ):
+        opts = ','.join( opts )
+
+    criteria = _fstab_entry( 
+	name = name, 
+	device = device )
+
     lines = []
     try:
         with salt.utils.fopen(config, 'r') as ifile:
             for line in ifile:
-                if line.startswith('#'):
-                    # Commented
-                    lines.append(line)
-                    continue
-                if not line.strip():
-                    # Blank line
-                    lines.append(line)
-                    continue
-                comps = line.split()
-                if len(comps) != 6:
-                    # Invalid entry
-                    lines.append(line)
-                    continue
-                comps = line.split()
-                if device:
-                    if comps[1] == name and comps[0] == device:
-                        continue
-                else:
-                    if comps[1] == name:
-                        continue
-                lines.append(line)
+                try:
+                    if criteria.match( line ):
+                        modified = True
+                    else:
+                        lines.append( line )
+
+                except _fstab_entry.ParseError:
+                    lines.append( line )
+
     except (IOError, OSError) as exc:
         msg = "Couldn't read from {0}: {1}"
         raise CommandExecutionError(msg.format(config, str(exc)))
 
-    try:
-        with salt.utils.fopen(config, 'w+') as ofile:
-            ofile.writelines(lines)
-    except (IOError, OSError) as exc:
-        msg = "Couldn't write to {0}: {1}"
-        raise CommandExecutionError(msg.format(config, str(exc)))
+    if modified:
+        try:
+            with salt.utils.fopen(config, 'w+') as ofile:
+                ofile.writelines(lines)
+        except (IOError, OSError) as exc:
+            msg = "Couldn't write to {0}: {1}"
+            raise CommandExecutionError(msg.format(config, str(exc)))
     return True
 
 
@@ -282,6 +330,7 @@ def set_fstab(
         pass_num=0,
         config='/etc/fstab',
         test=False,
+        match_on = 'auto',
         **kwargs):
     '''
     Verify that this mount is represented in the fstab, change the mount
@@ -293,66 +342,71 @@ def set_fstab(
 
         salt '*' mount.set_fstab /mnt/foo /dev/sdz1 ext4
     '''
+
     # Fix the opts type if it is a list
     if isinstance(opts, list):
         opts = ','.join(opts)
-    lines = []
-    change = False
-    present = False
 
+    # preserve arguments for updating
+    entry_args = dict( vars() )
+    map( entry_args.pop, [ 'test', 'config', 'match_on', 'kwargs' ] )
+
+    lines = []
+    ret = None
+
+    # Transform match_on into list--items will be checked later
+    if isinstance( match_on, list ):
+        pass
+    elif not isinstance( match_on, string_types ):
+	raise CommandExecutionError( 'match_on must be a string or list of strings' )
+    elif match_on == 'auto':
+        # Try to guess right criteria for auto....missing some special fstypes here
+        if fstype in frozenset([ 'tmpfs', 'sysfs', 'proc', 'fusectl', 'debugfs', 'securityfs', 'devtmpfs', 'cgroup']):
+            match_on = [ 'name' ]
+        else:
+            match_on = [ 'device' ]
+    else:
+	match_on = [ match_on ]
+
+    # generate entry and criteria objects, handle invalid keys in match_on
+    entry = _fstab_entry( **entry_args )
+    try:
+        criteria = entry.pick( match_on )
+
+    except KeyError:
+        invalid_keys = filter( lambda key: key in _fstab_entry.fstab_keys, match_on )
+        raise CommandExecutionError( 'Unrecognized keys in match_on: "{0}"'.format( invalid_keys ) )
+
+    # parse file, use ret to cache status
     if not os.path.isfile(config):
         raise CommandExecutionError('Bad config file "{0}"'.format(config))
 
     try:
         with salt.utils.fopen(config, 'r') as ifile:
             for line in ifile:
-                if line.startswith('#'):
-                    # Commented
-                    lines.append(line)
-                    continue
-                if not line.strip():
-                    # Blank line
-                    lines.append(line)
-                    continue
-                comps = line.split()
-                if len(comps) != 6:
-                    # Invalid entry
-                    lines.append(line)
-                    continue
-                if comps[1] == name and comps[0] == device:
-                    # check to see if there are changes
-                    # and fix them if there are any
-                    present = True
-                    if comps[2] != fstype:
-                        change = True
-                        comps[2] = fstype
-                    if comps[3] != opts:
-                        change = True
-                        comps[3] = opts
-                    if comps[4] != str(dump):
-                        change = True
-                        comps[4] = str(dump)
-                    if comps[5] != str(pass_num):
-                        change = True
-                        comps[5] = str(pass_num)
-                    if change:
-                        log.debug(
-                            'fstab entry for mount point {0} needs to be '
-                            'updated'.format(name)
-                        )
-                        newline = (
-                            '{0}\t\t{1}\t{2}\t{3}\t{4} {5}\n'.format(
-                                device, name, fstype, opts, dump, pass_num
-                            )
-                        )
-                        lines.append(newline)
-                else:
-                    lines.append(line)
+                try:
+                    if criteria.match( line ):
+                        # Note: If ret isn't None here, we've matched multiple lines
+                        ret = 'present'
+                        if entry.match( line ):
+                            lines.append( line )
+                        else
+                            ret = 'change'
+                            lines.append( str( entry ) )
+                except _fstab_entry.ParseError:
+                    lines.append( line )                        
+
     except (IOError, OSError) as exc:
         msg = 'Couldn\'t read from {0}: {1}'
         raise CommandExecutionError(msg.format(config, str(exc)))
 
-    if change:
+
+    # add line if not present or changed
+    if ret is None:
+	lines.append( str( entry ) )
+        ret = 'new'
+
+    if ret != 'present': # ret in [ 'new', 'change' ]:
         if not salt.utils.test_mode(test=test, **kwargs):
             try:
                 with salt.utils.fopen(config, 'w+') as ofile:
@@ -362,33 +416,7 @@ def set_fstab(
                 msg = 'File not writable {0}'
                 raise CommandExecutionError(msg.format(config))
 
-        return 'change'
-
-    if not change:
-        if present:
-            # The right entry is already here
-            return 'present'
-        else:
-            if not salt.utils.test_mode(test=test, **kwargs):
-                # The entry is new, add it to the end of the fstab
-                newline = '{0}\t\t{1}\t{2}\t{3}\t{4} {5}\n'.format(device,
-                                                                   name,
-                                                                   fstype,
-                                                                   opts,
-                                                                   dump,
-                                                                   pass_num)
-                lines.append(newline)
-                try:
-                    with salt.utils.fopen(config, 'w+') as ofile:
-                        # The line was changed, commit it!
-                        ofile.writelines(lines)
-                except (IOError, OSError):
-                    raise CommandExecutionError(
-                        'File not writable {0}'.format(
-                            config
-                        )
-                    )
-    return 'new'
+    return ret
 
 
 def rm_automaster(name, device, config='/etc/auto_salt'):
diff --git a/salt/states/mount.py b/salt/states/mount.py
index bfb9fd7ee5..0a4e5cb067 100644
--- a/salt/states/mount.py
+++ b/salt/states/mount.py
@@ -50,7 +50,8 @@ def mounted(name,
             config='/etc/fstab',
             persist=True,
             mount=True,
-            user=None):
+            user=None,
+            match_on='auto'):
     '''
     Verify that a device is mounted
 
@@ -90,6 +91,11 @@ def mounted(name,
     user
         The user to own the mount; this defaults to the user salt is
         running as on the minion
+    match_on
+        A name or list of fstab properties on which this state shoudl 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,
            'changes': {},
@@ -315,7 +321,8 @@ def mounted(name,
                                                   dump,
                                                   pass_num,
                                                   config,
-                                                  test=True)
+                                                  test=True,
+                                                  match_on=match_on)
             if out != 'present':
                 ret['result'] = None
                 if out == 'new':
@@ -357,7 +364,8 @@ def mounted(name,
                                                   opts,
                                                   dump,
                                                   pass_num,
-                                                  config)
+                                                  config,
+                                                  match_on=match_on)
 
         if out == 'present':
             ret['comment'] += '. Entry already exists in the fstab.'

From 3fd2e254f9cfdb3aaf370c1f9a1774b63267fd1f Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Sat, 17 Jan 2015 23:10:33 -0500
Subject: [PATCH 02/11] Debugged & moderately tested match_on functionality for
 modules/mount and states/mount with relation to fstab.

---
 salt/modules/mount.py | 26 +++++++++++++++++++-------
 1 file changed, 19 insertions(+), 7 deletions(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index 7e3f242b97..b92fefff54 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -193,6 +193,8 @@ class _fstab_entry():
 	Utility class for manipulating fstab entries. Primarily we're parsing,
 	formating, and comparing lines. Parsing emits dicts expected from
 	fstab() or raises a ValueError.
+
+	Note: We'll probably want to use os.normpath and os.normcase on 'name'
 	'''
 
 	class ParseError( ValueError ):
@@ -240,11 +242,18 @@ class _fstab_entry():
 
 	def __init__( self, **criteria ):
 		'''Store non-empty, non-null values to use as filter'''
-		self.criteria = dict( filter( lambda (key,value): value ), criteria.items )
+		items = filter( lambda (key, value): value is not None, criteria.items() )
+		items = map( lambda (key, value): (key, str( value ) ), items )
+		self.criteria = dict( items )
+
+	@staticmethod
+	def norm_path( path ):
+		'''Resolve equivalent paths equivalently'''
+		return os.path.normcase( os.path.normpath( path ) )
 
 	def match( self, line ):
 		'''compare potentially partial criteria against line'''
-		entry = self.line_to_dict( line )
+		entry = self.dict_from_line( line )
 		for key, value in self.criteria.items():
 			if entry[ key ] != value:
 				return False
@@ -267,7 +276,7 @@ def fstab(config='/etc/fstab'):
     with salt.utils.fopen(config) as ifile:
         for line in ifile:
 	    try:
-                entry = _fstab_entry.line_to_dict( line, _fstab_entry.compatibility_keys )
+                entry = _fstab_entry.dict_from_line( line, _fstab_entry.compatibility_keys )
                 ret[ entry.pop( 'name' ) ] = entry
             except _fstab_entry.ParseError:
                 pass
@@ -287,9 +296,6 @@ def rm_fstab(name, device, config='/etc/fstab' ):
     '''
     modified = False
 
-    if isinstance( opts, list ):
-        opts = ','.join( opts )
-
     criteria = _fstab_entry( 
 	name = name, 
 	device = device )
@@ -318,6 +324,9 @@ def rm_fstab(name, device, config='/etc/fstab' ):
         except (IOError, OSError) as exc:
             msg = "Couldn't write to {0}: {1}"
             raise CommandExecutionError(msg.format(config, str(exc)))
+
+    # Note: not clear why we always return 'True'
+    # --just copying previous behavior at this point...
     return True
 
 
@@ -390,9 +399,12 @@ def set_fstab(
                         ret = 'present'
                         if entry.match( line ):
                             lines.append( line )
-                        else
+                        else:
                             ret = 'change'
                             lines.append( str( entry ) )
+                    else:
+                       lines.append( line )
+
                 except _fstab_entry.ParseError:
                     lines.append( line )                        
 

From 61250467e426e4769f0302ceb3ba79fcc8b2b1a3 Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Sat, 17 Jan 2015 23:23:23 -0500
Subject: [PATCH 03/11] Debugged & moderately tested match_on functionality for
 modules/mount and states/mount with relation to fstab.

---
 salt/modules/mount.py | 2 +-
 salt/states/mount.py  | 3 ++-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index b92fefff54..491c76fd00 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -383,7 +383,7 @@ def set_fstab(
         criteria = entry.pick( match_on )
 
     except KeyError:
-        invalid_keys = filter( lambda key: key in _fstab_entry.fstab_keys, match_on )
+        invalid_keys = filter( lambda key: key not in _fstab_entry.fstab_keys, match_on )
         raise CommandExecutionError( 'Unrecognized keys in match_on: "{0}"'.format( invalid_keys ) )
 
     # parse file, use ret to cache status
diff --git a/salt/states/mount.py b/salt/states/mount.py
index 0a4e5cb067..8f0c414243 100644
--- a/salt/states/mount.py
+++ b/salt/states/mount.py
@@ -91,8 +91,9 @@ def mounted(name,
     user
         The user to own the mount; this defaults to the user salt is
         running as on the minion
+
     match_on
-        A name or list of fstab properties on which this state shoudl be applied. 
+        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.

From 9b1caf6b427d9b9c09a183f6f01277e999951abe Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Sun, 18 Jan 2015 00:04:35 -0500
Subject: [PATCH 04/11] Normalizing white space to four spaces to remedy pylint
 errors

---
 salt/modules/mount.py | 120 +++++++++++++++++++++---------------------
 1 file changed, 60 insertions(+), 60 deletions(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index 491c76fd00..fe01fdc77b 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -188,76 +188,76 @@ def active(extended=False):
     return ret
 
 
-class _fstab_entry():
-	'''
-	Utility class for manipulating fstab entries. Primarily we're parsing,
-	formating, and comparing lines. Parsing emits dicts expected from
-	fstab() or raises a ValueError.
+class _fstab_entry( object ):
+    '''
+    Utility class for manipulating fstab entries. Primarily we're parsing,
+    formating, and comparing lines. Parsing emits dicts expected from
+    fstab() or raises a ValueError.
 
-	Note: We'll probably want to use os.normpath and os.normcase on 'name'
-	'''
+    Note: We'll probably want to use os.normpath and os.normcase on 'name'
+    '''
 
-	class ParseError( ValueError ):
-		pass
+    class ParseError( ValueError ):
+        '''Error raised when a line isn't parsible as an fstab entry'''
 
-	fstab_keys = [ 'device', 'name', 'fstype', 'opts', 'dump', 'pass_num' ]
+    fstab_keys = [ 'device', 'name', 'fstype', 'opts', 'dump', 'pass_num' ]
 
-	# preserve data format
-	compatibility_keys = [ 'device', 'name', 'fstype', 'opts', 'dump', 'pass' ]
+    # preserve data format
+    compatibility_keys = [ 'device', 'name', 'fstype', 'opts', 'dump', 'pass' ]
 
-	fstab_format = '{device}\t\t{name}\t{fstype}\t{opts}\t{dump} {pass_num}\n'
+    fstab_format = '{device}\t\t{name}\t{fstype}\t{opts}\t{dump} {pass_num}\n'
 
-	@classmethod
-	def dict_from_line( cls, line, keys = fstab_keys ):
-		if len( keys ) != 6:
-			raise ValueError( 'Invalid key array: {0}'.format( keys ) )
-		if line.startswith('#'):
-			raise cls.ParseError( "Comment!" )
+    @classmethod
+    def dict_from_line( cls, line, keys = fstab_keys ):
+        if len( keys ) != 6:
+            raise ValueError( 'Invalid key array: {0}'.format( keys ) )
+        if line.startswith('#'):
+            raise cls.ParseError( "Comment!" )
 
-		comps = line.split()
-		if len(comps) != 6:
-			raise cls.ParseError( "Invalid Entry!" )
+        comps = line.split()
+        if len(comps) != 6:
+            raise cls.ParseError( "Invalid Entry!" )
 
-		return dict( zip( keys, comps ) )
+        return dict( zip( keys, comps ) )
 
-	@classmethod
-	def from_line( cls, *args, **kwargs ):
-		return cls( ** cls.dict_from_line( *args, **kwargs ) )
+    @classmethod
+    def from_line( cls, *args, **kwargs ):
+        return cls( ** cls.dict_from_line( *args, **kwargs ) )
 
-	@classmethod
-	def dict_to_line( cls, entry ):
-		return cls.fstab_format.format( **entry )
+    @classmethod
+    def dict_to_line( cls, entry ):
+        return cls.fstab_format.format( **entry )
 
-	def __str__( self ):
-		'''string value, only works for full repr'''
-		return self.dict_to_line( self.criteria )
+    def __str__( self ):
+        '''string value, only works for full repr'''
+        return self.dict_to_line( self.criteria )
 
-	def __repr__( self ):
-		'''always works'''
-		return str( self.criteria )
+    def __repr__( self ):
+        '''always works'''
+        return str( self.criteria )
 
-	def pick( self, keys ):
-		'''returns an instance with just those keys'''
-		return self.__class__( **dict( map( lambda key: ( key, self.criteria[ key ] ), keys ) ) )
+    def pick( self, keys ):
+        '''returns an instance with just those keys'''
+        return self.__class__( **dict( map( lambda key: ( key, self.criteria[ key ] ), keys ) ) )
 
-	def __init__( self, **criteria ):
-		'''Store non-empty, non-null values to use as filter'''
-		items = filter( lambda (key, value): value is not None, criteria.items() )
-		items = map( lambda (key, value): (key, str( value ) ), items )
-		self.criteria = dict( items )
+    def __init__( self, **criteria ):
+        '''Store non-empty, non-null values to use as filter'''
+        items = filter( lambda (key, value): value is not None, criteria.items() )
+        items = map( lambda (key, value): (key, str( value ) ), items )
+        self.criteria = dict( items )
 
-	@staticmethod
-	def norm_path( path ):
-		'''Resolve equivalent paths equivalently'''
-		return os.path.normcase( os.path.normpath( path ) )
+    @staticmethod
+    def norm_path( path ):
+        '''Resolve equivalent paths equivalently'''
+        return os.path.normcase( os.path.normpath( path ) )
 
-	def match( self, line ):
-		'''compare potentially partial criteria against line'''
-		entry = self.dict_from_line( line )
-		for key, value in self.criteria.items():
-			if entry[ key ] != value:
-				return False
-		return True
+    def match( self, line ):
+        '''compare potentially partial criteria against line'''
+        entry = self.dict_from_line( line )
+        for key, value in self.criteria.items():
+            if entry[ key ] != value:
+                return False
+        return True
 
 
 def fstab(config='/etc/fstab'):
@@ -275,7 +275,7 @@ def fstab(config='/etc/fstab'):
         return ret
     with salt.utils.fopen(config) as ifile:
         for line in ifile:
-	    try:
+        try:
                 entry = _fstab_entry.dict_from_line( line, _fstab_entry.compatibility_keys )
                 ret[ entry.pop( 'name' ) ] = entry
             except _fstab_entry.ParseError:
@@ -297,8 +297,8 @@ def rm_fstab(name, device, config='/etc/fstab' ):
     modified = False
 
     criteria = _fstab_entry( 
-	name = name, 
-	device = device )
+        name = name, 
+        device = device )
 
     lines = []
     try:
@@ -367,7 +367,7 @@ def set_fstab(
     if isinstance( match_on, list ):
         pass
     elif not isinstance( match_on, string_types ):
-	raise CommandExecutionError( 'match_on must be a string or list of strings' )
+        raise CommandExecutionError( 'match_on must be a string or list of strings' )
     elif match_on == 'auto':
         # Try to guess right criteria for auto....missing some special fstypes here
         if fstype in frozenset([ 'tmpfs', 'sysfs', 'proc', 'fusectl', 'debugfs', 'securityfs', 'devtmpfs', 'cgroup']):
@@ -375,7 +375,7 @@ def set_fstab(
         else:
             match_on = [ 'device' ]
     else:
-	match_on = [ match_on ]
+        match_on = [ match_on ]
 
     # generate entry and criteria objects, handle invalid keys in match_on
     entry = _fstab_entry( **entry_args )
@@ -415,7 +415,7 @@ def set_fstab(
 
     # add line if not present or changed
     if ret is None:
-	lines.append( str( entry ) )
+        lines.append( str( entry ) )
         ret = 'new'
 
     if ret != 'present': # ret in [ 'new', 'change' ]:

From 02cc8bb63145d47977d1688eeb9291921c09a1b9 Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Sun, 18 Jan 2015 09:36:20 -0500
Subject: [PATCH 05/11] Fixed whitespace error, ran unit tests

---
 salt/modules/mount.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index fe01fdc77b..3f03fb41c4 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -275,7 +275,7 @@ def fstab(config='/etc/fstab'):
         return ret
     with salt.utils.fopen(config) as ifile:
         for line in ifile:
-        try:
+            try:
                 entry = _fstab_entry.dict_from_line( line, _fstab_entry.compatibility_keys )
                 ret[ entry.pop( 'name' ) ] = entry
             except _fstab_entry.ParseError:

From ba1dcb972b770e470b8b13f467ae4f0ff49c4bcc Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Mon, 19 Jan 2015 16:46:57 -0500
Subject: [PATCH 06/11] Updating modules/mount.py (mount.fstab) to produce an
 array for opts, rather than the opts string literal. Allows mount to pass
 unit tests

---
 salt/modules/mount.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index 3f03fb41c4..c57c58dd19 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -277,6 +277,7 @@ def fstab(config='/etc/fstab'):
         for line in ifile:
             try:
                 entry = _fstab_entry.dict_from_line( line, _fstab_entry.compatibility_keys )
+		entry[ 'opts' ] = entry[ 'opts' ].split( ',' )
                 ret[ entry.pop( 'name' ) ] = entry
             except _fstab_entry.ParseError:
                 pass

From 61a0421adc22a873aa6eb99eff73f6aab197c6d5 Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Mon, 19 Jan 2015 17:34:32 -0500
Subject: [PATCH 07/11] PEP style compliance for changes to modules/mount.py

---
 salt/modules/mount.py | 128 +++++++++++++++++++++---------------------
 1 file changed, 64 insertions(+), 64 deletions(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index c57c58dd19..a89516ebce 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -188,7 +188,7 @@ def active(extended=False):
     return ret
 
 
-class _fstab_entry( object ):
+class _fstab_entry(object):
     '''
     Utility class for manipulating fstab entries. Primarily we're parsing,
     formating, and comparing lines. Parsing emits dicts expected from
@@ -197,65 +197,65 @@ class _fstab_entry( object ):
     Note: We'll probably want to use os.normpath and os.normcase on 'name'
     '''
 
-    class ParseError( ValueError ):
+    class ParseError(ValueError):
         '''Error raised when a line isn't parsible as an fstab entry'''
 
-    fstab_keys = [ 'device', 'name', 'fstype', 'opts', 'dump', 'pass_num' ]
+    fstab_keys = ['device', 'name', 'fstype', 'opts', 'dump', 'pass_num']
 
     # preserve data format
-    compatibility_keys = [ 'device', 'name', 'fstype', 'opts', 'dump', 'pass' ]
+    compatibility_keys = ['device', 'name', 'fstype', 'opts', 'dump', 'pass']
 
     fstab_format = '{device}\t\t{name}\t{fstype}\t{opts}\t{dump} {pass_num}\n'
 
     @classmethod
-    def dict_from_line( cls, line, keys = fstab_keys ):
-        if len( keys ) != 6:
-            raise ValueError( 'Invalid key array: {0}'.format( keys ) )
+    def dict_from_line(cls, line, keys = fstab_keys):
+        if len(keys) != 6:
+            raise ValueError('Invalid key array: {0}'.format(keys))
         if line.startswith('#'):
-            raise cls.ParseError( "Comment!" )
+            raise cls.ParseError("Comment!")
 
         comps = line.split()
         if len(comps) != 6:
-            raise cls.ParseError( "Invalid Entry!" )
+            raise cls.ParseError("Invalid Entry!")
 
-        return dict( zip( keys, comps ) )
+        return dict(zip(keys, comps))
 
     @classmethod
-    def from_line( cls, *args, **kwargs ):
-        return cls( ** cls.dict_from_line( *args, **kwargs ) )
+    def from_line(cls, *args, **kwargs):
+        return cls(** cls.dict_from_line(*args, **kwargs))
 
     @classmethod
-    def dict_to_line( cls, entry ):
-        return cls.fstab_format.format( **entry )
+    def dict_to_line(cls, entry):
+        return cls.fstab_format.format(**entry)
 
-    def __str__( self ):
+    def __str__(self):
         '''string value, only works for full repr'''
-        return self.dict_to_line( self.criteria )
+        return self.dict_to_line(self.criteria)
 
-    def __repr__( self ):
+    def __repr__(self):
         '''always works'''
-        return str( self.criteria )
+        return str(self.criteria)
 
-    def pick( self, keys ):
+    def pick(self, keys):
         '''returns an instance with just those keys'''
-        return self.__class__( **dict( map( lambda key: ( key, self.criteria[ key ] ), keys ) ) )
+        return self.__class__(**dict(map(lambda key: (key, self.criteria[key]), keys)))
 
-    def __init__( self, **criteria ):
+    def __init__(self, **criteria):
         '''Store non-empty, non-null values to use as filter'''
-        items = filter( lambda (key, value): value is not None, criteria.items() )
-        items = map( lambda (key, value): (key, str( value ) ), items )
-        self.criteria = dict( items )
+        items = filter(lambda (key, value): value is not None, criteria.items())
+        items = map(lambda (key, value): (key, str(value)), items)
+        self.criteria = dict(items)
 
     @staticmethod
-    def norm_path( path ):
+    def norm_path(path):
         '''Resolve equivalent paths equivalently'''
-        return os.path.normcase( os.path.normpath( path ) )
+        return os.path.normcase(os.path.normpath(path))
 
-    def match( self, line ):
+    def match(self, line):
         '''compare potentially partial criteria against line'''
-        entry = self.dict_from_line( line )
+        entry = self.dict_from_line(line)
         for key, value in self.criteria.items():
-            if entry[ key ] != value:
+            if entry[key] != value:
                 return False
         return True
 
@@ -276,16 +276,16 @@ def fstab(config='/etc/fstab'):
     with salt.utils.fopen(config) as ifile:
         for line in ifile:
             try:
-                entry = _fstab_entry.dict_from_line( line, _fstab_entry.compatibility_keys )
-		entry[ 'opts' ] = entry[ 'opts' ].split( ',' )
-                ret[ entry.pop( 'name' ) ] = entry
+                entry = _fstab_entry.dict_from_line(line, _fstab_entry.compatibility_keys)
+		entry['opts'] = entry['opts'].split(',')
+                ret[entry.pop('name')] = entry
             except _fstab_entry.ParseError:
                 pass
 
     return ret
 
 
-def rm_fstab(name, device, config='/etc/fstab' ):
+def rm_fstab(name, device, config='/etc/fstab'):
     '''
     Remove the mount point from the fstab
 
@@ -297,22 +297,22 @@ def rm_fstab(name, device, config='/etc/fstab' ):
     '''
     modified = False
 
-    criteria = _fstab_entry( 
+    criteria = _fstab_entry(
         name = name, 
-        device = device )
+        device = device)
 
     lines = []
     try:
         with salt.utils.fopen(config, 'r') as ifile:
             for line in ifile:
                 try:
-                    if criteria.match( line ):
+                    if criteria.match(line):
                         modified = True
                     else:
-                        lines.append( line )
+                        lines.append(line)
 
                 except _fstab_entry.ParseError:
-                    lines.append( line )
+                    lines.append(line)
 
     except (IOError, OSError) as exc:
         msg = "Couldn't read from {0}: {1}"
@@ -358,34 +358,34 @@ def set_fstab(
         opts = ','.join(opts)
 
     # preserve arguments for updating
-    entry_args = dict( vars() )
-    map( entry_args.pop, [ 'test', 'config', 'match_on', 'kwargs' ] )
+    entry_args = dict(vars())
+    map(entry_args.pop, ['test', 'config', 'match_on', 'kwargs'])
 
     lines = []
     ret = None
 
     # Transform match_on into list--items will be checked later
-    if isinstance( match_on, list ):
+    if isinstance(match_on, list):
         pass
-    elif not isinstance( match_on, string_types ):
-        raise CommandExecutionError( 'match_on must be a string or list of strings' )
+    elif not isinstance(match_on, string_types):
+        raise CommandExecutionError('match_on must be a string or list of strings')
     elif match_on == 'auto':
         # Try to guess right criteria for auto....missing some special fstypes here
-        if fstype in frozenset([ 'tmpfs', 'sysfs', 'proc', 'fusectl', 'debugfs', 'securityfs', 'devtmpfs', 'cgroup']):
-            match_on = [ 'name' ]
+        if fstype in frozenset(['tmpfs', 'sysfs', 'proc', 'fusectl', 'debugfs', 'securityfs', 'devtmpfs', 'cgroup']):
+            match_on = ['name']
         else:
-            match_on = [ 'device' ]
+            match_on = ['device']
     else:
-        match_on = [ match_on ]
+        match_on = [match_on]
 
     # generate entry and criteria objects, handle invalid keys in match_on
-    entry = _fstab_entry( **entry_args )
+    entry = _fstab_entry(**entry_args)
     try:
-        criteria = entry.pick( match_on )
+        criteria = entry.pick(match_on)
 
     except KeyError:
-        invalid_keys = filter( lambda key: key not in _fstab_entry.fstab_keys, match_on )
-        raise CommandExecutionError( 'Unrecognized keys in match_on: "{0}"'.format( invalid_keys ) )
+        invalid_keys = filter(lambda key: key not in _fstab_entry.fstab_keys, match_on)
+        raise CommandExecutionError('Unrecognized keys in match_on: "{0}"'.format(invalid_keys))
 
     # parse file, use ret to cache status
     if not os.path.isfile(config):
@@ -395,19 +395,19 @@ def set_fstab(
         with salt.utils.fopen(config, 'r') as ifile:
             for line in ifile:
                 try:
-                    if criteria.match( line ):
+                    if criteria.match(line):
                         # Note: If ret isn't None here, we've matched multiple lines
                         ret = 'present'
-                        if entry.match( line ):
-                            lines.append( line )
+                        if entry.match(line):
+                            lines.append(line)
                         else:
                             ret = 'change'
-                            lines.append( str( entry ) )
+                            lines.append(str(entry))
                     else:
-                       lines.append( line )
+                       lines.append(line)
 
                 except _fstab_entry.ParseError:
-                    lines.append( line )                        
+                    lines.append(line)                        
 
     except (IOError, OSError) as exc:
         msg = 'Couldn\'t read from {0}: {1}'
@@ -416,10 +416,10 @@ def set_fstab(
 
     # add line if not present or changed
     if ret is None:
-        lines.append( str( entry ) )
+        lines.append(str(entry))
         ret = 'new'
 
-    if ret != 'present': # ret in [ 'new', 'change' ]:
+    if ret != 'present': # ret in ['new', 'change']:
         if not salt.utils.test_mode(test=test, **kwargs):
             try:
                 with salt.utils.fopen(config, 'w+') as ofile:
@@ -562,11 +562,11 @@ def set_automaster(
                         log.debug(
                             'auto_master entry for mount point {0} needs to be '
                             'updated'.format(name)
-                        )
+                       )
                         newline = (
                             '{0}\t{1}\t{2}\n'.format(
                                 name, type_opts, device_fmt)
-                        )
+                       )
                         lines.append(newline)
                 else:
                     lines.append(line)
@@ -596,7 +596,7 @@ def set_automaster(
                 newline = (
                     '{0}\t{1}\t{2}\n'.format(
                         name, type_opts, device_fmt)
-                )
+               )
                 lines.append(newline)
                 try:
                     with salt.utils.fopen(config, 'w+') as ofile:
@@ -606,8 +606,8 @@ def set_automaster(
                     raise CommandExecutionError(
                         'File not writable {0}'.format(
                             config
-                        )
-                    )
+                       )
+                   )
     return 'new'
 
 

From da75fa8ce06cf7d4afa9c6b6bc05abe74dde8334 Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Mon, 19 Jan 2015 17:35:54 -0500
Subject: [PATCH 08/11] PEP style compliance for changes to states/mount.py

---
 salt/states/mount.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/salt/states/mount.py b/salt/states/mount.py
index 8f0c414243..1bc287700c 100644
--- a/salt/states/mount.py
+++ b/salt/states/mount.py
@@ -93,7 +93,7 @@ def mounted(name,
         running as on the minion
 
     match_on
-        A name or list of fstab properties on which this state should be applied. 
+        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.

From f3eba17d43dc22736e4ea269208d1a0eadd7da59 Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Mon, 19 Jan 2015 18:14:37 -0500
Subject: [PATCH 09/11] More pylint fixes for changes to modules/mount.py

---
 salt/modules/mount.py | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index a89516ebce..6002909a5e 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -200,15 +200,15 @@ class _fstab_entry(object):
     class ParseError(ValueError):
         '''Error raised when a line isn't parsible as an fstab entry'''
 
-    fstab_keys = ['device', 'name', 'fstype', 'opts', 'dump', 'pass_num']
+    fstab_keys = ('device', 'name', 'fstype', 'opts', 'dump', 'pass_num')
 
     # preserve data format
-    compatibility_keys = ['device', 'name', 'fstype', 'opts', 'dump', 'pass']
+    compatibility_keys = ('device', 'name', 'fstype', 'opts', 'dump', 'pass')
 
     fstab_format = '{device}\t\t{name}\t{fstype}\t{opts}\t{dump} {pass_num}\n'
 
     @classmethod
-    def dict_from_line(cls, line, keys = fstab_keys):
+    def dict_from_line(cls, line, keys=fstab_keys):
         if len(keys) != 6:
             raise ValueError('Invalid key array: {0}'.format(keys))
         if line.startswith('#'):
@@ -277,7 +277,7 @@ def fstab(config='/etc/fstab'):
         for line in ifile:
             try:
                 entry = _fstab_entry.dict_from_line(line, _fstab_entry.compatibility_keys)
-		entry['opts'] = entry['opts'].split(',')
+                entry['opts'] = entry['opts'].split(',')
                 ret[entry.pop('name')] = entry
             except _fstab_entry.ParseError:
                 pass

From 5c5d123bc85fdd12509272bae4bcdc063d3de450 Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Mon, 19 Jan 2015 19:08:28 -0500
Subject: [PATCH 10/11] Another pass over modules/mount.py against pylint....

---
 salt/modules/mount.py | 47 +++++++++++++++++++++++++++++--------------
 1 file changed, 32 insertions(+), 15 deletions(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index 6002909a5e..56dabd401f 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -238,7 +238,8 @@ class _fstab_entry(object):
 
     def pick(self, keys):
         '''returns an instance with just those keys'''
-        return self.__class__(**dict(map(lambda key: (key, self.criteria[key]), keys)))
+        subset = dict(map(lambda key: (key, self.criteria[key]), keys))
+        return self.__class__(**subset)
 
     def __init__(self, **criteria):
         '''Store non-empty, non-null values to use as filter'''
@@ -276,7 +277,10 @@ def fstab(config='/etc/fstab'):
     with salt.utils.fopen(config) as ifile:
         for line in ifile:
             try:
-                entry = _fstab_entry.dict_from_line(line, _fstab_entry.compatibility_keys)
+                entry = _fstab_entry.dict_from_line(
+                    line,
+                    _fstab_entry.compatibility_keys)
+
                 entry['opts'] = entry['opts'].split(',')
                 ret[entry.pop('name')] = entry
             except _fstab_entry.ParseError:
@@ -297,9 +301,7 @@ def rm_fstab(name, device, config='/etc/fstab'):
     '''
     modified = False
 
-    criteria = _fstab_entry(
-        name = name, 
-        device = device)
+    criteria = _fstab_entry(name=name, device=device)
 
     lines = []
     try:
@@ -340,7 +342,7 @@ def set_fstab(
         pass_num=0,
         config='/etc/fstab',
         test=False,
-        match_on = 'auto',
+        match_on='auto',
         **kwargs):
     '''
     Verify that this mount is represented in the fstab, change the mount
@@ -368,10 +370,22 @@ def set_fstab(
     if isinstance(match_on, list):
         pass
     elif not isinstance(match_on, string_types):
-        raise CommandExecutionError('match_on must be a string or list of strings')
+        msg = 'match_on must be a string or list of strings'
+        raise CommandExecutionError(msg)
     elif match_on == 'auto':
-        # Try to guess right criteria for auto....missing some special fstypes here
-        if fstype in frozenset(['tmpfs', 'sysfs', 'proc', 'fusectl', 'debugfs', 'securityfs', 'devtmpfs', 'cgroup']):
+        # Try to guess right criteria for auto....
+        # NOTE: missing some special fstypes here
+        specialFSes = frozenset([
+            'tmpfs',
+            'sysfs',
+            'proc',
+            'fusectl',
+            'debugfs',
+            'securityfs',
+            'devtmpfs',
+            'cgroup'])
+
+        if fstype in specialFSes:
             match_on = ['name']
         else:
             match_on = ['device']
@@ -384,8 +398,11 @@ def set_fstab(
         criteria = entry.pick(match_on)
 
     except KeyError:
-        invalid_keys = filter(lambda key: key not in _fstab_entry.fstab_keys, match_on)
-        raise CommandExecutionError('Unrecognized keys in match_on: "{0}"'.format(invalid_keys))
+        filterFn = lambda key: key not in _fstab_entry.fstab_keys
+        invalid_keys = filter(filterFn, match_on)
+
+        msg = 'Unrecognized keys in match_on: "{0}"'.format(invalid_keys)
+        raise CommandExecutionError(msg)
 
     # parse file, use ret to cache status
     if not os.path.isfile(config):
@@ -396,7 +413,8 @@ def set_fstab(
             for line in ifile:
                 try:
                     if criteria.match(line):
-                        # Note: If ret isn't None here, we've matched multiple lines
+                        # Note: If ret isn't None here, 
+                        # we've matched multiple lines
                         ret = 'present'
                         if entry.match(line):
                             lines.append(line)
@@ -404,16 +422,15 @@ def set_fstab(
                             ret = 'change'
                             lines.append(str(entry))
                     else:
-                       lines.append(line)
+                        lines.append(line)
 
                 except _fstab_entry.ParseError:
-                    lines.append(line)                        
+                    lines.append(line)
 
     except (IOError, OSError) as exc:
         msg = 'Couldn\'t read from {0}: {1}'
         raise CommandExecutionError(msg.format(config, str(exc)))
 
-
     # add line if not present or changed
     if ret is None:
         lines.append(str(entry))

From 203b700032ee1917292b329aa91ca2d72874a2ea Mon Sep 17 00:00:00 2001
From: Alexander Haase <alex@alexanderhaase.net>
Date: Mon, 19 Jan 2015 20:04:48 -0500
Subject: [PATCH 11/11] Another pass over modules/mount.py against pylint....

---
 salt/modules/mount.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/salt/modules/mount.py b/salt/modules/mount.py
index 56dabd401f..012b5cb0bb 100644
--- a/salt/modules/mount.py
+++ b/salt/modules/mount.py
@@ -413,7 +413,7 @@ def set_fstab(
             for line in ifile:
                 try:
                     if criteria.match(line):
-                        # Note: If ret isn't None here, 
+                        # Note: If ret isn't None here,
                         # we've matched multiple lines
                         ret = 'present'
                         if entry.match(line):
@@ -436,7 +436,7 @@ def set_fstab(
         lines.append(str(entry))
         ret = 'new'
 
-    if ret != 'present': # ret in ['new', 'change']:
+    if ret != 'present':  # ret in ['new', 'change']:
         if not salt.utils.test_mode(test=test, **kwargs):
             try:
                 with salt.utils.fopen(config, 'w+') as ofile: