Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat:Mount by API without makeConfig #1610

Merged
merged 1 commit into from
Aug 30, 2024

Conversation

fappy1234567
Copy link
Contributor

Details

Implement mounting through mountByAPI to evaluate its effectiveness
Omit the use of makeConfig

@fappy1234567 fappy1234567 requested a review from a team as a code owner August 19, 2024 03:53
@fappy1234567 fappy1234567 requested review from bergwolf, adamqqqplay and hsiangkao and removed request for a team August 19, 2024 03:53
Copy link

codecov bot commented Aug 19, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.38%. Comparing base (52ed07b) to head (6043273).
Report is 2 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1610      +/-   ##
==========================================
- Coverage   61.44%   61.38%   -0.06%     
==========================================
  Files         145      146       +1     
  Lines       48032    48040       +8     
  Branches    46007    46007              
==========================================
- Hits        29512    29490      -22     
- Misses      16963    16987      +24     
- Partials     1557     1563       +6     

see 9 files with indirect coverage changes

@Desiki-high
Copy link
Member

Please add commit Signed-off info, such as using git commit -s -m "xxxx" to commit your codes.

@fappy1234567 fappy1234567 force-pushed the mount-by-api branch 3 times, most recently from 5984d8a to 2b7539d Compare August 21, 2024 04:07
smoke/Makefile Outdated Show resolved Hide resolved
smoke/Makefile Outdated Show resolved Hide resolved
@fappy1234567 fappy1234567 force-pushed the mount-by-api branch 11 times, most recently from c27f8f6 to b811523 Compare August 21, 2024 07:46
smoke/Makefile Outdated
test: build
golangci-lint run
sudo -E ./smoke.test -test.v -test.timeout 10m -test.parallel=16 -test.run=$(TESTS)
sudo -E ./smoke.test -test.v -test.timeout 10m -test.parallel=16 -test.run=TestNativeLayer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why modify here.

ctx.PrepareWorkDir(t)
defer ctx.Destroy(t)

rootFs := texture.MakeLowerLayer(t, filepath.Join(ctx.Env.WorkDir, "root-fs"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

"rootfs"

f, err := os.Open(config.ConfigPath)
if err != nil {
return err
var tpl *template.Template
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why we modify this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to remove the makeConfig function so that the API can be used directly instead of first writing to a file and then using the API.

@@ -365,6 +371,23 @@ func (nydusd *Nydusd) Umount() error {
return nil
}

func (nydusd *Nydusd) UmountByAPI(point string) error {
path := point
// path: /mount1
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove useless comments.

@@ -365,6 +371,23 @@ func (nydusd *Nydusd) Umount() error {
return nil
}

func (nydusd *Nydusd) UmountByAPI(point string) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

point -> subPath

@@ -365,6 +371,23 @@ func (nydusd *Nydusd) Umount() error {
return nil
}

func (nydusd *Nydusd) UmountByAPI(point string) error {
path := point
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove useless assignment.

nydusd.VerifyByPath(t, expectedFileTree, nydusd.MountPath)
}

func (nydusd *Nydusd) VerifyByPath(t *testing.T, expectedFileTree map[string]*File, point string) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

point -> subPath

@@ -622,17 +645,20 @@ func (nydusd *Nydusd) GetInflightMetrics() (*InflightMetrics, error) {
}

func (nydusd *Nydusd) Verify(t *testing.T, expectedFileTree map[string]*File) {
nydusd.VerifyByPath(t, expectedFileTree, nydusd.MountPath)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the misunderstand, we should call nydusd.VerifyByPath(t, expectedFileTree, "") here.

nydusd.VerifyByPath(t, expectedFileTree, nydusd.MountPath)
}

func (nydusd *Nydusd) VerifyByPath(t *testing.T, expectedFileTree map[string]*File, point string) {
actualFiles := map[string]*File{}
err := filepath.WalkDir(nydusd.MountPath, func(path string, _ fs.DirEntry, err error) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

nydusd.MountPath ->

mountPath := filepath.Join(nydusd.MountPath, subPath)

@fappy1234567 fappy1234567 force-pushed the mount-by-api branch 2 times, most recently from ec35c6b to 90feab4 Compare August 22, 2024 00:36
@fappy1234567 fappy1234567 changed the title WIP:Attempt to mount using mountBYAPI without makeConfig Feat:Mount by API without makeConfig Aug 22, 2024
if targetPath == "." || targetPath == ".." {
pointMdr := strings.TrimPrefix(subPath, "/")
targetPath = strings.TrimPrefix(targetPath, pointMdr+"/")
if targetPath == "." || targetPath == ".." || targetPath == pointMdr {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why need to check targetPath == pointMdr?

switch tplType {
case NydusdConfigTpl:
tpl = template.Must(template.New("").Parse(configTpl))
case NydusdOvlConfigTpl:
Copy link
Collaborator

Choose a reason for hiding this comment

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

But the case path is not used.

@fappy1234567 fappy1234567 force-pushed the mount-by-api branch 5 times, most recently from b043cfc to 6135f08 Compare August 30, 2024 06:13
Copy link
Collaborator

@imeoer imeoer left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@imeoer imeoer merged commit 114ec88 into dragonflyoss:master Aug 30, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants