Add retries logic to get devices by label (#57)

Signed-off-by: David Cassany <dcassany@suse.com>

pkg/elemental/elemental.go

@@ -172,7 +172,8 @@ func (c Elemental) MountPartition(part *v1.Partition, opts ...string) error {
 	if err != nil {
 		return err
 	}
-	device, err := utils.GetDeviceByLabel(c.config.Runner, part.Label)
+	// Lets error out only after 10 attempts to find the device
+	device, err := utils.GetDeviceByLabel(c.config.Runner, part.Label, 10)
 	if err != nil {
 		c.config.Logger.Errorf("Could not find a device with label %s", part.Label)
 		return err
@@ -343,7 +344,7 @@ func (c *Elemental) CheckNoFormat() error {
 	c.config.Logger.Infof("Checking no-format condition")
 	// User asked for no format, lets check if there are already those labeled file systems in the disk
 	for _, label := range []string{c.config.ActiveImage.Label, c.config.PassiveLabel} {
-		found, _ := utils.FindLabel(c.config.Runner, label)
+		found, _ := utils.GetDeviceByLabel(c.config.Runner, label, 1)
 		if found != "" {
 			if c.config.Force {
 				msg := fmt.Sprintf("Forcing overwrite of existing OS image due to `force` flag")

pkg/elemental/elemental_test.go

@@ -20,6 +20,7 @@ import (
 	"errors"
 	"fmt"
 	. "github.com/onsi/ginkgo"
+	"github.com/onsi/ginkgo/config"
 	. "github.com/onsi/gomega"
 	cnst "github.com/rancher-sandbox/elemental/pkg/constants"
 	"github.com/rancher-sandbox/elemental/pkg/elemental"
@@ -39,7 +40,7 @@ const partTmpl = `
 
 func TestElementalSuite(t *testing.T) {
 	RegisterFailHandler(Fail)
-	//config.DefaultReporterConfig.SlowSpecThreshold = 10
+	config.DefaultReporterConfig.SlowSpecThreshold = 11
 	RunSpecs(t, "Elemental test suite")
 }
 
@@ -98,7 +99,7 @@ var _ = Describe("Elemental", func() {
 
 		It("Fails if oem partition is not found ", func() {
 			runner.SideEffect = func(cmd string, args ...string) ([]byte, error) {
-				if len(args) >= 2 && args[1] == fmt.Sprintf("LABEL=%s", cnst.OEMLabel) {
+				if len(args) >= 2 && args[1] == cnst.OEMLabel {
 					return []byte{}, nil
 				}
 				return []byte("/some/device"), nil

pkg/partitioner/disk.go

@@ -270,9 +270,8 @@ func (dev Disk) FindPartitionDevice(partNum int) (string, error) {
 	re, _ := regexp.Compile(fmt.Sprintf("(?m)^(/.*%d) part$", partNum))
 
 	for tries := 0; tries <= partitionTries; tries++ {
-		out, _ := dev.runner.Run("udevadm", "settle")
-		dev.logger.Debugf("Output of udevadm settle: %s", out)
 		dev.logger.Debugf("Trying to find the partition device %d of device %s (try number %d)", partNum, dev, tries+1)
+		dev.runner.Run("udevadm", "settle")
 		out, err := dev.runner.Run("lsblk", "-ltnpo", "name,type", dev.device)
 		dev.logger.Debugf("Output of lsblk: %s", out)
 		if err != nil && tries == (partitionTries-1) {

pkg/utils/common.go

@@ -25,6 +25,7 @@ import (
 	"io"
 	"os/exec"
 	"strings"
+	"time"
 )
 
 func CommandExists(command string) bool {
@@ -38,25 +39,19 @@ func BootedFrom(runner v1.Runner, label string) bool {
 	return strings.Contains(string(out), label)
 }
 
-// FindLabel will try to get the partition that has the label given in the current disk
-func FindLabel(runner v1.Runner, label string) (string, error) {
-	out, err := runner.Run("blkid", "-L", label)
-	if err != nil {
-		return "", err
-	}
-	return strings.TrimSpace(string(out)), nil
-}
-
-// GetDeviceByLabel will try to return the device that matches the given label
-func GetDeviceByLabel(runner v1.Runner, label string) (string, error) {
-	out, err := runner.Run("blkid", "-t", fmt.Sprintf("LABEL=%s", label), "-o", "device")
-	if err != nil {
-		return "", err
-	}
-	if strings.TrimSpace(string(out)) == "" {
-		return "", errors.New("no device found")
+// GetDeviceByLabel will try to return the device that matches the given label.
+// attempts value sets the number of attempts to find the device, it
+// waits a second between attempts.
+func GetDeviceByLabel(runner v1.Runner, label string, attempts int) (string, error) {
+	for tries := 0; tries < attempts; tries++ {
+		runner.Run("udevadm", "settle")
+		out, err := runner.Run("blkid", "--label", label)
+		if err == nil && strings.TrimSpace(string(out)) != "" {
+			return strings.TrimSpace(string(out)), nil
+		}
+		time.Sleep(1 * time.Second)
 	}
-	return strings.TrimSpace(string(out)), nil
+	return "", errors.New("no device found")
 }
 
 // Copies source file to target file using afero.Fs interface

pkg/utils/utils_test.go

@@ -145,56 +145,34 @@ var _ = Describe("Utils", func() {
 			Expect(utils.BootedFrom(runner, "FAKELABEL")).To(BeTrue())
 		})
 	})
-	Context("FindLabel", func() {
-		It("returns empty and no error if label does not exist", func() {
-			runner := v1mock.NewTestRunnerV2()
-			runner.ReturnValue = []byte("")
-			out, err := utils.FindLabel(runner, "FAKE")
-			Expect(err).To(BeNil())
-			Expect(out).To(BeEmpty())
-		})
-		It("returns path and no error if label does exist", func() {
-			runner := v1mock.NewTestRunnerV2()
-			runner.ReturnValue = []byte("/dev/fake")
-			out, err := utils.FindLabel(runner, "FAKE")
-			Expect(err).To(BeNil())
-			Expect(out).To(Equal("/dev/fake"))
-		})
-		It("returns empty and error if there is an error", func() {
-			runner := v1mock.NewTestRunnerV2()
-			runner.ReturnError = errors.New("something")
-			out, err := utils.FindLabel(runner, "FAKE")
-			Expect(err).ToNot(BeNil())
-			Expect(out).To(Equal(""))
-		})
-	})
 	Context("GetDeviceByLabel", func() {
 		var runner *v1mock.TestRunnerV2
 		var cmds [][]string
 		BeforeEach(func() {
 			runner = v1mock.NewTestRunnerV2()
 			cmds = [][]string{
-				{"blkid", "-t", "LABEL=FAKE", "-o", "device"},
+				{"udevadm", "settle"},
+				{"blkid", "--label", "FAKE"},
 			}
 		})
 		It("returns found device", func() {
 			runner.ReturnValue = []byte("/some/device")
-			out, err := utils.GetDeviceByLabel(runner, "FAKE")
+			out, err := utils.GetDeviceByLabel(runner, "FAKE", 1)
 			Expect(err).To(BeNil())
 			Expect(out).To(Equal("/some/device"))
 			Expect(runner.CmdsMatch(cmds)).To(BeNil())
 		})
 		It("fails to run blkid", func() {
 			runner.ReturnError = errors.New("failed running blkid")
-			_, err := utils.GetDeviceByLabel(runner, "FAKE")
+			_, err := utils.GetDeviceByLabel(runner, "FAKE", 1)
 			Expect(err).NotTo(BeNil())
 			Expect(runner.CmdsMatch(cmds)).To(BeNil())
 		})
-		It("fails if no device is found", func() {
+		It("fails if no device is found in two attempts", func() {
 			runner.ReturnValue = []byte("")
-			_, err := utils.GetDeviceByLabel(runner, "FAKE")
+			_, err := utils.GetDeviceByLabel(runner, "FAKE", 2)
 			Expect(err).NotTo(BeNil())
-			Expect(runner.CmdsMatch(cmds)).To(BeNil())
+			Expect(runner.CmdsMatch(append(cmds, cmds...))).To(BeNil())
 		})
 	})
 	Context("CopyFile", func() {