sealer run-app supports concurrent run #1850

kakaZhou719 · 2022-11-11T03:01:16Z

Describe what this PR does / why we need it

At present, our cluster image data is mixed with application image data and base image data, If we run multiple images at the same time, there will be conflicts.

Detach the mount directory
all imagedata mount directory will under below directory:
/var/lib/sealer/data/mount/${imageName}/${platform}

all application imagedata rootfs directory will under below directory:
var/lib/sealer/data/${imageName}/rootfs/application/apps

all basefs imagedata directory will under below directory:
var/lib/sealer/data/${imageName}/rootfs

add a file lock on .sealer/application.json

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

codecov-commenter · 2022-11-11T07:07:08Z

Codecov Report

Base: 21.65% // Head: 21.12% // Decreases project coverage by -0.52% ⚠️

Coverage data is based on head (3cc21a1) compared to base (f9cc112).
Patch coverage: 9.52% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1850      +/-   ##
==========================================
- Coverage   21.65%   21.12%   -0.53%     
==========================================
  Files          72       74       +2     
  Lines        6586     6760     +174     
==========================================
+ Hits         1426     1428       +2     
- Misses       4974     5146     +172     
  Partials      186      186

Impacted Files	Coverage Δ
pkg/cluster-runtime/apps.go	`0.00% <0.00%> (ø)`
pkg/cluster-runtime/installer.go	`0.00% <0.00%> (ø)`
pkg/cluster-runtime/scale.go	`0.00% <0.00%> (ø)`
pkg/imagedistributor/scp_distributor.go	`0.00% <0.00%> (ø)`
pkg/imagedistributor/mount.go	`2.77% <20.00%> (ø)`
pkg/infradriver/ssh_infradriver.go	`38.91% <100.00%> (ø)`

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

starnop · 2022-11-12T11:50:09Z

pkg/cluster-runtime/apps.go

-		clusterRootfs = i.infraDriver.GetClusterRootfsPath()
-		ex            = shell.NewLex('\\')
+		cmds    []string
+		appPath = i.infraDriver.GetClusterRootfsPath()


It looks like we can keep use clusterRootfs.

And In my understanding, we may need a new appRootPath variable, and it's going to be filepath.Join(clusterRootfs,"applications")

yes, we should launch app in their own workspace, thats why i rename this value from "clusterRootfs" to "appRootPath". later we only need to update this func to support it directly.

starnop · 2022-11-12T11:52:05Z

pkg/imagedistributor/mount.go

 	imagecommon "github.com/sealerio/sealer/pkg/define/options"
 	"github.com/sealerio/sealer/pkg/imageengine"
 	v1 "github.com/sealerio/sealer/types/api/v1"
 	osi "github.com/sealerio/sealer/utils/os"
 	"github.com/sealerio/sealer/utils/os/fs"
 )

+const (
+	mountBaseDir = "/var/lib/sealer/data/mount"


move it to. common/common.go file?

mountBaseDir is internal used , so i dont move it to pubic common.

starnop · 2022-11-12T11:54:04Z

pkg/imagedistributor/mount.go

-	if _, err := b.imageEngine.CreateWorkingContainer(&imagecommon.BuildRootfsOptions{
+	// make sure base mount Dir is existed.
+	err := fs.FS.MkdirAll(filepath.Dir(mountDir))
+	if err != nil {


consider the following syntax to avoid widening the scope of error

if err := fs.FS.MkdirAll(filepath.Dir(mountDir));err!=nil{}

starnop · 2022-11-12T11:56:08Z

pkg/imagedistributor/mount.go

 	for _, info := range imageMountInfo {
 		err := c.Mounter.Umount(info.MountDir)
 		if err != nil {
 			return fmt.Errorf("failed to umount %s:%v", info.MountDir, err)
 		}
 	}
+	// delete all mounted images
+	if err := fs.FS.RemoveAll(filepath.Join(mountBaseDir, formatImageName(imageName))); err != nil {


how about extracting a public method to return the mount root directory for the specified image name?

starnop · 2022-11-12T11:57:09Z

pkg/imagedistributor/mount.go

@@ -125,3 +139,7 @@ func NewImageMounter(imageEngine imageengine.Interface, hostsPlatform map[v1.Pla
 	c.Mounter = NewBuildAhMounter(imageEngine)
 	return c, nil
 }
+
+func formatImageName(name string) string {


add some unit tests and try to consider more scenarios

starnop · 2022-11-12T11:58:59Z

pkg/infradriver/ssh_infradriver.go

 }

 func (d *SSHInfraDriver) GetClusterBasePath() string {
-	return common.DefaultClusterBaseDir(d.clusterName)
+	return filepath.Join(common.DefaultSealerDataDir, d.clusterName)


starnop · 2022-11-12T11:59:19Z

pkg/infradriver/ssh_infradriver.go

@@ -337,11 +338,11 @@ func (d *SSHInfraDriver) GetHostsPlatform(hosts []net.IP) (map[v1.Platform][]net
 }

 func (d *SSHInfraDriver) GetClusterRootfsPath() string {
-	return common.DefaultTheClusterRootfsDir(d.clusterName)
+	return filepath.Join(common.DefaultSealerDataDir, d.clusterName, "rootfs")


why not keep it?

And I don't see a logical change

here i thought, the only way we get the cluster value such as "GetClusterRootfsPath" and the same kind parameters. not from common pkg directly .

And then, should we delete the function common.DefaultTheClusterRootfsDir?

starnop · 2022-11-12T12:01:28Z

pkg/cluster-runtime/apps.go

-	applicationFile := common.GetDefaultApplicationFile()
+func (i AppInstaller) save(applicationFile string) error {
+	f, err := os.OpenFile(applicationFile, os.O_RDWR|os.O_APPEND|os.O_CREATE, 0600)
+	defer func() {


use defer f.Close() is OK

golang lint will check the func return error ,to make sure each error was handled . so i use defer func()

starnop · 2022-11-12T12:06:37Z

pkg/cluster-runtime/apps.go

 	}

-	extensionList = append(extensionList, i.extension)
-	content, err := json.Marshal(extensionList)
+	content, err := json.Marshal(i.extension)


user defer func to unlock file locker here

starnop · 2022-11-12T12:09:09Z

pkg/cluster-runtime/apps.go

 	if err != nil {
 		return fmt.Errorf("failed to marshal image extension: %v", err)
 	}

-	return osutils.NewCommonWriter(applicationFile).WriteFile(content)
+	_, err = f.Write(content)
+	if err != nil {


unify use
if err:=func();err!=nil{}

please check the whole picture for yourself

starnop · 2022-11-12T12:11:19Z

pkg/cluster-runtime/apps.go

 	if err != nil {
 		return fmt.Errorf("failed to marshal image extension: %v", err)
 	}

-	return osutils.NewCommonWriter(applicationFile).WriteFile(content)
+	_, err = f.Write(content)


I find these codes changed the existing logic and the old logic will append the extensions from the old applicationFile.

Cloud you please make a simple explanation?

Currently ,it's also record app installations with append option, just separated from each installation record.

add image name to mount dir path add flock to application.json

kakaZhou719 force-pushed the detach-raw-cluster-image branch from 8d9c883 to 7dc802d Compare November 11, 2022 07:02

kakaZhou719 force-pushed the detach-raw-cluster-image branch from 7dc802d to 9bbd571 Compare November 11, 2022 07:13

kakaZhou719 changed the title ~~add image name to mount dir path~~ sealer run-app supports concurrent run Nov 11, 2022

jsparter mentioned this pull request Nov 11, 2022

detach the raw image data from rootfs #1846

Closed

starnop reviewed Nov 12, 2022

View reviewed changes

github-actions bot added the test label Nov 14, 2022

kakaZhou719 force-pushed the detach-raw-cluster-image branch from 3cc21a1 to 996f159 Compare November 14, 2022 06:56

starnop approved these changes Nov 15, 2022

View reviewed changes

opt: detach raw cluster image file

14d591a

add image name to mount dir path add flock to application.json

kakaZhou719 force-pushed the detach-raw-cluster-image branch from 996f159 to 14d591a Compare November 15, 2022 02:53

starnop merged commit 145eb4c into sealerio:main Nov 15, 2022

Stevent-fei pushed a commit to Stevent-fei/sealer that referenced this pull request Nov 17, 2022

opt: detach raw cluster image file (sealerio#1850)

8e272ee

add image name to mount dir path add flock to application.json

kakaZhou719 deleted the detach-raw-cluster-image branch June 28, 2023 09:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sealer run-app supports concurrent run #1850

sealer run-app supports concurrent run #1850

kakaZhou719 commented Nov 11, 2022 •

edited

Loading

codecov-commenter commented Nov 11, 2022 •

edited

Loading

starnop Nov 12, 2022

kakaZhou719 Nov 14, 2022

starnop Nov 12, 2022

kakaZhou719 Nov 14, 2022

starnop Nov 12, 2022

starnop Nov 12, 2022

starnop Nov 12, 2022

starnop Nov 12, 2022 •

edited

Loading

starnop Nov 12, 2022 •

edited

Loading

kakaZhou719 Nov 14, 2022

starnop Nov 15, 2022

starnop Nov 12, 2022

kakaZhou719 Nov 14, 2022

starnop Nov 12, 2022

starnop Nov 12, 2022

starnop Nov 12, 2022

kakaZhou719 Nov 14, 2022

sealer run-app supports concurrent run #1850

sealer run-app supports concurrent run #1850

Conversation

kakaZhou719 commented Nov 11, 2022 • edited Loading

Describe what this PR does / why we need it

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

codecov-commenter commented Nov 11, 2022 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

starnop Nov 12, 2022 • edited Loading

Choose a reason for hiding this comment

starnop Nov 12, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kakaZhou719 commented Nov 11, 2022 •

edited

Loading

codecov-commenter commented Nov 11, 2022 •

edited

Loading

starnop Nov 12, 2022 •

edited

Loading

starnop Nov 12, 2022 •

edited

Loading