Add sealer alpha mount command #1848

Stevent-fei · 2022-11-10T06:49:43Z

Describe what this PR does / why we need it

add sealer alpha mount command and support check image file
we can go to the mount directory to check the mirrored files, which can ensure that the kubefile image is built correctly.
#1742

support sealer alpha mount imageName/imageId
support sealer alpha umount imageNmae/imageId
support sealer alpha mount(view mount list)

Does this pull request fix one issue?

Describe how you did it

Describe how to verify it

Special notes for reviews

kakaZhou719 · 2022-11-10T06:59:07Z

cmd/sealer/cmd/alpha/create-rootfs.go

+
+func NewCreateRootfsCmd() *cobra.Command {
+	createRootfsCmd := &cobra.Command{
+		Use:     "create-rootfs",


how about named: sealer alpha mount my-image /tmp

kakaZhou719 · 2022-11-10T07:00:18Z

cmd/sealer/cmd/alpha/create-rootfs.go

+var longCreateRootfsCmdDescription = `Check the image file to ensure that the Kubefile image is constructed correctly.`
+
+var exampleForCreateRootfsCmd = `
+  sealer alpha create-rootfs [image name or id]


pls set more specific example

kakaZhou719 · 2022-11-10T07:00:57Z

cmd/sealer/cmd/alpha/create-rootfs.go

+			if err != nil {
+				return err
+			}
+			infraDriver, err := infradriver.NewInfraDriver(cluster)


why we need infraDriver? only use imageengine to do mount.

kakaZhou719 · 2022-11-10T07:01:19Z

cmd/sealer/cmd/alpha/create-rootfs.go

+`
+
+func NewCreateRootfsCmd() *cobra.Command {
+	createRootfsCmd := &cobra.Command{


we also need umount sub command.

kakaZhou719 · 2022-11-10T07:01:40Z

cmd/sealer/cmd/alpha/create-rootfs.go

+	"github.com/spf13/cobra"
+)
+
+var longCreateRootfsCmdDescription = `Check the image file to ensure that the Kubefile image is constructed correctly.`


VinceCui · 2022-11-10T09:17:00Z

cmd/sealer/cmd/alpha/mount.go

+		Example: exampleForMountCmd,
+		RunE: func(cmd *cobra.Command, args []string) error {
+			defaultPlatform := platform.GetDefaultPlatform()
+			path := defaultPlatform.OS + "_" + defaultPlatform.Architecture + "_" + defaultPlatform.Variant


Use a common function to gen path.

kakazhou is modify the mount dir, if his work done, the path should be different?

The specified installation directory is used here and is not loaded back to the mount directory

kakaZhou719 · 2022-11-11T07:15:49Z

cmd/sealer/cmd/alpha/umount.go

+		Example: exampleForUmountCmd,
+		Args:    cobra.ExactArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+


Useless Blank

kakaZhou719 · 2022-11-11T07:16:27Z

cmd/sealer/cmd/alpha/mount.go

+		Short:   "mount image file",
+		Long:    longMountCmdDescription,
+		Example: exampleForMountCmd,
+		Args:    cobra.ExactArgs(2),


Args: cobra.MinimumNArgs(1),

kakaZhou719 · 2022-11-11T07:16:44Z

cmd/sealer/cmd/alpha/mount.go

+				return err
+			}
+			if _, err := imageEngine.CreateWorkingContainer(&imagecommon.BuildRootfsOptions{
+				DestDir:       args[1] + "/rootfs",


we do not modify the DestDir

kakaZhou719 · 2022-11-11T07:17:58Z

cmd/sealer/cmd/alpha/mount.go

+				return err
+			}
+			if _, err := imageEngine.CreateWorkingContainer(&imagecommon.BuildRootfsOptions{
+				DestDir:       args[1] + "/rootfs",


You need to determine in advance whether args[1] is valid path address.

kakaZhou719 · 2022-11-11T07:21:36Z

cmd/sealer/cmd/alpha/mount.go

+	"github.com/spf13/cobra"
+)
+
+var longMountCmdDescription = `mount image file dir`


A more detailed description is needed if we write longMountCmdDescription

codecov-commenter · 2022-11-11T07:26:32Z

Codecov Report

Base: 21.11% // Head: 21.70% // Increases project coverage by +0.58% 🎉

Coverage data is based on head (edf57f5) compared to base (fbd817f).
Patch coverage: 40.00% of modified lines in pull request are covered.

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

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1848      +/-   ##
==========================================
+ Coverage   21.11%   21.70%   +0.58%     
==========================================
  Files          74       78       +4     
  Lines        6762     7055     +293     
==========================================
+ Hits         1428     1531     +103     
- Misses       5148     5322     +174     
- Partials      186      202      +16

Impacted Files	Coverage Δ
build/kubefile/parser/kubefile.go	`53.44% <0.00%> (-1.91%)`	⬇️
cmd/sealer/cmd/utils/cluster.go	`63.54% <ø> (ø)`
cmd/sealer/cmd/utils/hosts.go	`84.21% <ø> (ø)`
cmd/sealer/cmd/utils/validate.go	`38.88% <ø> (ø)`
pkg/cluster-runtime/hook.go	`3.75% <ø> (-0.05%)`	⬇️
pkg/image/reference/util.go	`83.78% <ø> (ø)`
pkg/clusterfile/clusterfile.go	`28.45% <14.89%> (-5.99%)`	⬇️
build/kubefile/parser/utils.go	`60.75% <53.68%> (-10.67%)`	⬇️
build/kubefile/parser/app_handler.go	`47.36% <100.00%> (-2.03%)`	⬇️
... and 5 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.

kakaZhou719 · 2022-11-16T04:01:30Z

cmd/sealer/cmd/alpha/umount.go

+				return err
+			}
+			mounter := imagedistributor.NewBuildAhMounter(imageEngine)
+			if err = mounter.Umount(args[0]); err != nil {


use imageengine to do umount

kakaZhou719 · 2022-11-16T04:02:05Z

cmd/sealer/cmd/alpha/mount.go

+			}); err != nil {
+				return err
+			}
+			return nil


show user the specific path via fmt.println

mount cluster image %s to %s successful

kakaZhou719 · 2022-11-16T04:02:58Z

cmd/sealer/cmd/alpha/mount.go

+			if err != nil {
+				return err
+			}
+			if exists {


if we need virtual path for mount, we should tell user via examples.

kakaZhou719 · 2022-11-16T04:04:05Z

cmd/sealer/cmd/alpha/mount.go

+			if len(args) < 2 {
+				return fmt.Errorf("please enter full parameters")
+			}
+			exists, err := mount.PathExists(args[1])


use utils os pkg IsFileExist

kakaZhou719 · 2022-11-16T10:17:49Z

cmd/sealer/cmd/alpha/mount.go

+			if len(args) < 2 {
+				return fmt.Errorf("please enter full parameters")
+			}
+			path, err := filepath.Abs(args[1])


kakaZhou719 · 2022-11-16T10:18:10Z

cmd/sealer/cmd/alpha/mount.go

+			if err != nil {
+				return err
+			}
+			if os.IsFileExist(path) {


use one line to check

kakaZhou719 · 2022-11-16T10:19:37Z

cmd/sealer/cmd/alpha/mount.go

+			}); err != nil {
+				return err
+			}
+			return nil


mount cluster image %s to %s successful

kakaZhou719 · 2022-11-16T10:22:10Z

cmd/sealer/cmd/alpha/umount.go

+				return err
+			}
+			if err := imageEngine.RemoveContainer(&imagecommon.RemoveContainerOptions{
+				ContainerNamesOrIDs: nil,


dangerous ops: this will delete all running containers.

kakaZhou719 · 2022-11-29T02:42:57Z

sealer alpha mount registry.cn-qingdao.aliyuncs.com/sealer-io/kubernetes:v1.22.15
sealer alpha mount : if no cluster image specified ,will show all mounted cluster image with its absolutely dir name.
sealer alpha umount /var/lib/sealer/data/overlay2/${ID}, will umount this cluster image.
sealer alpha umount --all

kakaZhou719 · 2022-12-02T09:17:16Z

cmd/sealer/cmd/alpha/umount.go

+	for _, image := range images {
+		for _, n := range image.Names {
+			if n == imageName {
+				cid = image.ID


just return string

kakaZhou719 · 2022-12-02T09:17:44Z

cmd/sealer/cmd/alpha/umount.go

+
+func removeContainerDir(image storage.Image, file fs.FileInfo, imageName string) error {
+	for _, n := range image.Names {
+		if n == imageName {


if n == imageName && file.Name() == image.ID {return os.RemoveAll}

Stevent-fei · 2022-12-07T06:50:55Z

starnop · 2022-12-19T08:38:01Z

@Stevent-fei Cloud please add some demo case as the description of this PR?

starnop · 2022-12-19T08:41:28Z

cmd/sealer/cmd/alpha/mount.go

+)
+
+var longMountCmdDescription = `
+mount the cluster image to '/var/lib/sealer/data/mountdir' the directory and check whether the contents of the build image and rootfs are consistent in advance


I think we should make mount dir configurable

I think we should make mount dir configurable

@starnop I think your suggestion is very good, but mount directory configurability needs to consider more parameters, how about setting it to the default directory /var/lib/sealer/data/overlay2?

starnop · 2022-12-19T08:42:58Z

cmd/sealer/cmd/alpha/mount.go

+			}
+
+			//output mount list
+			if len(args) == 0 {


add a new function for listing images

we should make the main logic clean

Stevent-fei · 2023-01-12T07:45:30Z

cmd/sealer/cmd/alpha/mount.go

+)
+
+var longMountCmdDescription = `
+mount the cluster image to '/var/lib/sealer/data/overlay2' the directory and check whether the contents of the build image and rootfs are consistent in advance


Can we only mount to this path? Is it possible to specify the mount path?

@starnop Yes, mount to the default path; users don't need to care about the mount path, similar to the image file in containerd

starnop · 2023-01-12T07:39:04Z

cmd/sealer/cmd/alpha/umount.go

+		RunE: func(cmd *cobra.Command, args []string) error {
+			var cid string
+			var imgName string
+


Please pre-check the args here. Otherwise, errors may be reported in the subsequent flow.

starnop · 2023-01-12T07:41:24Z

pkg/imageengine/buildah/from.go

 	"github.com/containers/buildah"
 	"github.com/containers/buildah/define"
 	buildahcli "github.com/containers/buildah/pkg/cli"
 	"github.com/containers/buildah/pkg/parse"
 	"github.com/containers/common/pkg/config"
 	encconfig "github.com/containers/ocicrypt/config"
+


delete unused blank

starnop · 2023-01-12T07:43:37Z

cmd/sealer/cmd/alpha/mount.go

+				for _, name := range i.Names {
+					if name == args[0] || strings.Contains(i.ID, args[0]) {
+						imageID = i.ID
+						path = filepath.Join(common.DefaultLayerDir, imageID)


make DefaultLayerDir configurable

And I think the default mount dir should not equal with the DefaultLayerDir to prevent us from damaging an installed image.

starnop · 2023-01-12T07:53:54Z

cmd/sealer/cmd/alpha/mount.go

+	"github.com/sealerio/sealer/pkg/imageengine/buildah"
+)
+
+var (


why defined as global variable

starnop · 2023-01-12T08:00:58Z

cmd/sealer/cmd/alpha/umount.go

+		Example: exampleForUmountCmd,
+		Args:    cobra.MaximumNArgs(1),
+		RunE: func(cmd *cobra.Command, args []string) error {
+			var cid string


s/cid/containerID

starnop

LGTM

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

Stevent-fei changed the title ~~Add sealer alpha command~~ Add sealer alpha create-rootfs command Nov 10, 2022

kakaZhou719 reviewed Nov 10, 2022

View reviewed changes

Stevent-fei changed the title ~~Add sealer alpha create-rootfs command~~ Add sealer alpha mount command Nov 10, 2022

VinceCui reviewed Nov 10, 2022

View reviewed changes

Stevent-fei force-pushed the support_multiple_app branch from 284285d to 101be51 Compare November 10, 2022 10:15

kakaZhou719 reviewed Nov 11, 2022

View reviewed changes

Stevent-fei changed the title ~~Add sealer alpha mount command~~ [WIP]Add sealer alpha mount command Nov 15, 2022

Stevent-fei force-pushed the support_multiple_app branch 2 times, most recently from dc6ecc0 to ad09883 Compare November 16, 2022 03:50

Stevent-fei changed the title ~~[WIP]Add sealer alpha mount command~~ Add sealer alpha mount command Nov 16, 2022

kakaZhou719 reviewed Nov 16, 2022

View reviewed changes

Stevent-fei force-pushed the support_multiple_app branch from 6877f1e to 5191d56 Compare November 16, 2022 07:37

kakaZhou719 reviewed Nov 16, 2022

View reviewed changes

github-actions bot added area/doc ImageBuilding related to all staff with image building labels Nov 17, 2022

Stevent-fei force-pushed the support_multiple_app branch from 51ad922 to 615a23e Compare November 17, 2022 10:05

github-actions bot removed area/doc ImageBuilding related to all staff with image building labels Nov 17, 2022

Stevent-fei force-pushed the support_multiple_app branch 2 times, most recently from f6f04c5 to b3696cc Compare November 22, 2022 05:33

Stevent-fei changed the title ~~Add sealer alpha mount command~~ [WIP]Add sealer alpha mount command Nov 30, 2022

Stevent-fei force-pushed the support_multiple_app branch 3 times, most recently from b92d915 to eff42b9 Compare November 30, 2022 09:03

Stevent-fei changed the title ~~[WIP]Add sealer alpha mount command~~ Add sealer alpha mount command Nov 30, 2022

Stevent-fei force-pushed the support_multiple_app branch from beacaa9 to 82cbad8 Compare November 30, 2022 10:10

Stevent-fei force-pushed the support_multiple_app branch from 82cbad8 to b63060d Compare December 1, 2022 07:05

kakaZhou719 reviewed Dec 2, 2022

View reviewed changes

Stevent-fei force-pushed the support_multiple_app branch 2 times, most recently from 9a0b28c to 7599094 Compare December 6, 2022 05:38

Stevent-fei force-pushed the support_multiple_app branch 2 times, most recently from edf57f5 to 4acc83c Compare December 7, 2022 12:47

Stevent-fei force-pushed the support_multiple_app branch from 4acc83c to 362e5f4 Compare December 19, 2022 05:49

starnop reviewed Dec 19, 2022

View reviewed changes

Stevent-fei force-pushed the support_multiple_app branch from 44e2007 to e578c51 Compare December 19, 2022 09:28

Stevent-fei force-pushed the support_multiple_app branch 2 times, most recently from b1c40a6 to b259d60 Compare January 11, 2023 03:15

github-actions bot removed the test label Jan 11, 2023

starnop reviewed Jan 12, 2023

View reviewed changes

Stevent-fei force-pushed the support_multiple_app branch from d0dc148 to a521add Compare January 12, 2023 08:39

Add sealer alpha mount and umount command

9900a90

Stevent-fei force-pushed the support_multiple_app branch from a521add to 9900a90 Compare January 16, 2023 02:58

starnop approved these changes Jan 16, 2023

View reviewed changes

starnop merged commit f816172 into sealerio:main Jan 16, 2023

Add sealer alpha mount command #1848

Add sealer alpha mount command #1848

Conversation

Stevent-fei commented Nov 10, 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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kakaZhou719 commented Nov 29, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Stevent-fei commented Dec 7, 2022

starnop commented Dec 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

starnop Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

starnop Jan 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

starnop left a comment

Choose a reason for hiding this comment

Stevent-fei commented Nov 10, 2022 •

edited

Loading

codecov-commenter commented Nov 11, 2022 •

edited

Loading

kakaZhou719 commented Nov 29, 2022 •

edited

Loading

starnop Jan 12, 2023 •

edited

Loading

starnop Jan 12, 2023 •

edited

Loading