feat: support local tarball for nydusify copy #1612

BruceAko · 2024-08-21T08:58:58Z

Relevant Issue

#1592

Details

proposal:
https://www.yuque.com/chongzhi-rrdux/qn22ch/rolbzm0xe00g1a3b

implement:
https://www.yuque.com/chongzhi-rrdux/qn22ch/tgg8w0rcazmyoo8f

Types of changes

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation Update (if none of the other choices apply)

Checklist

I have updated the documentation accordingly.
I have added tests to cover my changes.

codecov · 2024-08-21T15:04:18Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.28%. Comparing base (6106fbc) to head (974b4a4).
Report is 4 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1612      +/-   ##
==========================================
- Coverage   61.29%   61.28%   -0.01%     
==========================================
  Files         146      146              
  Lines       48143    48143              
  Branches    46110    46110              
==========================================
- Hits        29509    29506       -3     
- Misses      17074    17075       +1     
- Partials     1560     1562       +2

see 2 files with indirect coverage changes

imeoer · 2024-08-22T01:34:13Z

contrib/nydusify/pkg/copier/copier.go

-	source := sourceNamed.String()
-	target := targetNamed.String()
+	var source string
+	if strings.HasPrefix(opt.Source, "file:///") {


Can we use url.Parse here?

When given an absolute path, if we use url.Parse, it incorrectly resolves it into URL.host instead of URL.path. So as an alternative, I've referenced helm and added a function getLocalPath(). A detailed description of this function is in the second commit.

imeoer · 2024-08-22T01:36:18Z

contrib/nydusify/pkg/copier/copier.go

+			return err
+		}
+		defer f.Close()
+		decompressor, err := compression.DecompressStream(f)


decompressor should be closed.

imeoer · 2024-08-22T01:47:51Z

contrib/nydusify/pkg/converter/provider/provider.go

+	}
+
+	if len(images) != 1 {
+		return "", errors.New("importing multiple images")


We should support multi platforms image.

The length of images array represents the number of images in the tarball, not manifests. As shown in the figure, there are two images airpine and nginx in the tarball, each of which is multi platform image storing multiple manifests (linux/amd64 and linux/arm64/v8).

Since all of our exported tarball will only contain 1 image and it's manifests, it can be assumed that if the length of images in the tarball is greater than 1, the tarball is not formatted correctly.

So maybe this code doesn't need to be changed?

Thanks for the detail, LGTM.

imeoer · 2024-08-22T01:53:39Z

Please add test case in smoke/tests/nydusify_copy_test.go, thanks!

imeoer · 2024-08-22T02:00:01Z

Also add a doc about nydusify copy in docs/nydusify.md. :)

BruceAko · 2024-09-04T08:26:10Z

Also add a doc about nydusify copy in docs/nydusify.md. :)

Done.

BruceAko · 2024-09-04T09:27:21Z

Please add test case in smoke/tests/nydusify_copy_test.go, thanks!

The E2E test process is designed as follows:

source is registry repository, target is local tarball, run nydusify copy to determine if the tarball was successfully exported;
source is the just-exported tarball, target is remote reposiroty, perform another nydusify copy to determine whether the tarball was successfully imported;
Run nydusify check to determine if the image which was just imported into reposiroty is correct.

BruceAko · 2024-09-11T01:36:57Z

Please add test case in smoke/tests/nydusify_copy_test.go, thanks!

Since smoke/tests/image_test.go already contained several test cases about nydusify copy, instead of creating a new file, I added a function testNydusifyCopy() to the image_test.go to refine the checking of the correctness of copy by different Nydus images.

Signed-off-by: BruceAko <[email protected]>

imeoer

LGTM! Thanks for the work.

BruceAko requested a review from a team as a code owner August 21, 2024 08:58

BruceAko requested review from imeoer, jiangliu and Desiki-high and removed request for a team August 21, 2024 08:58

imeoer reviewed Aug 22, 2024

View reviewed changes

BruceAko force-pushed the master branch from 3b61ebb to 4b8d9c1 Compare August 28, 2024 07:18

BruceAko changed the title ~~feat: support local tarball for nydusify copy~~ [WIP] feat: support local tarball for nydusify copy Aug 28, 2024

BruceAko force-pushed the master branch 4 times, most recently from 85d8fbe to 57740b6 Compare September 4, 2024 07:53

BruceAko force-pushed the master branch 7 times, most recently from 1c012cb to 269831f Compare September 10, 2024 15:38

BruceAko changed the title ~~[WIP] feat: support local tarball for nydusify copy~~ feat: support local tarball for nydusify copy Sep 11, 2024

BruceAko requested a review from imeoer September 11, 2024 08:57

BruceAko added 3 commits September 15, 2024 20:56

feat: support local tarball for nydusify copy

2be9aa0

Signed-off-by: BruceAko <[email protected]>

fix: add getLocalPath() and close decompressor

6011df0

Signed-off-by: BruceAko <[email protected]>

fix: add a doc about nydusify copy

3b4915b

Signed-off-by: BruceAko <[email protected]>

fix: add tests for nydusify copy

974b4a4

Signed-off-by: BruceAko <[email protected]>

imeoer force-pushed the master branch from 269831f to 974b4a4 Compare September 15, 2024 12:56

imeoer approved these changes Sep 15, 2024

View reviewed changes

imeoer merged commit 9b4c272 into dragonflyoss:master Sep 15, 2024
25 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: support local tarball for nydusify copy #1612

feat: support local tarball for nydusify copy #1612

BruceAko commented Aug 21, 2024 •

edited

Loading

codecov bot commented Aug 21, 2024 •

edited

Loading

imeoer Aug 22, 2024

BruceAko Aug 28, 2024 •

edited

Loading

imeoer Aug 22, 2024

BruceAko Aug 28, 2024

imeoer Aug 22, 2024

BruceAko Sep 4, 2024 •

edited

Loading

imeoer Sep 9, 2024

imeoer commented Aug 22, 2024

imeoer commented Aug 22, 2024

BruceAko commented Sep 4, 2024

BruceAko commented Sep 4, 2024 •

edited

Loading

BruceAko commented Sep 11, 2024

imeoer left a comment

feat: support local tarball for nydusify copy #1612

feat: support local tarball for nydusify copy #1612

Conversation

BruceAko commented Aug 21, 2024 • edited Loading

Relevant Issue

Details

Types of changes

Checklist

codecov bot commented Aug 21, 2024 • edited Loading

Codecov Report

imeoer Aug 22, 2024

Choose a reason for hiding this comment

BruceAko Aug 28, 2024 • edited Loading

Choose a reason for hiding this comment

imeoer Aug 22, 2024

Choose a reason for hiding this comment

BruceAko Aug 28, 2024

Choose a reason for hiding this comment

imeoer Aug 22, 2024

Choose a reason for hiding this comment

BruceAko Sep 4, 2024 • edited Loading

Choose a reason for hiding this comment

imeoer Sep 9, 2024

Choose a reason for hiding this comment

imeoer commented Aug 22, 2024

imeoer commented Aug 22, 2024

BruceAko commented Sep 4, 2024

BruceAko commented Sep 4, 2024 • edited Loading

BruceAko commented Sep 11, 2024

imeoer left a comment

Choose a reason for hiding this comment

BruceAko commented Aug 21, 2024 •

edited

Loading

codecov bot commented Aug 21, 2024 •

edited

Loading

BruceAko Aug 28, 2024 •

edited

Loading

BruceAko Sep 4, 2024 •

edited

Loading

BruceAko commented Sep 4, 2024 •

edited

Loading