Merge lp:~elopio/snappy/integration-fix-rollback into lp:~snappy-dev/snappy/snappy-moved-to-github

[Leo Arias]

Fixed the integration fake rollback test. It was using the deprecated uboot config file.

[Leo Arias (elopio)]

This fixes the queries to the uboot, but doens't yet make the test pass in bbb.
I'm proposing this now as a clear bug fix, and I'll start to think what to do for the bbb. The problem is that it's being extra slow, so we have to increase the timeout. It would be nice to have a solution so the timeout is not hardcoded. And it would be even nicer if the bug that causes this to happen is fixed. Bug #1498293

Unmerged revisions

779. By Leo Arias on 2015-10-16

Merged with parent.

778. By Leo Arias on 2015-10-16

Check for the wait error.

777. By Leo Arias on 2015-10-16

Use the binary uboot configuration file.

776. By Leo Arias on 2015-10-15

Added a test for the grub confValue.

775. By Leo Arias on 2015-10-15

Replaced the confValue in the tests for a fake.

=== modified file '_integration-tests/tests/rollback_test.go'
--- _integration-tests/tests/rollback_test.go	2015-10-16 00:47:50 +0000
+++ _integration-tests/tests/rollback_test.go	2015-10-16 00:47:50 +0000
@@ -44,7 +44,8 @@
 		common.RemoveRebootMark(c)
 		// Workaround for bug https://bugs.launchpad.net/snappy/+bug/1498293
 		// TODO remove once the bug is fixed. --elopio - 2015-09-30
-		wait.ForFunction(c, "regular", partition.Mode)
+		err := wait.ForFunction(c, "regular", partition.Mode)
+		c.Assert(err, check.IsNil, check.Commentf("Failed to switch to regular mode: %s", err))
 		currentVersion := common.GetCurrentUbuntuCoreVersion(c)
 		c.Assert(currentVersion > common.GetSavedVersion(c), check.Equals, true,
 			check.Commentf("Rebooted to the wrong version: %d", currentVersion))

=== modified file '_integration-tests/testutils/partition/bootloader.go'
--- _integration-tests/testutils/partition/bootloader.go	2015-10-02 10:29:29 +0000
+++ _integration-tests/testutils/partition/bootloader.go	2015-10-16 00:47:50 +0000
@@ -25,13 +25,15 @@
 	"os"
 	"path/filepath"
 	"strings"
+
+	"github.com/mvo5/uboot-go/uenv"
 )
 
 const (
 	bootBase        = "/boot"
 	ubootDir        = bootBase + "/uboot"
 	grubDir         = bootBase + "/grub"
-	ubootConfigFile = ubootDir + "/snappy-system.txt"
+	ubootConfigFile = ubootDir + "/uboot.env"
 	grubConfigFile  = grubDir + "/grubenv"
 )
 
@@ -41,6 +43,8 @@
 	// BootSystem proxies bootSystem
 	BootSystem = bootSystem
 
+	confValue = getConfValue
+
 	configFiles = map[string]string{"uboot": ubootConfigFile, "grub": grubConfigFile}
 )
 
@@ -101,14 +105,22 @@
 	return confValue("snappy_mode")
 }
 
-func confValue(key string) (partition string, err error) {
+func getConfValue(key string) (value string, err error) {
 	system, err := BootSystem()
 	if err != nil {
 		return
 	}
 
-	bootConfigFile := configFiles[system]
+	if system == "grub" {
+		value, err = getGrubConfValue(key)
+	} else if system == "uboot" {
+		value, err = getUbootConfValue(key)
+	}
+	return
+}
 
+func getGrubConfValue(key string) (value string, err error) {
+	bootConfigFile := configFiles["grub"]
 	file, err := os.Open(bootConfigFile)
 	if err != nil {
 		return
@@ -123,7 +135,7 @@
 		if strings.HasPrefix(scanner.Text(), key) {
 			fields := strings.Split(scanner.Text(), "=")
 			if len(fields) > 1 {
-				partition = fields[1]
+				value = fields[1]
 			}
 			return
 		}
@@ -131,6 +143,16 @@
 	return
 }
 
+func getUbootConfValue(key string) (value string, err error) {
+	bootConfigFile := configFiles["uboot"]
+	env, err := uenv.Open(bootConfigFile)
+	if err != nil {
+		return "", err
+	}
+
+	return env.Get(key), nil
+}
+
 // OtherPartition returns the backup partition, a or b.
 func OtherPartition(current string) string {
 	if current == "a" {

=== modified file '_integration-tests/testutils/partition/bootloader_test.go'
--- _integration-tests/testutils/partition/bootloader_test.go	2015-10-01 15:12:30 +0000
+++ _integration-tests/testutils/partition/bootloader_test.go	2015-10-16 00:47:50 +0000
@@ -23,8 +23,10 @@
 	"fmt"
 	"io/ioutil"
 	"os"
+	"path"
 	"testing"
 
+	"github.com/mvo5/uboot-go/uenv"
 	"gopkg.in/check.v1"
 )
 
@@ -36,25 +38,23 @@
 	backFilepathGlob         func(string) ([]string, error)
 	filepathGlobFail         bool
 	filepathGlobReturnValues []string
+	backConfValue            func(string) (string, error)
+	fakeConf                 map[string]string
 }
 
 var _ = check.Suite(&bootloaderTestSuite{})
 
-func createConfigFile(c *check.C, contents string) (name string) {
-	file, _ := ioutil.TempFile("", "snappy-bootloader-test")
-	err := ioutil.WriteFile(file.Name(), []byte(contents), 0644)
-	c.Assert(err, check.IsNil, check.Commentf("Error writing test bootloader file"))
-
-	return file.Name()
-}
-
 func (s *bootloaderTestSuite) SetUpSuite(c *check.C) {
 	s.backFilepathGlob = filepathGlob
 	filepathGlob = s.fakeFilepathGlob
+	s.backConfValue = confValue
+	confValue = s.fakeConfValue
+	s.fakeConf = map[string]string{}
 }
 
 func (s *bootloaderTestSuite) TearDownSuite(c *check.C) {
 	filepathGlob = s.backFilepathGlob
+	confValue = s.backConfValue
 }
 
 func (s *bootloaderTestSuite) SetUpTest(c *check.C) {
@@ -73,6 +73,10 @@
 	return s.filepathGlobReturnValues, nil
 }
 
+func (s *bootloaderTestSuite) fakeConfValue(key string) (value string, err error) {
+	return s.fakeConf[key], nil
+}
+
 func (s *bootloaderTestSuite) TestOtherPartition(c *check.C) {
 	c.Assert(OtherPartition("a"), check.Equals, "b",
 		check.Commentf("Expected OtherPartition of 'a' to be 'b'"))
@@ -98,11 +102,11 @@
 func (s *bootloaderTestSuite) TestBootSystemCallsFilepathGlob(c *check.C) {
 	BootSystem()
 
-	path := bootBase + "/grub"
-	calls := s.filepathGlobCalls[path]
+	p := bootBase + "/grub"
+	calls := s.filepathGlobCalls[p]
 
 	c.Assert(calls, check.Equals, 1,
-		check.Commentf("Expected calls to filepath.Glob with path %s to be 1, %d found", path, calls))
+		check.Commentf("Expected calls to filepath.Glob with path %s to be 1, %d found", p, calls))
 }
 
 func (s *bootloaderTestSuite) TestBootSystemForGrub(c *check.C) {
@@ -126,6 +130,11 @@
 }
 
 func (s *bootloaderTestSuite) TestNextBootPartitionReturnsBootSystemError(c *check.C) {
+	s.fakeConf = map[string]string{
+		"snappy_mode": "try",
+		"snappy_ab":   "dummy",
+	}
+
 	backBootSystem := BootSystem
 	defer func() { BootSystem = backBootSystem }()
 	expectedErr := fmt.Errorf("Error from BootSystem!")
@@ -139,30 +148,8 @@
 		check.Commentf("Expected error %v not found, %v", expectedErr, err))
 }
 
-func (s *bootloaderTestSuite) TestNextBootPartitionReturnsOsOpenError(c *check.C) {
-	backBootSystem := BootSystem
-	defer func() { BootSystem = backBootSystem }()
-	BootSystem = func() (system string, err error) {
-		return "not-expected-system", nil
-	}
-
-	backConfigFiles := configFiles
-	defer func() { configFiles = backConfigFiles }()
-	configFiles = map[string]string{"not-expected-system": "not-a-file"}
-
-	_, err := NextBootPartition()
-
-	c.Assert(err, check.FitsTypeOf, &os.PathError{},
-		check.Commentf("Expected error type *os.PathError not found, %T", err))
-}
-
 func (s *bootloaderTestSuite) TestNextBootPartitionReturnsEmptyIfPatternsNotFound(c *check.C) {
-	cfgFile := createConfigFile(c, "snappy_mode=try")
-	defer os.Remove(cfgFile)
-
-	backConfigFiles := configFiles
-	defer func() { configFiles = backConfigFiles }()
-	configFiles = map[string]string{"test-system": cfgFile}
+	s.fakeConf = map[string]string{"snappy_mode": "try"}
 
 	backBootSystem := BootSystem
 	defer func() { BootSystem = backBootSystem }()
@@ -178,16 +165,10 @@
 }
 
 func (s *bootloaderTestSuite) TestNextBootPartitionReturnsSamePartitionForGrub(c *check.C) {
-	cfgFileContents := `blabla
-snappy_mode=try
-snappy_ab=a
-blabla`
-	cfgFile := createConfigFile(c, cfgFileContents)
-	defer os.Remove(cfgFile)
-
-	backConfigFiles := configFiles
-	defer func() { configFiles = backConfigFiles }()
-	configFiles = map[string]string{"grub": cfgFile}
+	s.fakeConf = map[string]string{
+		"snappy_mode": "try",
+		"snappy_ab":   "a",
+	}
 
 	backBootSystem := BootSystem
 	defer func() { BootSystem = backBootSystem }()
@@ -203,16 +184,10 @@
 }
 
 func (s *bootloaderTestSuite) TestNextBootPartitionReturnsOtherPartitionForUBoot(c *check.C) {
-	cfgFileContents := `blabla
-snappy_mode=try
-snappy_ab=a
-blabla`
-	cfgFile := createConfigFile(c, cfgFileContents)
-	defer os.Remove(cfgFile)
-
-	backConfigFiles := configFiles
-	defer func() { configFiles = backConfigFiles }()
-	configFiles = map[string]string{"uboot": cfgFile}
+	s.fakeConf = map[string]string{
+		"snappy_mode": "try",
+		"snappy_ab":   "a",
+	}
 
 	backBootSystem := BootSystem
 	defer func() { BootSystem = backBootSystem }()
@@ -228,20 +203,8 @@
 }
 
 func (s *bootloaderTestSuite) TestModeReturnsSnappyModeFromConf(c *check.C) {
-	cfgFileContents := `blabla
-snappy_mode=test_mode
-blabla`
-	cfgFile := createConfigFile(c, cfgFileContents)
-	defer os.Remove(cfgFile)
-
-	backConfigFiles := configFiles
-	defer func() { configFiles = backConfigFiles }()
-	configFiles = map[string]string{"test-system": cfgFile}
-
-	backBootSystem := BootSystem
-	defer func() { BootSystem = backBootSystem }()
-	BootSystem = func() (system string, err error) {
-		return "test-system", nil
+	s.fakeConf = map[string]string{
+		"snappy_mode": "test_mode",
 	}
 
 	mode, err := Mode()
@@ -251,40 +214,22 @@
 }
 
 func (s *bootloaderTestSuite) TestCurrentPartitionNotOnTryMode(c *check.C) {
-	cfgFileContents := `blabla
-snappy_mode=nottry
-snappy_ab=test_partition
-blabla`
-	cfgFile := createConfigFile(c, cfgFileContents)
-	defer os.Remove(cfgFile)
-
-	backConfigFiles := configFiles
-	defer func() { configFiles = backConfigFiles }()
-	configFiles = map[string]string{"test-system": cfgFile}
-
-	backBootSystem := BootSystem
-	defer func() { BootSystem = backBootSystem }()
-	BootSystem = func() (system string, err error) {
-		return "test-system", nil
+	s.fakeConf = map[string]string{
+		"snappy_mode": "nottry",
+		"snappy_ab":   "test_partition",
 	}
 
-	mode, err := CurrentPartition()
+	part, err := CurrentPartition()
 
 	c.Assert(err, check.IsNil, check.Commentf("Unexpected error %v", err))
-	c.Assert(mode, check.Equals, "test_partition", check.Commentf("Wrong partition"))
+	c.Assert(part, check.Equals, "test_partition", check.Commentf("Wrong partition"))
 }
 
 func (s *bootloaderTestSuite) TestCurrentPartitionOnTryModeReturnsSamePartitionForUboot(c *check.C) {
-	cfgFileContents := `blabla
-snappy_mode=try
-snappy_ab=a
-blabla`
-	cfgFile := createConfigFile(c, cfgFileContents)
-	defer os.Remove(cfgFile)
-
-	backConfigFiles := configFiles
-	defer func() { configFiles = backConfigFiles }()
-	configFiles = map[string]string{"uboot": cfgFile}
+	s.fakeConf = map[string]string{
+		"snappy_mode": "try",
+		"snappy_ab":   "a",
+	}
 
 	backBootSystem := BootSystem
 	defer func() { BootSystem = backBootSystem }()
@@ -298,17 +243,11 @@
 	c.Assert(mode, check.Equals, "a", check.Commentf("Wrong partition"))
 }
 
-func (s *bootloaderTestSuite) TestCurrentPartitionOnTryModeReturnsOtherPartitionForUboot(c *check.C) {
-	cfgFileContents := `blabla
-snappy_mode=try
-snappy_ab=a
-blabla`
-	cfgFile := createConfigFile(c, cfgFileContents)
-	defer os.Remove(cfgFile)
-
-	backConfigFiles := configFiles
-	defer func() { configFiles = backConfigFiles }()
-	configFiles = map[string]string{"grub": cfgFile}
+func (s *bootloaderTestSuite) TestCurrentPartitionOnTryModeReturnsOtherPartitionForGrub(c *check.C) {
+	s.fakeConf = map[string]string{
+		"snappy_mode": "try",
+		"snappy_ab":   "a",
+	}
 
 	backBootSystem := BootSystem
 	defer func() { BootSystem = backBootSystem }()
@@ -321,3 +260,79 @@
 	c.Assert(err, check.IsNil, check.Commentf("Unexpected error %v", err))
 	c.Assert(mode, check.Equals, "b", check.Commentf("Wrong partition"))
 }
+
+type confTestSuite struct {
+	backBootSystem  func() (string, error)
+	system          string
+	backConfigFiles map[string]string
+}
+
+var _ = check.Suite(&confTestSuite{})
+
+func (s *confTestSuite) SetUpSuite(c *check.C) {
+	s.backBootSystem = BootSystem
+	BootSystem = s.fakeBootSystem
+	s.backConfigFiles = configFiles
+}
+
+func (s *confTestSuite) fakeBootSystem() (system string, err error) {
+	return s.system, nil
+}
+
+func (s *confTestSuite) TearDownSuite(c *check.C) {
+	BootSystem = s.backBootSystem
+	configFiles = s.backConfigFiles
+}
+
+func createConfigFile(c *check.C, system string, contents map[string]string) (name string) {
+	if system == "grub" {
+		name = createGrubConfigFile(c, contents)
+	} else if system == "uboot" {
+		name = createUbootConfigFile(c, contents)
+	}
+	return
+}
+
+func createGrubConfigFile(c *check.C, contents map[string]string) (name string) {
+	var contentsStr string
+	for key, value := range contents {
+		contentsStr += fmt.Sprintf("%s=%s\n", key, value)
+	}
+	file, err := ioutil.TempFile("", "snappy-grub-test")
+	c.Assert(err, check.IsNil, check.Commentf("Error creating temp file: %s", err))
+	err = ioutil.WriteFile(file.Name(), []byte(contentsStr), 0644)
+	c.Assert(err, check.IsNil, check.Commentf("Error writing test bootloader file: %s", err))
+
+	return file.Name()
+}
+
+func createUbootConfigFile(c *check.C, contents map[string]string) (name string) {
+	tmpDir, err := ioutil.TempDir("", "snappy-uboot-test")
+	c.Assert(err, check.IsNil, check.Commentf("Error creating temp dir: %s", err))
+	fileName := path.Join(tmpDir, "uboot.env")
+	env, err := uenv.Create(fileName, 4096)
+	c.Assert(err, check.IsNil, check.Commentf("Error creating the uboot env file: %s", err))
+	for key, value := range contents {
+		env.Set(key, value)
+	}
+	err = env.Save()
+	c.Assert(err, check.IsNil, check.Commentf("Error saving the uboot env file: %s", err))
+	return fileName
+}
+
+func (s *confTestSuite) TestGetConfValue(c *check.C) {
+	for _, s.system = range []string{"grub", "uboot"} {
+		cfgFileContents := map[string]string{
+			"blabla1": "bla",
+			"testkey": "testvalue",
+			"blabla2": "bla",
+		}
+		cfgFile := createConfigFile(c, s.system, cfgFileContents)
+		defer os.Remove(cfgFile)
+		configFiles = map[string]string{s.system: cfgFile}
+
+		value, err := confValue("testkey")
+		c.Check(err, check.IsNil, check.Commentf("Error getting configuration value: %s", err))
+		c.Check(value, check.Equals, "testvalue", check.Commentf("Wrong configuration value"))
+	}
+}

=== modified file '_integration-tests/testutils/partition/partition_test.go'
--- _integration-tests/testutils/partition/partition_test.go	2015-10-02 13:10:24 +0000
+++ _integration-tests/testutils/partition/partition_test.go	2015-10-16 00:47:50 +0000
@@ -27,9 +27,9 @@
 )
 
 const (
-	path        = "mypath"
-	writableCmd = "sudo mount -o remount,rw " + path
-	readonlyCmd = "sudo mount -o remount,ro " + path
+	testPath    = "mypath"
+	writableCmd = "sudo mount -o remount,rw " + testPath
+	readonlyCmd = "sudo mount -o remount,ro " + testPath
 	waitCmd     = lsofNotBeingWritten
 )
 
@@ -86,14 +86,14 @@
 }
 
 func (s *partitionTestSuite) TestMakeWritableCallsExecCommand(c *check.C) {
-	err := MakeWritable(c, path)
+	err := MakeWritable(c, testPath)
 
 	c.Assert(err, check.IsNil)
 	c.Assert(s.execCalls[writableCmd], check.Equals, 1)
 }
 
 func (s *partitionTestSuite) TestMakeWritableWaitsForIdlePartition(c *check.C) {
-	err := MakeWritable(c, path)
+	err := MakeWritable(c, testPath)
 
 	c.Assert(err, check.IsNil)
 	c.Assert(s.waitCalls[waitCmd], check.Equals, 1)
@@ -101,7 +101,7 @@
 
 func (s *partitionTestSuite) TestMakeWritableReturnsWaitError(c *check.C) {
 	s.waitError = true
-	err := MakeWritable(c, path)
+	err := MakeWritable(c, testPath)
 
 	c.Assert(err, check.NotNil)
 	c.Assert(s.waitCalls[waitCmd], check.Equals, 1)
@@ -109,14 +109,14 @@
 }
 
 func (s *partitionTestSuite) TestMakeReadOnlyCallsExecCommand(c *check.C) {
-	err := MakeReadonly(c, path)
+	err := MakeReadonly(c, testPath)
 
 	c.Assert(err, check.IsNil)
 	c.Assert(s.execCalls[readonlyCmd], check.Equals, 1)
 }
 
 func (s *partitionTestSuite) TestMakeReadonlyWaitsForIdlePartition(c *check.C) {
-	err := MakeReadonly(c, path)
+	err := MakeReadonly(c, testPath)
 
 	c.Assert(err, check.IsNil)
 	c.Assert(s.waitCalls[waitCmd], check.Equals, 1)
@@ -124,7 +124,7 @@
 
 func (s *partitionTestSuite) TestMakeReadonlyReturnsWaitError(c *check.C) {
 	s.waitError = true
-	err := MakeReadonly(c, path)
+	err := MakeReadonly(c, testPath)
 
 	c.Assert(err, check.NotNil)
 	c.Assert(s.waitCalls[waitCmd], check.Equals, 1)
@@ -147,7 +147,7 @@
 
 	for _, testCase := range testCases {
 		s.execOutput = testCase.execCommandOutput
-		f := checkPathBusyFunc(path)
+		f := checkPathBusyFunc(testPath)
 
 		actual, err := f()
 		c.Check(err, check.IsNil)
@@ -161,7 +161,7 @@
 	s.execOutput = "not a lsof common output on not used partitions"
 	s.execError = true
 
-	f := checkPathBusyFunc(path)
+	f := checkPathBusyFunc(testPath)
 
 	actual, err := f()