update reviews

pkg/cluster-runtime/apps.go

@@ -21,6 +21,8 @@ import (
 	"os"
 	"syscall"
 
+	"github.com/sirupsen/logrus"
+
 	"github.com/moby/buildkit/frontend/dockerfile/shell"
 	"github.com/sealerio/sealer/common"
 	containerruntime "github.com/sealerio/sealer/pkg/container-runtime"
@@ -107,28 +109,32 @@ func (i AppInstaller) LaunchClusterImage(master0 net.IP, launchCmds []string) er
 // todo save image info to disk or api server, we need new interface to do this.
 func (i AppInstaller) save(applicationFile string) error {
 	f, err := os.OpenFile(applicationFile, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0600)
-	defer func() {
-		_ = f.Close()
-	}()
-
 	if err != nil {
 		return err
 	}
+	defer func() {
+		_ = f.Close()
+	}()
 
 	err = syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB)
 	if err != nil {
 		return fmt.Errorf("cannot flock file %s - %s", applicationFile, err)
 	}
+	defer func() {
+		err := syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
+		if err != nil {
+			logrus.Errorf("failed to unlock %s", applicationFile)
+		}
+	}()
 
 	content, err := json.Marshal(i.extension)
 	if err != nil {
 		return fmt.Errorf("failed to marshal image extension: %v", err)
 	}
 
-	_, err = f.Write(content)
-	if err != nil {
+	if _, err = f.Write(content); err != nil {
 		return err
 	}
 
-	return syscall.Flock(int(f.Fd()), syscall.LOCK_UN)
+	return nil
 }

pkg/imagedistributor/mount.go

@@ -38,7 +38,7 @@ type buildAhMounter struct {
 
 func (b buildAhMounter) Mount(imageName string, platform v1.Platform) (string, error) {
 	path := platform.OS + "_" + platform.Architecture + "_" + platform.Variant
-	mountDir := filepath.Join(mountBaseDir, formatImageName(imageName), path)
+	mountDir := filepath.Join(imageMountDir(imageName), path)
 	if osi.IsFileExist(mountDir) {
 		err := os.RemoveAll(mountDir)
 		if err != nil {
@@ -55,12 +55,11 @@ func (b buildAhMounter) Mount(imageName string, platform v1.Platform) (string, e
 	}
 
 	// make sure base mount Dir is existed.
-	err := fs.FS.MkdirAll(filepath.Dir(mountDir))
-	if err != nil {
+	if err := fs.FS.MkdirAll(filepath.Dir(mountDir)); err != nil {
 		return "", err
 	}
 
-	if _, err = b.imageEngine.CreateWorkingContainer(&imagecommon.BuildRootfsOptions{
+	if _, err := b.imageEngine.CreateWorkingContainer(&imagecommon.BuildRootfsOptions{
 		DestDir:       mountDir,
 		ImageNameOrID: imageName,
 	}); err != nil {
@@ -126,7 +125,7 @@ func (c ImagerMounter) Umount(imageName string, imageMountInfo []ClusterImageMou
 		}
 	}
 	// delete all mounted images
-	if err := fs.FS.RemoveAll(filepath.Join(mountBaseDir, formatImageName(imageName))); err != nil {
+	if err := fs.FS.RemoveAll(imageMountDir(imageName)); err != nil {
 		return err
 	}
 	return nil
@@ -140,6 +139,6 @@ func NewImageMounter(imageEngine imageengine.Interface, hostsPlatform map[v1.Pla
 	return c, nil
 }
 
-func formatImageName(name string) string {
-	return strings.ReplaceAll(name, "/", "_")
+func imageMountDir(name string) string {
+	return filepath.Join(mountBaseDir, strings.ReplaceAll(name, "/", "_"))
 }

pkg/imagedistributor/mount_test.go

@@ -0,0 +1,40 @@
+// Copyright © 2022 Alibaba Group Holding Ltd.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package imagedistributor
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestImageMountDir(t *testing.T) {
+	tests := []struct {
+		testName   string
+		imageName  string
+		wantedPath string
+	}{
+		{testName: "image name with tag", imageName: "nginx:v1", wantedPath: "/var/lib/sealer/data/mount/nginx:v1"},
+		{testName: "image name with repo and tag", imageName: "library/nginx:v1", wantedPath: "/var/lib/sealer/data/mount/library_nginx:v1"},
+		{testName: "image name with domain repo and tag", imageName: "docker.io/library/nginx:v1", wantedPath: "/var/lib/sealer/data/mount/docker.io_library_nginx:v1"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.testName, func(t *testing.T) {
+			mountedPath := imageMountDir(tt.imageName)
+			assert.Equal(t, tt.wantedPath, mountedPath)
+		})
+	}
+}