lvm: more tests for lowlevel.*

resource/lvm/fs/fs_test.go

@@ -25,7 +25,7 @@ func TestCreateFilesystem(t *testing.T) {
 	me.On("ReadFile", "/etc/systemd/system/mnt-data.mount").Return([]byte(""), nil)
 	me.On("WriteFile", "/etc/systemd/system/mnt-data.mount", mock.Anything, mock.Anything).Return(nil)
 	me.On("Run", "mkfs", []string{"-t", "xfs", "/dev/mapper/vg0-data"}).Return(nil)
-	me.On("RunExitCode", "mountpoint", []string{"-q", "/mnt/data"}).Return(1, nil)
+	me.On("RunWithExitCode", "mountpoint", []string{"-q", "/mnt/data"}).Return(1, nil)
 	me.On("Run", "systemctl", []string{"daemon-reload"}).Return(nil)
 	me.On("Run", "systemctl", []string{"start", "mnt-data.mount"}).Return(nil)
 

resource/lvm/lowlevel/exec.go

@@ -18,7 +18,7 @@ import (
 // FIXME: split to ExecInterface, FilesystemInterface (possible also SystemInterface for Getuid)
 type Exec interface {
 	Run(prog string, args []string) error
-	RunExitCode(prog string, args []string) (int, error) // for mountpoint querying
+	RunWithExitCode(prog string, args []string) (int, error) // for mountpoint querying
 	Read(prog string, args []string) (stdout string, err error)
 	ReadWithExitCode(prog string, args []string) (stdout string, rc int, err error) // for blkid querying
 	Lookup(prog string) error
@@ -34,6 +34,7 @@ type Exec interface {
 type osExec struct {
 }
 
+// MakeOsExec create Exec backend
 func MakeOsExec() Exec {
 	return &osExec{}
 }
@@ -49,23 +50,24 @@ func (*osExec) Run(prog string, args []string) error {
 	return e
 }
 
-func (e *osExec) RunExitCode(prog string, args []string) (int, error) {
+func (e *osExec) RunWithExitCode(prog string, args []string) (int, error) {
 	err := e.Run(prog, args)
 	return exitStatus(err)
 }
 
 func (*osExec) ReadWithExitCode(prog string, args []string) (stdout string, rc int, err error) {
+	rc = 0
 	log.WithField("module", "lvm").Infof("Executing (read) %s: %v", prog, args)
 	out, err := exec.Command(prog, args...).Output()
+	strOut := strings.Trim(string(out), "\n ")
 	if err != nil {
-		if rc, err := exitStatus(err); err != nil {
+		rc, err = exitStatus(err)
+		if err != nil {
 			log.WithField("module", "lvm").Debugf("%s: terminated with %s", prog, err.Error())
 			return "", 0, errors.Wrapf(err, "reading output of process %s: %s", prog, args)
-		} else {
-			return strings.Trim(string(out), "\n "), rc, nil
 		}
 	}
-	return strings.Trim(string(out), "\n "), 0, err
+	return strOut, rc, err
 }
 
 func (e *osExec) Read(prog string, args []string) (stdout string, err error) {

resource/lvm/lowlevel/exec_test.go

@@ -22,7 +22,7 @@ func TestExecRun(t *testing.T) {
 	t.Run("fail", func(t *testing.T) {
 		defer logging.HideLogs(t)()
 		e := lowlevel.MakeOsExec()
-		err := e.Run("sh", []string{"-c", "false"})
+		err := e.Run("/bin/sh", []string{"-c", "false"})
 		assert.Error(t, err)
 	})
 }
@@ -34,15 +34,15 @@ func TestExecRead(t *testing.T) {
 	t.Run("normal", func(t *testing.T) {
 		defer logging.HideLogs(t)()
 		e := lowlevel.MakeOsExec()
-		out, err := e.Read("sh", []string{"-c", "echo foo"})
+		out, err := e.Read("/bin/sh", []string{"-c", "echo foo"})
 		assert.NoError(t, err)
 		assert.Equal(t, "foo", out)
 	})
 
 	t.Run("failure", func(t *testing.T) {
 		defer logging.HideLogs(t)()
 		e := lowlevel.MakeOsExec()
-		_, err := e.Read("sh", []string{"-c", "echo foo && false"})
+		_, err := e.Read("/bin/sh", []string{"-c", "echo foo && false"})
 		assert.Error(t, err)
 		// FIXME: underlying exec.Command looks not return output on error
 		//        would be nice to have all output in logs in case of error
@@ -52,8 +52,92 @@ func TestExecRead(t *testing.T) {
 	t.Run("multiline", func(t *testing.T) {
 		defer logging.HideLogs(t)()
 		e := lowlevel.MakeOsExec()
-		out, err := e.Read("sh", []string{"-c", "echo foo && echo bar"})
+		out, err := e.Read("/bin/sh", []string{"-c", "echo foo && echo bar"})
 		assert.NoError(t, err)
 		assert.Equal(t, "foo\nbar", out)
 	})
 }
+
+// TestExecRunWithExitCode tests Exec.RunWithExitCode
+func TestExecRunWithExitCode(t *testing.T) {
+	t.Run("zero code", func(t *testing.T) {
+		defer logging.HideLogs(t)()
+		e := lowlevel.MakeOsExec()
+		rc, err := e.RunWithExitCode("/bin/sh", []string{"-c", "true"})
+		assert.NoError(t, err)
+		assert.Equal(t, 0, rc)
+	})
+
+	t.Run("non-zero code", func(t *testing.T) {
+		defer logging.HideLogs(t)()
+		e := lowlevel.MakeOsExec()
+		rc, err := e.RunWithExitCode("/bin/sh", []string{"-c", "exit 42"})
+		assert.NoError(t, err)
+		assert.Equal(t, 42, rc)
+	})
+
+	t.Run("failure", func(t *testing.T) {
+		defer logging.HideLogs(t)()
+		e := lowlevel.MakeOsExec()
+		_, err := e.RunWithExitCode("/tricky/command/which/never-exists", []string{"-c", "true"})
+		assert.Error(t, err)
+	})
+}
+
+// TestExecReadWithExitCode tests Exec.ReadWithExitCode
+func TestExecReadWithExitCode(t *testing.T) {
+	t.Run("zero code", func(t *testing.T) {
+		defer logging.HideLogs(t)()
+		e := lowlevel.MakeOsExec()
+		out, rc, err := e.ReadWithExitCode("/bin/sh", []string{"-c", "echo foo"})
+		assert.NoError(t, err)
+		assert.Equal(t, 0, rc)
+		assert.Equal(t, "foo", out)
+	})
+
+	t.Run("non-zero code", func(t *testing.T) {
+		defer logging.HideLogs(t)()
+		e := lowlevel.MakeOsExec()
+		out, rc, err := e.ReadWithExitCode("/bin/sh", []string{"-c", "echo foo && exit 42"})
+		assert.NoError(t, err)
+		assert.Equal(t, 42, rc)
+		assert.Equal(t, "foo", out)
+	})
+
+	t.Run("failure", func(t *testing.T) {
+		defer logging.HideLogs(t)()
+		e := lowlevel.MakeOsExec()
+		_, _, err := e.ReadWithExitCode("/tricky/command/which/never-exists", []string{"-c", "echo foo"})
+		assert.Error(t, err)
+	})
+}
+
+// TestExecLookup tests Exec.Lookup()
+func TestExecLookup(t *testing.T) {
+	t.Run("command `sh` which always exists", func(t *testing.T) {
+		e := lowlevel.MakeOsExec()
+		ok := e.Lookup("sh")
+		assert.Error(t, ok)
+	})
+	t.Run("some command which never exists", func(t *testing.T) {
+		e := lowlevel.MakeOsExec()
+		ok := e.Lookup("some-command-which-never-exists-in-normal-system")
+		assert.NoError(t, ok)
+	})
+}
+
+// TestExecExists tests Exec.Exists()
+func TestExecExists(t *testing.T) {
+	t.Run("file /bin/sh exists", func(t *testing.T) {
+		e := lowlevel.MakeOsExec()
+		ok, err := e.Exists("/bin/sh")
+		assert.NoError(t, err)
+		assert.True(t, ok)
+	})
+	t.Run("some file which does not exists", func(t *testing.T) {
+		e := lowlevel.MakeOsExec()
+		ok, err := e.Exists("/tricky/file/which/never-exists")
+		assert.NoError(t, err)
+		assert.False(t, ok)
+	})
+}

resource/lvm/lowlevel/lv.go

@@ -1,5 +1,7 @@
 package lowlevel
 
+// LogicalVolume is parsed record for LVM Logical Volume (from `lvs` output)
+// Add more fields, if required
 type LogicalVolume struct {
 	Name       string `mapstructure:"LVM2_LV_NAME"`
 	DevicePath string `mapstructure:"LVM2_LV_DM_PATH"`

resource/lvm/lowlevel/pv.go

@@ -4,6 +4,8 @@ import (
 	"strings"
 )
 
+// PhysicalVolume is parsed record for LVM Physical Volume (from `pvs` output)
+// Add more fields, if required
 type PhysicalVolume struct {
 	Name   string `mapstructure:"LVM2_PV_NAME"`
 	Group  string `mapstructure:"LVM2_VG_NAME"`

resource/lvm/lowlevel/size.go

@@ -41,6 +41,7 @@ func (size *LvmSize) CommandLine() [2]string {
 	return [2]string{o, s}
 }
 
+// ParseSize parsing and validating sizes in format acceptable by LVM tools
 func ParseSize(sizeToParse string) (*LvmSize, error) {
 	var err error
 	size := &LvmSize{}
@@ -52,7 +53,7 @@ func ParseSize(sizeToParse string) (*LvmSize, error) {
 			return nil, errors.Wrap(err, "Parse LVM size")
 		}
 		if size.Size > 100 {
-			return nil, fmt.Errorf("size in %% can't be more than 100%%: %d", size)
+			return nil, fmt.Errorf("size in %% can't be more than 100%%: %d", size.Size)
 		}
 	} else if m := sizeRE.FindStringSubmatch(sizeToParse); m != nil {
 		size.Relative = false

resource/lvm/lowlevel/size_test.go

@@ -8,6 +8,7 @@ import (
 	"testing"
 )
 
+// TestParseSize tests ParseSize()
 func TestParseSize(t *testing.T) {
 	t.Parallel()
 	defer logging.HideLogs(t)()

resource/lvm/lowlevel/systemd_test.go

@@ -11,43 +11,50 @@ import (
 	"testing"
 )
 
-func TestUnitFileNotExists(t *testing.T) {
-	filename := "/test-unit-file-which-never-exists.xxx"
-	currentContent := "this is a test"
-	lvm := lowlevel.MakeLvmBackend()
-	ok, err := lvm.CheckUnit(filename, currentContent)
-	assert.NoError(t, err)
-	assert.True(t, ok)
-}
-
-func TestUnitFileContentDiffs(t *testing.T) {
-	originalContent := "a test this is"
-	currentContent := "this is a test"
-	tmpfile, err := ioutil.TempFile("", "test-unit-file-contents-diff")
-
-	require.NoError(t, err)
-	defer func() { require.NoError(t, os.RemoveAll(tmpfile.Name())) }()
-
-	_, err = tmpfile.Write([]byte(originalContent))
-	require.NoError(t, err)
-	require.NoError(t, tmpfile.Sync())
+// TestLVMCheckUnit tests LVM.CheckUnit
+func TestLVMCheckUnit(t *testing.T) {
+	t.Run("unit file not exists", func(t *testing.T) {
+		filename := "/test-unit-file-which-never-exists.xxx"
+		currentContent := "this is a test"
+		lvm := lowlevel.MakeLvmBackend()
+		ok, err := lvm.CheckUnit(filename, currentContent)
+		assert.NoError(t, err)
+		assert.True(t, ok)
+	})
+
+	t.Run("unit file content diffs", func(t *testing.T) {
+		originalContent := "a test this is"
+		currentContent := "this is a test"
+		tmpfile, err := ioutil.TempFile("", "test-unit-file-contents-diff")
+
+		require.NoError(t, err)
+		defer func() { require.NoError(t, os.RemoveAll(tmpfile.Name())) }()
+
+		_, err = tmpfile.Write([]byte(originalContent))
+		require.NoError(t, err)
+		require.NoError(t, tmpfile.Sync())
+
+		lvm := lowlevel.MakeLvmBackend()
+		ok, err := lvm.CheckUnit(tmpfile.Name(), currentContent)
+		assert.NoError(t, err)
+		assert.True(t, ok)
+	})
 
-	lvm := lowlevel.MakeLvmBackend()
-	ok, err := lvm.CheckUnit(tmpfile.Name(), currentContent)
-	assert.NoError(t, err)
-	assert.True(t, ok)
 }
 
-func TestUnitFileUpdate(t *testing.T) {
-	currentContent := "this is a test"
-	filename := "/systemd/test.unit"
+// TestLVMUpdateUnit tests LVM.UpdateUnit()
+func TestLVMUpdateUnit(t *testing.T) {
+	t.Run("update unit file", func(t *testing.T) {
+		currentContent := "this is a test"
+		filename := "/systemd/test.unit"
 
-	lvm, me := testhelpers.MakeLvmWithMockExec()
+		lvm, me := testhelpers.MakeLvmWithMockExec()
 
-	// FIXME:   should be 0644 here, but call mismatch. Looks like BUG
-	me.On("WriteFile", filename, []byte(currentContent), mock.Anything).Return(nil)
-	me.On("Run", "systemctl", []string{"daemon-reload"}).Return(nil)
+		// FIXME:   should be 0644 here, but call mismatch. Looks like BUG
+		me.On("WriteFile", filename, []byte(currentContent), mock.Anything).Return(nil)
+		me.On("Run", "systemctl", []string{"daemon-reload"}).Return(nil)
 
-	err := lvm.UpdateUnit(filename, currentContent)
-	assert.NoError(t, err)
+		err := lvm.UpdateUnit(filename, currentContent)
+		assert.NoError(t, err)
+	})
 }

resource/lvm/lowlevel/util.go

@@ -80,7 +80,7 @@ func (lvm *realLVM) Mkfs(dev string, fstype string) error {
 }
 
 func (lvm *realLVM) Mountpoint(path string) (bool, error) {
-	rc, err := lvm.backend.RunExitCode("mountpoint", []string{"-q", path})
+	rc, err := lvm.backend.RunWithExitCode("mountpoint", []string{"-q", path})
 	if err != nil {
 		return false, err
 	}

resource/lvm/lowlevel/vg.go

@@ -1,5 +1,8 @@
 package lowlevel
 
+// VolumeGroup is parsed record for LVM Volume Groups (from `vgs` output)
+// Add more fields, if required
+// (at the moment we need only LVM2_VG_NAME to get list all existing groups)
 type VolumeGroup struct {
 	Name string `mapstructure:"LVM2_VG_NAME"`
 }

resource/lvm/testhelpers/mock.go

@@ -25,7 +25,7 @@ func (me *MockExecutor) Run(prog string, args []string) error {
 	return c.Error(0)
 }
 
-func (e *MockExecutor) RunExitCode(prog string, args []string) (int, error) {
+func (e *MockExecutor) RunWithExitCode(prog string, args []string) (int, error) {
 	c := e.Called(prog, args)
 	return c.Int(0), c.Error(1)
 }