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

{un,re}pack: support overlayfs whiteouts #342

Merged
merged 5 commits into from
Sep 22, 2020

Conversation

tych0
Copy link
Member

@tych0 tych0 commented Jul 7, 2020

Here's a couple of patches. The first is me cleaning up a mess I made, and the second one is adding overlay style whiteout support. See patch notes for details.

@cyphar
Copy link
Member

cyphar commented Jul 7, 2020

👍 on UnpackOptions, I found the exact same gaff yesterday and had put it in my backlog to fix so thanks for doing it. 😉

@tych0
Copy link
Member Author

tych0 commented Jul 7, 2020

Yeah, I should have done it a while ago :)

The tests are failing locally for me with:

Retrieving repository 'obs-tools' metadata [.error]
Repository 'obs-tools' is invalid.
[obs-tools|https://download.opensuse.org/repositories/devel:/tools/openSUSE_Leap_15.0] Valid metadata not found at specified URL
Please check if the URIs defined for this repository are pointing to a valid repository.
Skipping repository 'obs-tools' because of the above error.
Retrieving repository 'obs-vc' metadata [...

Automatically importing the following key:

  Repository:       obs-vc                                                        
  Key Name:         Virtualization OBS Project <Virtualization@build.opensuse.org>
  Key Fingerprint:  55A0B34D 49501BB7 CA474F5A A193FBB5 72174FC2                  
  Key Created:      Mon Feb 17 05:10:11 2020                                      
  Key Expires:      Wed Apr 27 05:10:11 2022                                      
  Rpm Name:         gpg-pubkey-72174fc2-5e4a2033                                  


.done]
Building repository 'obs-vc' cache [....done]
Retrieving repository 'openSUSE-Leap-15.0-Non-Oss' metadata [....done]
Building repository 'openSUSE-Leap-15.0-Non-Oss' cache [....done]
Retrieving repository 'openSUSE-Leap-15.0-Oss' metadata [.........done]
Building repository 'openSUSE-Leap-15.0-Oss' cache [....done]
Retrieving repository 'openSUSE-Leap-15.0-Update' metadata [........................done]
Building repository 'openSUSE-Leap-15.0-Update' cache [....done]
Retrieving repository 'openSUSE-Leap-15.0-Update-Non-Oss' metadata [....done]
Building repository 'openSUSE-Leap-15.0-Update-Non-Oss' cache [....done]
Some of the repositories have not been refreshed because of an error.
The command '/bin/sh -c zypper ar -f -p 10 -g obs://Virtualization:containers obs-vc && 	zypper ar -f -p 10 -g obs://devel:tools obs-tools && 	zypper ar -f -p 10 -g obs://devel:languages:go obs-go && zypper --gpg-auto-import-keys -n ref && 	zypper -n up' returned a non-zero code: 4
make: *** [Makefile:177: umociimage] Error 4

and I see they're failing with a different error in actual CI, so I'll try again tomorrow and see if they'll fail the same way for me.

@cyphar
Copy link
Member

cyphar commented Jul 8, 2020

The tests are failing locally because you have an old version of opensuse/leap:latest -- 15.0 is no longer supported by the devel:tools OBS project it seems. The key part of the CI failure is some golint errors about missing documentation:

/go/src/github.com/opencontainers/umoci/oci/layer/types.go:7:6: exported type WhiteoutMode should have comment or be unexported
/go/src/github.com/opencontainers/umoci/oci/layer/types.go:20:6: exported type UnpackOptions should have comment or be unexported

@tych0
Copy link
Member Author

tych0 commented Jul 8, 2020

The tests are failing locally because you have an old version of opensuse/leap:latest

Oh, I thought docker was smart enough to figure that out. But you're right, I cleared my cache and now it's working.

@tych0 tych0 force-pushed the add-whiteout-option branch 6 times, most recently from 2b9f23b to 70f0eee Compare July 10, 2020 14:23
go.mod Show resolved Hide resolved
@cyphar cyphar changed the title Add whiteout option {un,re}pack: support overlayfs whiteouts Jul 13, 2020
@tych0 tych0 force-pushed the add-whiteout-option branch from 70f0eee to 989fd97 Compare July 13, 2020 13:31
tych0 added 2 commits July 13, 2020 20:15
A long time ago as a hack to avoid this very patch, I added KeepDirlinks to
MapOptions. Then, I added a callback and the StartFrom argument, having to
thread that through all the callsites. In the next patch, I'll add another
unpack option.

So, let's bite the bullet and create an UnpackOptions struct to pass to the
unpacking code. Note that this doesn't have *all* of the options like
bundle path and such, since those aren't necessarily relevant for all
unpacking operations (UnpackRootfs(), UnpackManifest(), Unpack(), etc.).

Signed-off-by: Tycho Andersen <tycho@tycho.ws>
we produce this via 'make ci', let's ignore it too.

Signed-off-by: Tycho Andersen <tycho@tycho.ws>
@tych0 tych0 force-pushed the add-whiteout-option branch from 989fd97 to 6b7327b Compare July 14, 2020 02:15
@tych0
Copy link
Member Author

tych0 commented Jul 14, 2020

Ok, I rebased too because of the gitignore conflict :)

@tych0 tych0 closed this Jul 14, 2020
@tych0 tych0 reopened this Jul 14, 2020
cyphar
cyphar previously requested changes Jul 14, 2020
oci/layer/generate.go Outdated Show resolved Hide resolved
oci/layer/tar_extract.go Outdated Show resolved Hide resolved
oci/layer/generate.go Show resolved Hide resolved
oci/layer/types.go Outdated Show resolved Hide resolved
oci/layer/types.go Show resolved Hide resolved
oci/layer/utils.go Outdated Show resolved Hide resolved
oci/layer/utils.go Outdated Show resolved Hide resolved
repack.go Outdated Show resolved Hide resolved
tych0 added 2 commits July 14, 2020 15:51
While not strictly OCI standard, it can be useful to have umoci write
overlayfs stile whiteouts when unpacking instead of just manipulating the
filesystem on its own, for example for use with drivers which are going to
use overlayfs to speed up image building and extraction.

Note that writing whiteouts (i.e. mknod foo c 0 0) is supported as an
unprivileged user in the upstream kernel via a3c751a50fe6 ("vfs: allow
unprivileged whiteout creation"), which will be released in 5.8. There is
*no* support right now for writing the opaque overlay attribute as an
unprivileged user, but since that's mostly done via GenerateInsertLayer()
and not in the general case, I doubt most people will run into this. And if
they do, they can send a kernel patch :)

Note that we only run the test on linux, since osx presumably doesn't have
this hack to let us mknod c 0 0 as an unprivileged user.

Signed-off-by: Tycho Andersen <tycho@tycho.ws>
This is the compliment to the previous commit, in case people want to
re-pack layers using the OCI standard instead of the overlayfs standard.

Also add a PackOptions{} struct, similar to UnpackOptions{}, which
can control the low level behavior of the packing routines. I expect we'll
use this later for hooks, too.

Signed-off-by: Tycho Andersen <tycho@tycho.ws>
@tych0 tych0 force-pushed the add-whiteout-option branch from 6b7327b to a425f92 Compare July 14, 2020 21:52
This way we can automatically DTRT if people set the WhiteoutMode during
Unpack().

Signed-off-by: Tycho Andersen <tycho@tycho.ws>
@tych0
Copy link
Member Author

tych0 commented Jul 14, 2020

Seems I can't run the CI manually?

#4 importing cache manifest from umoci/ci:latest
#4 ERROR: pull access denied, repository does not exist or may require authorization: server message: insufficient_scope: authorization failed

@tych0
Copy link
Member Author

tych0 commented Jul 14, 2020

Ok, I think I addressed everything modulo the Translate bit.

@cyphar
Copy link
Member

cyphar commented Jul 15, 2020

Sorry, I forgot that I needed to push the image. I've pushed it to Docker Hub.

@tych0
Copy link
Member Author

tych0 commented Jul 29, 2020

Ping, I think this one is good to go.

@tych0
Copy link
Member Author

tych0 commented Aug 19, 2020

Ping?

1 similar comment
@tych0
Copy link
Member Author

tych0 commented Aug 31, 2020

Ping?

@tych0
Copy link
Member Author

tych0 commented Sep 8, 2020

Ping?

@cyphar cyphar dismissed their stale review September 21, 2020 19:21

outdated

@cyphar
Copy link
Member

cyphar commented Sep 21, 2020

LGTM, sorry for letting this one sit for so long (I got half-way through re-reviewing it several times).

@tych0
Copy link
Member Author

tych0 commented Sep 22, 2020

NP, thanks :)

@tych0 tych0 merged commit f495332 into opencontainers:master Sep 22, 2020
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.

2 participants