fix mount and umount bug #2029

Stevent-fei · 2023-02-14T04:57:28Z

Describe what this PR does / why we need it

fix sealer alpha mount and sealer alpha umount bug

sealer alpha mount ex:

Mount image name：
    sealer alpha mount imageName

Mount image id：
    sealer alpha mount ba15e47f5969

sealer alpha umount ex:

Umount image containerID：
    sealer alpha umount ba15e47f5969

Uninstall all images:
    sealer alpha umount --all

Does this pull request fix one issue?

Fixes #2000

Describe how you did it

Describe how to verify it

Special notes for reviews

codecov-commenter · 2023-02-14T05:04:31Z

Codecov Report

Patch coverage has no change and project coverage change: +0.89 🎉

Comparison is base (f983d9b) 18.40% compared to head (a4cfea4) 19.29%.

❗ Current head a4cfea4 differs from pull request most recent head cad1aa6. Consider uploading reports for the commit cad1aa6 to get more accurate results

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2029      +/-   ##
==========================================
+ Coverage   18.40%   19.29%   +0.89%     
==========================================
  Files         101       96       -5     
  Lines        9201     8985     -216     
==========================================
+ Hits         1693     1734      +41     
+ Misses       7277     7026     -251     
+ Partials      231      225       -6

Impacted Files	Coverage Δ
pkg/imageengine/buildah/commit.go	`0.00% <0.00%> (ø)`
pkg/imageengine/buildah/common.go	`0.00% <0.00%> (ø)`
pkg/imageengine/buildah/config.go	`0.00% <0.00%> (ø)`
pkg/imageengine/buildah/copy.go	`0.00% <0.00%> (ø)`
pkg/imageengine/buildah/images.go	`0.00% <0.00%> (ø)`
pkg/imageengine/buildah/mount.go	`0.00% <0.00%> (ø)`
pkg/imageengine/buildah/remove_container.go	`0.00% <0.00%> (ø)`
pkg/application/v2app.go	`29.34% <0.00%> (-24.94%)`	⬇️
pkg/infradriver/ssh_infradriver.go	`31.32% <0.00%> (-3.64%)`	⬇️
build/kubefile/parser/kubefile.go	`33.33% <0.00%> (-3.49%)`	⬇️
... and 34 more

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 · 2023-02-21T03:48:45Z

cmd/sealer/cmd/alpha/mount.go

 `

+const (
+	containerID = "CONTAINER ID"


add table prefix

tableHeaderContainerID

starnop · 2023-02-21T03:49:29Z

cmd/sealer/cmd/alpha/mount.go

 			images, err := store.Images()
 			if err != nil {
 				return err
 			}

 			//output mount list
 			if len(args) == 0 {
-				if err := mountList(images); err != nil {
-					return err
+				table := tablewriter.NewWriter(common.StdOut)


extract the table writer as separate method

starnop · 2023-02-21T03:50:35Z

cmd/sealer/cmd/alpha/mount.go

-			if _, err = os.Stat(path); err == nil {
-				return fmt.Errorf("destination directionay %s exists, you should remove it first", path)
+					for _, container := range containers {
+						if builder.ContainerID == container.ID && mounted {


return fast, avoid large code depth

starnop · 2023-02-21T03:52:52Z

cmd/sealer/cmd/alpha/umount.go

-					if err := removeContainerDir(image, file, imgName); err != nil {
-						return err
+				for _, container := range containers {
+					if container.ImageID == imageID {


if container.ImageID != imageID{
continue
}

starnop · 2023-02-28T02:23:55Z

cmd/sealer/cmd/alpha/mount.go

+	return id
+}
+
+func contain(imageName string, images []string) bool {


string slice contains

starnop · 2023-02-28T02:26:05Z

cmd/sealer/cmd/alpha/mount.go

-				return err
+			for _, name := range args {
+				imageID := getImageID(images, name)
+				if ok := contain(imageID, imageIDList); !ok {


return fast

starnop · 2023-02-28T02:30:34Z

cmd/sealer/cmd/alpha/mount.go

+	return imgName, imageID
+}
+
+func getImageID(images []storage.Image, name string) string {


how to determine the current os arch

starnop · 2023-02-28T02:36:47Z

cmd/sealer/cmd/alpha/mount.go

+
+func contain(imageName string, images []string) bool {
+	for _, image := range images {
+		if imageName == image || strings.Contains(image, imageName) {


Contains/hasPrefix

kakaZhou719 · 2023-03-01T07:57:33Z

cmd/sealer/cmd/alpha/mount.go

+	tableHeaderContainerID = "CONTAINER ID"
+)
+
+type Mounter struct {


MountService

kakaZhou719 · 2023-03-01T07:58:03Z

cmd/sealer/cmd/alpha/mount.go


-			//too fast to mount.
-			time.Sleep(time.Second * 1)
+func NewMountInfo() (Mounter, error) {


NewMountService

kakaZhou719 · 2023-03-01T07:58:34Z

cmd/sealer/cmd/alpha/mount.go

 			if err != nil {
 				return err
 			}

 			//output mount list
 			if len(args) == 0 {
-				if err := mountList(images); err != nil {
+				if err := mountInfo.generateMountList(); err != nil {


kakaZhou719 · 2023-03-01T08:00:30Z

cmd/sealer/cmd/alpha/mount.go

-				return err
+func (m Mounter) generateMountList() error {
+	table := tablewriter.NewWriter(common.StdOut)
+	table.SetHeader([]string{imageName, tableHeaderContainerID, tableHeaderMountPath})


you should split table render from this func

kakaZhou719 · 2023-03-01T08:00:57Z

cmd/sealer/cmd/alpha/mount.go

+	return ""
+}
+
+func (m Mounter) mount(imageNameOrID []string) error {


Mount(imageNameOrID []string) error

kakaZhou719 · 2023-03-01T08:03:28Z

cmd/sealer/cmd/alpha/mount.go

+
+	for _, name := range imageNameOrID {
+		imageID := m.getImageID(name)
+		if ok := utilsstrings.IsInSlice(imageID, imageIDList); !ok {


ok := utilsstrings.IsInSlice(imageID, imageIDList)
if ok{
logrus.Infof("this image has already been mounted, please do not repeat the operation")
}
// do others

kakaZhou719 · 2023-03-01T08:09:16Z

cmd/sealer/cmd/alpha/umount.go

 				}
-				logrus.Infof("umount all cluster image successful")
+				logrus.Infof("umount all mount information was successful")


successful to umount all sealer image

kakaZhou719 · 2023-03-01T08:09:41Z

cmd/sealer/cmd/alpha/umount.go

-					imgName = args[0]
-				}
-			}
+func (m Mounter) umount(imageNameOrID []string) {


Umount(imageNameOrID []string)

kakaZhou719 · 2023-03-01T08:10:12Z

cmd/sealer/cmd/alpha/umount.go

-		for _, n := range image.Names {
-			if n == imageName {
-				return image.ID, nil
+func (m Mounter) umountAllContainers() error {


UmountAllContainers() error

kakaZhou719 · 2023-03-01T08:10:51Z

cmd/sealer/cmd/alpha/umount.go

 		RunE: func(cmd *cobra.Command, args []string) error {
-			var containerID string
-			var imgName string
-
 			if len(args) == 0 && !umountAll {
 				return fmt.Errorf("you must input imageName Or imageIp")


kakaZhou719 · 2023-03-01T08:13:45Z

cmd/sealer/cmd/alpha/umount.go

 			}
+			lastError = fmt.Errorf("error unmounting container %q: %w", builder.Container, err)


kakaZhou719 · 2023-03-01T08:13:58Z

cmd/sealer/cmd/alpha/umount.go

 			}
+			lastError = fmt.Errorf("error unmounting container %s: %w", name, err)


kakaZhou719 · 2023-03-01T08:14:09Z

cmd/sealer/cmd/alpha/umount.go

 			}
+			lastError = fmt.Errorf("error unmounting container %s: %w", name, err)


container ?

kakaZhou719 · 2023-03-01T08:15:03Z

cmd/sealer/cmd/alpha/umount.go

-				ContainerNamesOrIDs: []string{containerID}},
-			); err != nil {
-				return err
+		builder, err := buildah.OpenBuilder(context.TODO(), m.store, containerID)


what is builder? rename it to client ?

starnop · 2023-03-03T02:09:23Z

cmd/sealer/cmd/alpha/mount.go

+		ok := utilsstrings.IsInSlice(imageID, imageIDList)
+		if ok {
+			logrus.Infof("this image has already been mounted, please do not repeat the operation")
+		} else {


continue here directly

starnop · 2023-03-03T02:11:33Z

cmd/sealer/cmd/alpha/umount.go

-				}
+		if err = client.Unmount(); err != nil {
+			if lastError != nil {
+				fmt.Fprintln(os.Stderr, lastError)


return error here

kakaZhou719

LGTM

Stevent-fei force-pushed the fix_mount_bug branch from e9eb8f2 to 3b09379 Compare February 14, 2023 05:43

starnop reviewed Feb 21, 2023

View reviewed changes

Stevent-fei force-pushed the fix_mount_bug branch from 9e61c57 to 72cdb75 Compare February 23, 2023 06:09

Stevent-fei changed the title ~~fix mount and umount bug~~ [WIP]fix mount and umount bug Feb 27, 2023

Stevent-fei force-pushed the fix_mount_bug branch from 7b6bffc to 98d2004 Compare February 27, 2023 16:59

Stevent-fei changed the title ~~[WIP]fix mount and umount bug~~ fix mount and umount bug Feb 28, 2023

Stevent-fei force-pushed the fix_mount_bug branch from e3146d8 to d438bb1 Compare February 28, 2023 01:56

starnop reviewed Feb 28, 2023

View reviewed changes

Stevent-fei force-pushed the fix_mount_bug branch 2 times, most recently from 4918848 to d7242af Compare March 1, 2023 06:44

kakaZhou719 reviewed Mar 1, 2023

View reviewed changes

cmd/sealer/cmd/alpha/umount.go Outdated

}

lastError = fmt.Errorf("error unmounting container %s: %w", name, err)

Copy link

Member

kakaZhou719 Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

failed to

kakaZhou719 reviewed Mar 1, 2023

View reviewed changes

cmd/sealer/cmd/alpha/umount.go Outdated

}

lastError = fmt.Errorf("error unmounting container %s: %w", name, err)

Copy link

Member

kakaZhou719 Mar 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

container ?

kakaZhou719 reviewed Mar 1, 2023

View reviewed changes

Stevent-fei force-pushed the fix_mount_bug branch from 27bdd07 to bd2d1c4 Compare March 1, 2023 08:47

starnop reviewed Mar 3, 2023

View reviewed changes

Stevent-fei force-pushed the fix_mount_bug branch from 7cf2803 to 79262b6 Compare March 3, 2023 03:45

Stevent-fei force-pushed the fix_mount_bug branch 3 times, most recently from e779dee to 59035bf Compare March 7, 2023 03:49

fix mount and umount bug

cad1aa6

Stevent-fei force-pushed the fix_mount_bug branch from a4cfea4 to cad1aa6 Compare March 7, 2023 05:53

kakaZhou719 approved these changes Mar 7, 2023

View reviewed changes

kakaZhou719 merged commit 3d81283 into sealerio:main Mar 7, 2023

This was referenced Mar 8, 2023

[WeeklyReport] Weekly report for sealer 3/1/2023 to 3/8/2023 #2083

Closed

[WeeklyReport] Weekly report for sealer 3/6/2023 to 3/13/2023 #2098

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix mount and umount bug #2029

fix mount and umount bug #2029

Stevent-fei commented Feb 14, 2023 •

edited

Loading

codecov-commenter commented Feb 14, 2023 •

edited

Loading

starnop Feb 21, 2023

starnop Feb 21, 2023

starnop Feb 21, 2023

starnop Feb 21, 2023

starnop Feb 21, 2023

starnop Feb 28, 2023

starnop Feb 28, 2023

starnop Feb 28, 2023

starnop Feb 28, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

kakaZhou719 Mar 1, 2023

starnop Mar 3, 2023

starnop Mar 3, 2023

kakaZhou719 left a comment

		}
		lastError = fmt.Errorf("error unmounting container %q: %w", builder.Container, err)

fix mount and umount bug #2029

fix mount and umount bug #2029

Conversation

Stevent-fei commented Feb 14, 2023 • 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 Feb 14, 2023 • 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

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

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 left a comment

Choose a reason for hiding this comment

Stevent-fei commented Feb 14, 2023 •

edited

Loading

codecov-commenter commented Feb 14, 2023 •

edited

Loading