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: OverlayFS -> AUFS whiteout conversion #55

Merged
merged 2 commits into from
Jun 4, 2024
Merged

Conversation

dtrudg
Copy link
Member

@dtrudg dtrudg commented May 31, 2024

Allow OverlayFS whiteouts to be converted to AUFS, by filtering a tar stream.

In this direction the conversion is a single pass, single operation. There is no requirement to pre-scan the tar.

Close #53

@dtrudg dtrudg self-assigned this May 31, 2024
@dtrudg dtrudg force-pushed the issue53 branch 3 times, most recently from a3b4797 to f808c64 Compare June 3, 2024 08:21
@dtrudg dtrudg changed the title WIP: OverlayFS -> AUFS whiteout conversion feat: OverlayFS -> AUFS whiteout conversion Jun 3, 2024
Allow OverlayFS whiteouts to be converted to AUFS, by filtering a tar
stream.

In this direction the conversion is a single pass, single operation.
There is no requirement to pre-scan the tar.

Close sylabs#53
@dtrudg dtrudg marked this pull request as ready for review June 3, 2024 08:40
@dtrudg dtrudg requested review from wobito and tri-adam June 3, 2024 08:40
Copy link
Member

@tri-adam tri-adam left a comment

Choose a reason for hiding this comment

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

LGTM. Just one observation, the round trip exposes that some of the TAR entries end up getting Cleaned, while others do not. I'm not aware of any specific issue this causes, but thought I'd raise it just in case you can think of any?

Specifically, the input in your test case is:

$ gunzip -c test/images/aufs-docker-v2-manifest/blobs/sha256/da55812559dec81445c289c3832cee4a2f725b15aeb258791640185c3126b2bf | tar tf - 
./
./dir/
./dir/.wh..wh..opq
./.wh.file

When converted to OverlayFS, you end up with the whited out file trimmed of its ./:

$ tar tf 'pkg/mutate/testdata/Test_WhiteoutRoundTrip/overlayfs.golden'
./
./dir/
file

And then of course when you go back the other direction, both whiteout files have also been trimmed of their leading ./.

$ tar tf 'pkg/mutate/testdata/Test_WhiteoutRoundTrip/aufs.golden'
./
./dir/
dir/.wh..wh..opq
.wh.file

@dtrudg
Copy link
Member Author

dtrudg commented Jun 4, 2024

LGTM. Just one observation, the round trip exposes that some of the TAR entries end up getting Cleaned, while others do not. I'm not aware of any specific issue this causes, but thought I'd raise it just in case you can think of any?

Right - they are equivalent, but yeah.. it would be nicer for comparison purposes if the ./ is preserved. Let me look at that.

@dtrudg
Copy link
Member Author

dtrudg commented Jun 4, 2024

@tri-adam - please take a look at 580119f

09:36 am $ gunzip -c test/images/aufs-docker-v2-manifest/blobs/sha256/da55812559dec81445c289c3832cee4a2f725b15aeb258791640185c3126b2bf | tar tf -
./
./dir/
./dir/.wh..wh..opq
./.wh.file

09:36 am $ tar tf pkg/mutate/testdata/Test_WhiteoutRoundTrip/overlayfs.golden 
./
./dir/
./file

09:36 am $ tar tf pkg/mutate/testdata/Test_WhiteoutRoundTrip/aufs.golden 
./
./dir/
./dir/.wh..wh..opq
./.wh.file

Prior to this PR, when a tar file contains an entry prefixed with `./`
that is processed by a whiteout transformation, the `./` prefix is lost.

This loss of the prefix was due to the use of `filepath` functions that
clean the resulting path. Instead, use `filepath.Split` and manual
concatenation with filepath.Seperator` to avoid the clean.
Copy link
Member

@tri-adam tri-adam left a comment

Choose a reason for hiding this comment

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

LGTM

@dtrudg dtrudg merged commit 12236ae into sylabs:main Jun 4, 2024
10 checks passed
@dtrudg dtrudg deleted the issue53 branch June 4, 2024 16:39
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.

Whiteout conversion OverlayFS - > AUFS
2 participants