Merge lp:~fgimenez/snappy/wait-for-idle-partition into lp:~snappy-dev/snappy/snappy-moved-to-github

[Federico Gimenez]

Commands to wait for partition idle before trying to remount

Commands to wait for partition idle before trying to remount

workaround for bug #1496665

[lp:~fgimenez/snappy/wait-for-idle-partition]

727. By Federico Gimenez on 2015-10-02

typo

[Leo Arias (elopio)]

all tests pass, and looks good. Thanks.

review: Approve

=== modified file '_integration-tests/testutils/cli/cli.go'
--- _integration-tests/testutils/cli/cli.go	2015-10-01 08:21:24 +0000
+++ _integration-tests/testutils/cli/cli.go	2015-10-02 13:11:04 +0000
@@ -33,19 +33,15 @@
 // ExecCommand executes a shell command and returns a string with the output
 // of the command. In case of error, it will fail the test.
 func ExecCommand(c *check.C, cmds ...string) string {
-	fmt.Println(strings.Join(cmds, " "))
-	cmd := execCommand(cmds[0], cmds[1:len(cmds)]...)
-	output, err := cmd.CombinedOutput()
-	stringOutput := string(output)
-	fmt.Print(stringOutput)
-	c.Assert(err, check.IsNil, check.Commentf("Error: %v", stringOutput))
-	return stringOutput
+	output, err := ExecCommandErr(cmds...)
+	c.Assert(err, check.IsNil, check.Commentf("Error: %v", output))
+	return output
 }
 
 // ExecCommandToFile executes a shell command and saves the output of the
 // command to a file. In case of error, it will fail the test.
 func ExecCommandToFile(c *check.C, filename string, cmds ...string) {
-	cmd := execCommand(cmds[0], cmds[1:len(cmds)]...)
+	cmd := execCommand(cmds[0], cmds[1:]...)
 	outfile, err := os.Create(filename)
 	c.Assert(err, check.IsNil, check.Commentf("Error creating output file %s", filename))
 
@@ -55,3 +51,14 @@
 	err = cmd.Run()
 	c.Assert(err, check.IsNil, check.Commentf("Error executing command '%v': %v", cmds, err))
 }
+
+// ExecCommandErr executes a shell command and returns a string with the output
+// of the command and eventually the obtained error
+func ExecCommandErr(cmds ...string) (output string, err error) {
+	fmt.Println(strings.Join(cmds, " "))
+	cmd := execCommand(cmds[0], cmds[1:]...)
+	outputByte, err := cmd.CombinedOutput()
+	output = string(outputByte)
+	fmt.Print(output)
+	return
+}

=== modified file '_integration-tests/testutils/cli/cli_test.go'
--- _integration-tests/testutils/cli/cli_test.go	2015-10-01 08:21:24 +0000
+++ _integration-tests/testutils/cli/cli_test.go	2015-10-02 13:11:04 +0000
@@ -36,6 +36,7 @@
 
 type cliTestSuite struct {
 	backExecCommand func(string, ...string) *exec.Cmd
+	helperProcess   string
 }
 
 var _ = check.Suite(&cliTestSuite{})
@@ -49,8 +50,12 @@
 	execCommand = s.backExecCommand
 }
 
+func (s *cliTestSuite) SetUpTest(c *check.C) {
+	s.helperProcess = "TestHelperProcess"
+}
+
 func (s *cliTestSuite) fakeExecCommand(command string, args ...string) *exec.Cmd {
-	cs := []string{"-check.f=cliTestSuite.TestHelperProcess", "--", command}
+	cs := []string{"-check.f=cliTestSuite." + s.helperProcess, "--", command}
 	cs = append(cs, args...)
 	cmd := exec.Command(os.Args[0], cs...)
 	cmd.Env = []string{"GO_WANT_HELPER_PROCESS=1"}
@@ -58,11 +63,19 @@
 }
 
 func (s *cliTestSuite) TestHelperProcess(c *check.C) {
+	baseHelperProcess(0)
+}
+
+func (s *cliTestSuite) TestHelperProcessErr(c *check.C) {
+	baseHelperProcess(1)
+}
+
+func baseHelperProcess(exitValue int) {
 	if os.Getenv("GO_WANT_HELPER_PROCESS") != "1" {
 		return
 	}
 	fmt.Fprintf(os.Stdout, execOutput)
-	os.Exit(0)
+	os.Exit(exitValue)
 }
 
 func (s *cliTestSuite) TestExecCommand(c *check.C) {
@@ -83,3 +96,18 @@
 	c.Assert(err, check.IsNil)
 	c.Assert(string(actualFileContents), check.Equals, execOutput)
 }
+
+func (s *cliTestSuite) TestExecCommandErr(c *check.C) {
+	actualOutput, err := ExecCommandErr("mycmd")
+
+	c.Assert(actualOutput, check.Equals, execOutput)
+	c.Assert(err, check.IsNil)
+}
+
+func (s *cliTestSuite) TestExecCommandErrWithError(c *check.C) {
+	s.helperProcess = "TestHelperProcessErr"
+	actualOutput, err := ExecCommandErr("mycmd")
+
+	c.Assert(actualOutput, check.Equals, execOutput)
+	c.Assert(err, check.NotNil)
+}

=== modified file '_integration-tests/testutils/partition/partition.go'
--- _integration-tests/testutils/partition/partition.go	2015-10-01 09:37:04 +0000
+++ _integration-tests/testutils/partition/partition.go	2015-10-02 13:11:04 +0000
@@ -20,19 +20,70 @@
 package partition
 
 import (
+	"bufio"
+	"fmt"
+	"regexp"
+	"strings"
+
 	"gopkg.in/check.v1"
 
 	"launchpad.net/snappy/_integration-tests/testutils/cli"
-)
-
-var execCommand = cli.ExecCommand
+	"launchpad.net/snappy/_integration-tests/testutils/wait"
+)
+
+const (
+	// beginning of the string returned by lsof -V when the partition
+	// is not being used, in this case the exit status of lsof is 1
+	lsofNotUsed = "lsof: no file system use located"
+	// custom pattern to be returned by the check functions
+	lsofNotBeingWritten = "volume-is-idle"
+)
+
+var (
+	execCommand     = cli.ExecCommandErr
+	waitForFunction = wait.ForFunction
+)
 
 // MakeWritable remounts a path with read and write permissions.
-func MakeWritable(c *check.C, path string) {
-	execCommand(c, "sudo", "mount", "-o", "remount,rw", path)
+func MakeWritable(c *check.C, path string) (err error) {
+	return commonMount(c, path, "remount,rw")
 }
 
 // MakeReadonly remounts a path with only read permissions.
-func MakeReadonly(c *check.C, path string) {
-	execCommand(c, "sudo", "mount", "-o", "remount,ro", path)
+func MakeReadonly(c *check.C, path string) (err error) {
+	return commonMount(c, path, "remount,ro")
+}
+
+func commonMount(c *check.C, path, mountOption string) (err error) {
+	err = waitForFunction(c, lsofNotBeingWritten, checkPathBusyFunc(path))
+
+	if err != nil {
+		return
+	}
+
+	execCommand("sudo", "mount", "-o", mountOption, path)
+	return
+}
+
+func checkPathBusyFunc(path string) func() (string, error) {
+	return func() (result string, err error) {
+		// lsof exit status is 1 for unused partitions
+		var info string
+		if info, err = execCommand("lsof", "-V", path); err != nil {
+			// check if the output matches lsofNotUsed
+			if !strings.HasPrefix(info, lsofNotUsed) {
+				return info, err
+			}
+		}
+		reader := strings.NewReader(info)
+		scanner := bufio.NewScanner(reader)
+		for scanner.Scan() {
+			fields := strings.Fields(scanner.Text())
+			if match, _ := regexp.MatchString("^[0-9]+w$", fields[3]); match {
+				fmt.Printf("match! %s", fields[3])
+				return fields[3], nil
+			}
+		}
+		return lsofNotBeingWritten, nil
+	}
 }

=== modified file '_integration-tests/testutils/partition/partition_test.go'
--- _integration-tests/testutils/partition/partition_test.go	2015-10-01 09:59:40 +0000
+++ _integration-tests/testutils/partition/partition_test.go	2015-10-02 13:11:04 +0000
@@ -20,50 +20,151 @@
 package partition
 
 import (
+	"fmt"
 	"strings"
 
 	"gopkg.in/check.v1"
 )
 
-const path = "mypath"
+const (
+	path        = "mypath"
+	writableCmd = "sudo mount -o remount,rw " + path
+	readonlyCmd = "sudo mount -o remount,ro " + path
+	waitCmd     = lsofNotBeingWritten
+)
 
 type partitionTestSuite struct {
-	execCalls       map[string]int
-	backExecCommand func(*check.C, ...string) string
+	execCalls           map[string]int
+	waitCalls           map[string]int
+	execOutput          string
+	execError           bool
+	backExecCommand     func(...string) (string, error)
+	backWaitForFunction func(*check.C, string, func() (string, error)) error
+	waitError           bool
 }
 
 var _ = check.Suite(&partitionTestSuite{})
 
 func (s *partitionTestSuite) SetUpSuite(c *check.C) {
 	s.backExecCommand = execCommand
+	s.backWaitForFunction = waitForFunction
 	execCommand = s.fakeExecCommand
+	waitForFunction = s.fakeWaitForFunction
 }
 
 func (s *partitionTestSuite) TearDownSuite(c *check.C) {
 	execCommand = s.backExecCommand
+	waitForFunction = s.backWaitForFunction
 }
 
 func (s *partitionTestSuite) SetUpTest(c *check.C) {
 	s.execCalls = make(map[string]int)
+	s.waitCalls = make(map[string]int)
+	s.waitError = false
+	s.execOutput = ""
+	s.execError = false
 }
 
-func (s *partitionTestSuite) fakeExecCommand(c *check.C, args ...string) (output string) {
+func (s *partitionTestSuite) fakeExecCommand(args ...string) (output string, err error) {
 	s.execCalls[strings.Join(args, " ")]++
+
+	if s.execError {
+		err = fmt.Errorf("Exec error!")
+	}
+
+	return s.execOutput, err
+}
+
+func (s *partitionTestSuite) fakeWaitForFunction(c *check.C, pattern string, f func() (string, error)) (err error) {
+	s.waitCalls[pattern]++
+
+	if s.waitError {
+		err = fmt.Errorf("Wait error!")
+	}
+
 	return
 }
 
-func (s *partitionTestSuite) TestMakeWritable(c *check.C) {
-	cmd := "sudo mount -o remount,rw " + path
-
-	MakeWritable(c, path)
-
-	c.Assert(s.execCalls[cmd], check.Equals, 1)
-}
-
-func (s *partitionTestSuite) TestMakeReadOnly(c *check.C) {
-	cmd := "sudo mount -o remount,ro " + path
-
-	MakeReadonly(c, path)
-
-	c.Assert(s.execCalls[cmd], check.Equals, 1)
+func (s *partitionTestSuite) TestMakeWritableCallsExecCommand(c *check.C) {
+	err := MakeWritable(c, path)
+
+	c.Assert(err, check.IsNil)
+	c.Assert(s.execCalls[writableCmd], check.Equals, 1)
+}
+
+func (s *partitionTestSuite) TestMakeWritableWaitsForIdlePartition(c *check.C) {
+	err := MakeWritable(c, path)
+
+	c.Assert(err, check.IsNil)
+	c.Assert(s.waitCalls[waitCmd], check.Equals, 1)
+}
+
+func (s *partitionTestSuite) TestMakeWritableReturnsWaitError(c *check.C) {
+	s.waitError = true
+	err := MakeWritable(c, path)
+
+	c.Assert(err, check.NotNil)
+	c.Assert(s.waitCalls[waitCmd], check.Equals, 1)
+	c.Assert(s.execCalls[writableCmd], check.Equals, 0)
+}
+
+func (s *partitionTestSuite) TestMakeReadOnlyCallsExecCommand(c *check.C) {
+	err := MakeReadonly(c, path)
+
+	c.Assert(err, check.IsNil)
+	c.Assert(s.execCalls[readonlyCmd], check.Equals, 1)
+}
+
+func (s *partitionTestSuite) TestMakeReadonlyWaitsForIdlePartition(c *check.C) {
+	err := MakeReadonly(c, path)
+
+	c.Assert(err, check.IsNil)
+	c.Assert(s.waitCalls[waitCmd], check.Equals, 1)
+}
+
+func (s *partitionTestSuite) TestMakeReadonlyReturnsWaitError(c *check.C) {
+	s.waitError = true
+	err := MakeReadonly(c, path)
+
+	c.Assert(err, check.NotNil)
+	c.Assert(s.waitCalls[waitCmd], check.Equals, 1)
+	c.Assert(s.execCalls[readonlyCmd], check.Equals, 0)
+}
+
+func (s *partitionTestSuite) TestCheckPartitionBusyFunc(c *check.C) {
+	testCases := []struct {
+		execCommandOutput string
+		expected          string
+	}{
+		{`prg  4827 user  mem    REG    8,2      3339 10354893 /usr/share/prg
+prg  4827 user   15w   REG    8,2    197132 12452026 /home/user/prg`, "15w"},
+		{`prg  4827 user  mem    REG    8,2      3339 10354893 /usr/share/prg
+prg  4827 user   15w   REG    8,2    197132 12452026 /home/user/prg
+prg  4827 user   1w   REG    8,2    197132 12452026 /home/user/prg`, "15w"},
+		{`prg  4827 user  mem    REG    8,2      3339 10354893 /usr/share/prg`, lsofNotBeingWritten},
+		{`prg  4827 user  cwd    REG    8,2      3339 10354893 /usr/share/prg`, lsofNotBeingWritten},
+	}
+
+	for _, testCase := range testCases {
+		s.execOutput = testCase.execCommandOutput
+		f := checkPathBusyFunc(path)
+
+		actual, err := f()
+		c.Check(err, check.IsNil)
+		c.Check(actual, check.Equals, testCase.expected,
+			check.Commentf("input text %s, expected output %s, obtained %s",
+				testCase.execCommandOutput, testCase.expected, actual))
+	}
+}
+
+func (s *partitionTestSuite) TestCheckPartitionBusyFuncReturnsErrorOnLsofError(c *check.C) {
+	s.execOutput = "not a lsof common output on not used partitions"
+	s.execError = true
+
+	f := checkPathBusyFunc(path)
+
+	actual, err := f()
+
+	c.Check(err, check.NotNil)
+	c.Check(actual, check.Equals, s.execOutput)
 }