Merge pull request juju#9599 from wallyworld/storage-attach-fstab

juju#9599 ## Description of change When a block device is attached for storage, create an /etc/fstab entry. Delete the /etc/fstab entry when storage is detached. ## QA steps bootstrap aws deploy postgresql with storage ssh into the postgresql machine and check that there's an fstab entry matching the mtab entry detach the storage ensure that the fstab entry is removed ## Bug reference https://bugs.launchpad.net/juju/+bug/1783419
bnhassin · Jan 11, 2019 · fd9a95a · fd9a95a
1 parent 8d39b83
commit fd9a95a
Show file tree

Hide file tree

Showing 8 changed files with 218 additions and 19 deletions.
diff --git a/storage/provider/common_test.go b/storage/provider/common_test.go
@@ -4,6 +4,8 @@
 package provider_test
 
 import (
+	"io/ioutil"
+	"os"
 	"path/filepath"
 
 	jc "github.com/juju/testing/checkers"
@@ -39,9 +41,13 @@ func (s *providerCommonSuite) TestCommonProvidersExported(c *gc.C) {
 
 // testDetachFilesystems is a test-case for detaching filesystems that use
 // the common "maybeUnmount" method.
-func testDetachFilesystems(c *gc.C, commands *mockRunCommand, source storage.FilesystemSource, callCtx context.ProviderCallContext, mounted bool) {
-	const testMountPoint = "/in/the/place"
-
+func testDetachFilesystems(
+	c *gc.C, commands *mockRunCommand,
+	source storage.FilesystemSource,
+	callCtx context.ProviderCallContext,
+	mounted bool,
+	etcDir, fstab string,
+) {
 	cmd := commands.expect("df", "--output=source", filepath.Dir(testMountPoint))
 	cmd.respond("headers\n/same/as/rootfs", nil)
 	cmd = commands.expect("df", "--output=source", testMountPoint)
@@ -64,4 +70,12 @@ func testDetachFilesystems(c *gc.C, commands *mockRunCommand, source storage.Fil
 	c.Assert(err, jc.ErrorIsNil)
 	c.Assert(results, gc.HasLen, 1)
 	c.Assert(results[0], jc.ErrorIsNil)
+
+	data, err := ioutil.ReadFile(filepath.Join(etcDir, "fstab"))
+	if os.IsNotExist(err) {
+		c.Assert(fstab, gc.Equals, "")
+		return
+	}
+	c.Assert(err, jc.ErrorIsNil)
+	c.Assert(string(data), gc.Equals, fstab)
 }
diff --git a/storage/provider/dirfuncs.go b/storage/provider/dirfuncs.go
@@ -15,6 +15,7 @@ import (
 // dirFuncs is used to allow the real directory operations to
 // be stubbed out for testing.
 type dirFuncs interface {
+	etcDir() string
 	mkDirAll(path string, perm os.FileMode) error
 	lstat(path string) (fi os.FileInfo, err error)
 	fileCount(path string) (int, error)
@@ -41,6 +42,10 @@ type osDirFuncs struct {
 	run runCommandFunc
 }
 
+func (*osDirFuncs) etcDir() string {
+	return "/etc"
+}
+
 func (*osDirFuncs) mkDirAll(path string, perm os.FileMode) error {
 	return os.MkdirAll(path, perm)
 }

diff --git a/storage/provider/export_test.go b/storage/provider/export_test.go
@@ -17,11 +17,13 @@ import (
 var Getpagesize = &getpagesize
 
 func LoopVolumeSource(
+	etcDir string,
 	storageDir string,
 	run func(string, ...string) (string, error),
 ) (storage.VolumeSource, *MockDirFuncs) {
 	dirFuncs := &MockDirFuncs{
 		osDirFuncs{run},
+		etcDir,
 		set.NewStrings(),
 	}
 	return &loopVolumeSource{dirFuncs, run, storageDir}, dirFuncs
@@ -34,12 +36,14 @@ func LoopProvider(
 }
 
 func NewMockManagedFilesystemSource(
+	etcDir string,
 	run func(string, ...string) (string, error),
 	volumeBlockDevices map[names.VolumeTag]storage.BlockDevice,
 	filesystems map[names.FilesystemTag]storage.Filesystem,
 ) (storage.FilesystemSource, *MockDirFuncs) {
 	dirFuncs := &MockDirFuncs{
 		osDirFuncs{run},
+		etcDir,
 		set.NewStrings(),
 	}
 	return &managedFilesystemSource{
@@ -53,7 +57,12 @@ var _ dirFuncs = (*MockDirFuncs)(nil)
 // MockDirFuncs stub out the real mkdir and lstat functions from stdlib.
 type MockDirFuncs struct {
 	osDirFuncs
-	Dirs set.Strings
+	fakeEtcDir string
+	Dirs       set.Strings
+}
+
+func (m *MockDirFuncs) etcDir() string {
+	return m.fakeEtcDir
 }
 
 func (m *MockDirFuncs) mkDirAll(path string, perm os.FileMode) error {
@@ -102,9 +111,10 @@ func (m *MockDirFuncs) fileCount(name string) (int, error) {
 	return 0, nil
 }
 
-func RootfsFilesystemSource(storageDir string, run func(string, ...string) (string, error)) (storage.FilesystemSource, *MockDirFuncs) {
+func RootfsFilesystemSource(etcDir, storageDir string, run func(string, ...string) (string, error)) (storage.FilesystemSource, *MockDirFuncs) {
 	d := &MockDirFuncs{
 		osDirFuncs{run},
+		etcDir,
 		set.NewStrings(),
 	}
 	return &rootfsFilesystemSource{d, run, storageDir}, d
@@ -114,10 +124,11 @@ func RootfsProvider(run func(string, ...string) (string, error)) storage.Provide
 	return &rootfsProvider{run}
 }
 
-func TmpfsFilesystemSource(storageDir string, run func(string, ...string) (string, error)) storage.FilesystemSource {
+func TmpfsFilesystemSource(etcDir, storageDir string, run func(string, ...string) (string, error)) storage.FilesystemSource {
 	return &tmpfsFilesystemSource{
 		&MockDirFuncs{
 			osDirFuncs{run},
+			etcDir,
 			set.NewStrings(),
 		},
 		run,

diff --git a/storage/provider/loop_test.go b/storage/provider/loop_test.go
@@ -85,6 +85,7 @@ func (s *loopSuite) TestScope(c *gc.C) {
 func (s *loopSuite) loopVolumeSource(c *gc.C) (storage.VolumeSource, *provider.MockDirFuncs) {
 	s.commands = &mockRunCommand{c: c}
 	return provider.LoopVolumeSource(
+		c.MkDir(),
 		s.storageDir,
 		s.commands.run,
 	)

diff --git a/storage/provider/managedfs.go b/storage/provider/managedfs.go
@@ -4,8 +4,12 @@
 package provider
 
 import (
+	"bufio"
+	"io/ioutil"
+	"os"
 	"path"
 	"path/filepath"
+	"strings"
 	"unicode"
 
 	"github.com/juju/errors"
@@ -225,6 +229,73 @@ func mountFilesystem(run runCommandFunc, dirFuncs dirFuncs, devicePath, mountPoi
 		return errors.Annotate(err, "mount failed")
 	}
 	logger.Infof("mounted filesystem on %q at %q", devicePath, mountPoint)
+
+	// Look for the mtab entry resulting from the mount and copy it to fstab.
+	// This ensures the mount is available available after a reboot.
+	etcDir := dirFuncs.etcDir()
+	mtabEntry, err := extractMtabEntry(etcDir, devicePath, mountPoint)
+	if err != nil {
+		return errors.Annotate(err, "parsing /etc/mtab")
+	}
+	if mtabEntry == "" {
+		return nil
+	}
+	return addFstabEntry(etcDir, devicePath, mountPoint, mtabEntry)
+}
+
+// extractMtabEntry returns any /etc/mtab entry for the specified
+// device path and mount point, or "" if none exists.
+func extractMtabEntry(etcDir string, devicePath, mountPoint string) (string, error) {
+	f, err := os.Open(filepath.Join(etcDir, "mtab"))
+	if os.IsNotExist(err) {
+		return "", nil
+	}
+	if err != nil {
+		return "", errors.Trace(err)
+	}
+	defer f.Close()
+	scanner := bufio.NewScanner(f)
+
+	for scanner.Scan() {
+		line := scanner.Text()
+		fields := strings.Fields(line)
+		if len(fields) >= 2 && fields[0] == devicePath && fields[1] == mountPoint {
+			return line, nil
+		}
+	}
+
+	if err := scanner.Err(); err != nil {
+		return "", errors.Trace(err)
+	}
+	return "", nil
+}
+
+// addFstabEntry creates an entry in /etc/fstab for the specified
+// device path and mount point so long as there's no existing entry already.
+func addFstabEntry(etcDir string, devicePath, mountPoint, entry string) error {
+	f, err := os.OpenFile(filepath.Join(etcDir, "fstab"), os.O_CREATE|os.O_APPEND|os.O_RDWR, 0644)
+	if err != nil {
+		return errors.Annotate(err, "opening /etc/fstab")
+	}
+	defer f.Close()
+
+	// Ensure there's no entry there already
+	scanner := bufio.NewScanner(f)
+	for scanner.Scan() {
+		line := scanner.Text()
+		fields := strings.Fields(line)
+		if len(fields) >= 2 && fields[0] == devicePath && fields[1] == mountPoint {
+			return nil
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		return errors.Trace(err)
+	}
+
+	// The entry will be written at the end of the fstab file.
+	if _, err = f.WriteString("\n" + entry + "\n"); err != nil {
+		return errors.Annotate(err, "writing /etc/fstab")
+	}
 	return nil
 }
 
@@ -237,13 +308,60 @@ func maybeUnmount(run runCommandFunc, dirFuncs dirFuncs, mountPoint string) erro
 		return nil
 	}
 	logger.Debugf("attempting to unmount filesystem at %q", mountPoint)
+	if err := removeFstabEntry(dirFuncs.etcDir(), mountPoint); err != nil {
+		return errors.Annotate(err, "updating /etc/fstab failed")
+	}
 	if _, err := run("umount", mountPoint); err != nil {
 		return errors.Annotate(err, "umount failed")
 	}
 	logger.Infof("unmounted filesystem at %q", mountPoint)
 	return nil
 }
 
+// removeFstabEntry removes any existing /etc/fstab entry for
+// the specified mount point.
+func removeFstabEntry(etcDir string, mountPoint string) error {
+	fstab := filepath.Join(etcDir, "fstab")
+	f, err := os.Open(fstab)
+	if os.IsNotExist(err) {
+		return nil
+	}
+	if err != nil {
+		return errors.Trace(err)
+	}
+	defer f.Close()
+	scanner := bufio.NewScanner(f)
+
+	// Use a tempfile in /etc and rename when done.
+	newFsTab, err := ioutil.TempFile(etcDir, "juju-fstab-")
+	if err != nil {
+		return errors.Trace(err)
+	}
+	defer func() {
+		newFsTab.Close()
+		os.Remove(newFsTab.Name())
+	}()
+	if err := os.Chmod(newFsTab.Name(), 0644); err != nil {
+		return errors.Trace(err)
+	}
+
+	for scanner.Scan() {
+		line := scanner.Text()
+		fields := strings.Fields(line)
+		if len(fields) < 2 || fields[1] != mountPoint {
+			_, err := newFsTab.WriteString(line + "\n")
+			if err != nil {
+				return errors.Trace(err)
+			}
+		}
+	}
+	if err := scanner.Err(); err != nil {
+		return errors.Trace(err)
+	}
+
+	return os.Rename(newFsTab.Name(), fstab)
+}
+
 func isMounted(dirFuncs dirFuncs, mountPoint string) (bool, string, error) {
 	mountPointParent := filepath.Dir(mountPoint)
 	parentSource, err := dirFuncs.mountPointSource(mountPointParent)