Merge lp:~niedbalski/charms/precise/swift-storage/add-fstab-persist into lp:~openstack-charmers-archive/charms/trusty/swift-storage/next

[James Page]

- Updated charm-helpers with fstab persist support
- Modified mount command for specify "xfs" filesystem type

[James Page (james-page)]

Hi Jorge

This all looks really good - I have it a test and the one comment I have is that its possible to add multiple entries for a single block device to /etc/fstab.

This happened in my testing as I use the ephemeral storage that cloud-init configures mounted on /mnt as storage for swift - linux deals with the double mounting OK - but it looks odd to a user.

[James Page (james-page)]

/dev/vda1 on / type ext4 (rw)
...
/dev/vdb on /mnt type xfs (rw)
/dev/vdb on /srv/node/vdb type xfs (rw)

[James Page (james-page)]

It might be worth checking and removing any existing mounted block device before adding an explicit mapping.

[Jorge Niedbalski (niedbalski)]

Hey james-page,

I tried to reproduce your comment adding the same block-device twice, with no success:

In [8]: Fstab.add('/dev/sdaf', '/mnt/sdaf', 'xfs', None)
Out[8]: <charmhelpers.core.fstab.Entry at 0x17abcd0>

In [9]: Fstab.add('/dev/sdaf', '/mnt/sdaf', 'xfs', None)
Out[9]: False

Does is that initial /dev/vdb on fstab.d/ directory?

[lp:~niedbalski/charms/precise/swift-storage/add-fstab-persist]

30. By Jorge Niedbalski on 2014-06-02

Re-imported from ~charm-helpers, added support for UUID,LABEL entries

31. By Jorge Niedbalski on 2014-06-04

Re-imported charm-helpers from latest merged version

[Jorge Niedbalski (niedbalski)]

Updated with latest merged revision.

=== added file 'hooks/charmhelpers/core/fstab.py'
--- hooks/charmhelpers/core/fstab.py	1970-01-01 00:00:00 +0000
+++ hooks/charmhelpers/core/fstab.py	2014-06-04 19:11:16 +0000
@@ -0,0 +1,114 @@
+#!/usr/bin/env python
+# -*- coding: utf-8 -*-
+
+__author__ = 'Jorge Niedbalski R. <[e-mail]>'
+
+import os
+
+
+class Fstab(file):
+    """This class extends file in order to implement a file reader/writer
+    for file `/etc/fstab`
+    """
+
+    class Entry(object):
+        """Entry class represents a non-comment line on the `/etc/fstab` file
+        """
+        def __init__(self, device, mountpoint, filesystem,
+                     options, d=0, p=0):
+            self.device = device
+            self.mountpoint = mountpoint
+            self.filesystem = filesystem
+
+            if not options:
+                options = "defaults"
+
+            self.options = options
+            self.d = d
+            self.p = p
+
+        def __eq__(self, o):
+            return str(self) == str(o)
+
+        def __str__(self):
+            return "{} {} {} {} {} {}".format(self.device,
+                                              self.mountpoint,
+                                              self.filesystem,
+                                              self.options,
+                                              self.d,
+                                              self.p)
+
+    DEFAULT_PATH = os.path.join(os.path.sep, 'etc', 'fstab')
+
+    def __init__(self, path=None):
+        if path:
+            self._path = path
+        else:
+            self._path = self.DEFAULT_PATH
+        file.__init__(self, self._path, 'r+')
+
+    def _hydrate_entry(self, line):
+        return Fstab.Entry(*filter(
+            lambda x: x not in ('', None),
+            line.strip("\n").split(" ")))
+
+    @property
+    def entries(self):
+        self.seek(0)
+        for line in self.readlines():
+            try:
+                if not line.startswith("#"):
+                    yield self._hydrate_entry(line)
+            except ValueError:
+                pass
+
+    def get_entry_by_attr(self, attr, value):
+        for entry in self.entries:
+            e_attr = getattr(entry, attr)
+            if e_attr == value:
+                return entry
+        return None
+
+    def add_entry(self, entry):
+        if self.get_entry_by_attr('device', entry.device):
+            return False
+
+        self.write(str(entry) + '\n')
+        self.truncate()
+        return entry
+
+    def remove_entry(self, entry):
+        self.seek(0)
+
+        lines = self.readlines()
+
+        found = False
+        for index, line in enumerate(lines):
+            if not line.startswith("#"):
+                if self._hydrate_entry(line) == entry:
+                    found = True
+                    break
+
+        if not found:
+            return False
+
+        lines.remove(line)
+
+        self.seek(0)
+        self.write(''.join(lines))
+        self.truncate()
+        return True
+
+    @classmethod
+    def remove_by_mountpoint(cls, mountpoint, path=None):
+        fstab = cls(path=path)
+        entry = fstab.get_entry_by_attr('mountpoint', mountpoint)
+        if entry:
+            return fstab.remove_entry(entry)
+        return False
+
+    @classmethod
+    def add(cls, device, mountpoint, filesystem, options=None, path=None):
+        return cls(path=path).add_entry(Fstab.Entry(device,
+                                                    mountpoint, filesystem,
+                                                    options=options))

=== modified file 'hooks/charmhelpers/core/host.py'
--- hooks/charmhelpers/core/host.py	2014-05-19 11:41:35 +0000
+++ hooks/charmhelpers/core/host.py	2014-06-04 19:11:16 +0000
@@ -17,6 +17,7 @@
 from collections import OrderedDict
 
 from hookenv import log
+from fstab import Fstab
 
 
 def service_start(service_name):
@@ -35,7 +36,8 @@
 
 
 def service_reload(service_name, restart_on_failure=False):
-    """Reload a system service, optionally falling back to restart if reload fails"""
+    """Reload a system service, optionally falling back to restart if
+    reload fails"""
     service_result = service('reload', service_name)
     if not service_result and restart_on_failure:
         service_result = service('restart', service_name)
@@ -144,7 +146,19 @@
         target.write(content)
 
 
-def mount(device, mountpoint, options=None, persist=False):
+def fstab_remove(mp):
+    """Remove the given mountpoint entry from /etc/fstab
+    """
+    return Fstab.remove_by_mountpoint(mp)
+
+
+def fstab_add(dev, mp, fs, options=None):
+    """Adds the given device entry to the /etc/fstab file
+    """
+    return Fstab.add(dev, mp, fs, options=options)
+
+
+def mount(device, mountpoint, options=None, persist=False, filesystem="ext3"):
     """Mount a filesystem at a particular mountpoint"""
     cmd_args = ['mount']
     if options is not None:
@@ -155,9 +169,9 @@
     except subprocess.CalledProcessError, e:
         log('Error mounting {} at {}\n{}'.format(device, mountpoint, e.output))
         return False
+
     if persist:
-        # TODO: update fstab
-        pass
+        return fstab_add(device, mountpoint, filesystem, options=options)
     return True
 
 
@@ -169,9 +183,9 @@
     except subprocess.CalledProcessError, e:
         log('Error unmounting {}\n{}'.format(mountpoint, e.output))
         return False
+
     if persist:
-        # TODO: update fstab
-        pass
+        return fstab_remove(mountpoint)
     return True
 
 

=== modified file 'hooks/swift_storage_utils.py'
--- hooks/swift_storage_utils.py	2014-04-07 14:50:34 +0000
+++ hooks/swift_storage_utils.py	2014-06-04 19:11:16 +0000
@@ -181,7 +181,8 @@
         _dev = os.path.basename(dev)
         _mp = os.path.join('/srv', 'node', _dev)
         mkdir(_mp, owner='swift', group='swift')
-        mount(dev, '/srv/node/%s' % _dev, persist=True)
+        mount(dev, '/srv/node/%s' % _dev, persist=True,
+              filesystem="xfs")
     check_call(['chown', '-R', 'swift:swift', '/srv/node/'])
     check_call(['chmod', '-R', '0750', '/srv/node/'])