Merge e6614d5 into c2a1a1d

UpdateHub · Jul 24, 2018 · 4247022 · 4247022
2 parents c2a1a1d + e6614d5
commit 4247022
Show file tree

Hide file tree

Showing 2 changed files with 29 additions and 7 deletions.
diff --git a/installmodes/tarball/tarball.go b/installmodes/tarball/tarball.go
@@ -55,6 +55,7 @@ type TarballObject struct {
 	CopyBackend            copy.Interface `json:"-"`
 	mtd.MtdUtils
 	mtd.UbifsUtils
+	tempDirPath string
 
 	Target        string `json:"target"`
 	TargetType    string `json:"target-type"`
@@ -108,21 +109,22 @@ func (tb *TarballObject) Install(downloadDir string) error {
 		}
 	}
 
-	tempDirPath, err := tb.TempDir(tb.FileSystemBackend, "tarball-handler")
+	var err error
+	tb.tempDirPath, err = tb.TempDir(tb.FileSystemBackend, "tarball-handler")
 	if err != nil {
 		return err
 	}
 	// we can't "defer os.RemoveAll(tempDirPath)" here because it
 	// could happen an "Umount" error and then the mounted dir
 	// contents would be removed as well
 
-	err = tb.Mount(tb.Target, tempDirPath, tb.FSType, tb.MountOptions)
+	err = tb.Mount(tb.Target, tb.tempDirPath, tb.FSType, tb.MountOptions)
 	if err != nil {
-		tb.FileSystemBackend.RemoveAll(tempDirPath)
+		tb.FileSystemBackend.RemoveAll(tb.tempDirPath)
 		return err
 	}
 
-	targetPath := path.Join(tempDirPath, tb.TargetPath)
+	targetPath := path.Join(tb.tempDirPath, tb.TargetPath)
 
 	errorList := []error{}
 
@@ -132,11 +134,11 @@ func (tb *TarballObject) Install(downloadDir string) error {
 		errorList = append(errorList, err)
 	}
 
-	umountErr := tb.Umount(tempDirPath)
+	umountErr := tb.Umount(tb.tempDirPath)
 	if umountErr != nil {
 		errorList = append(errorList, umountErr)
 	} else {
-		tb.FileSystemBackend.RemoveAll(tempDirPath)
+		tb.FileSystemBackend.RemoveAll(tb.tempDirPath)
 	}
 
 	return utils.MergeErrorList(errorList)
@@ -145,5 +147,17 @@ func (tb *TarballObject) Install(downloadDir string) error {
 // Cleanup implementation for the "tarball" handler
 func (tb *TarballObject) Cleanup() error {
 	log.Debug("'tarball' handler Cleanup")
+
+	err := tb.Umount(tb.tempDirPath)
+	if err != nil {
+		return err
+	}
+
+	// make sure there is NO umount error when calling
+	// "os.RemoveAll(tb.tempDirPath)" here. because in this case the
+	// mounted dir contents would be removed too
+	tb.FileSystemBackend.RemoveAll(tb.tempDirPath)
+	tb.tempDirPath = ""
+
 	return nil
 }
diff --git a/installmodes/tarball/tarball_test.go b/installmodes/tarball/tarball_test.go
@@ -454,6 +454,14 @@ func TestTarballInstallWithSuccess(t *testing.T) {
 }
 
 func TestTarballCleanupNil(t *testing.T) {
-	tb := TarballObject{}
+	memFs := afero.NewMemMapFs()
+
+	tempDirPath, err := afero.TempDir(memFs, "", "tarball-handler")
+	assert.NoError(t, err)
+
+	fsm := &filesystemmock.FileSystemHelperMock{}
+	fsm.On("Umount", tempDirPath).Return(nil)
+
+	tb := TarballObject{FileSystemHelper: fsm, FileSystemBackend: memFs, tempDirPath: tempDirPath}
 	assert.Nil(t, tb.Cleanup())
 }