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

Bad whiteout when importing in docker #254

Closed
CajuM opened this issue Sep 3, 2018 · 9 comments · Fixed by #258
Closed

Bad whiteout when importing in docker #254

CajuM opened this issue Sep 3, 2018 · 9 comments · Fixed by #258

Comments

@CajuM
Copy link

CajuM commented Sep 3, 2018

The umoci version in question is 0.4.1
Steps to reproduce:

umoci init --layout test
umoci new --image test:latest

sudo umoci unpack --image test:latest test-bundle1
sudo mkdir test-bundle1/rootfs/test
sudo touch test-bundle1/rootfs/test/test
sudo umoci repack --image test:latest test-bundle1

sudo umoci unpack --image test:latest test-bundle2
sudo rm -rf test-bundle2/rootfs/test
sudo umoci repack --image test:latest test-bundle2

sudo skopeo --insecure-policy copy oci:test:latest docker-archive:test.tar
cat test.tar | docker load

Docker will output the following error: "Error processing tar file(exit status 1): not a directory"
If I tar tvf the second layer, I get:
drwxr-xr-x root/root 0 2018-09-03 16:25 .
---------- 0/0 0 2018-09-03 16:25 .wh.test
---------- 0/0 0 2018-09-03 16:25 test/.wh.test
I believe test/.wh.test should not be present

@cyphar
Copy link
Member

cyphar commented Sep 3, 2018

Okay, I see the issue. Docker has a different opinion to umoci over how whiteouts of whole directories should be handled in images (the specification doesn't actually say which is correct -- and in umoci we support both -- but I guess Docker has introduced a regression where they no longer support both).

To fix this we're going to need to come up with a way of removing any descendants within the same []mtree.Delta. I'm going to have to hack on this for a bit...

This does look like a Docker regression though -- I'm pretty sure this worked in the past (/me goes and tests this on 17.09). It is a Docker regression.

@cyphar
Copy link
Member

cyphar commented Sep 3, 2018

Yeah, your reproducer works without issue on Docker 17.09. I imagine this is a fairly recent change in Docker.

@CajuM
Copy link
Author

CajuM commented Sep 3, 2018

My docker is "Docker version 17.12.1-ce, build 7390fc6" on Ubuntu 18.04

@cyphar
Copy link
Member

cyphar commented Sep 3, 2018

Okay, so it was a containerd 1.0 change.

@CajuM
Copy link
Author

CajuM commented Sep 3, 2018

The relevant code is called from here:
https://github.com/docker/docker-ce/blob/master/components/engine/pkg/chrootarchive/archive_unix.go
Besides that, I have an strace:
lstat("/var/lib/yum/yumdb/l/.wh.5fdbe753a75779754e10414648989d7579bbd2ef-libcap-2.22-8.el7-x86_64", 0xc4205cf628) = -1 ENOENT (No such file or directory)
mknodat(AT_FDCWD, "/var/lib/yum/yumdb/l/5fdbe753a75779754e10414648989d7579bbd2ef-libcap-2.22-8.el7-x86_64", S_IFCHR|000, makedev(0, 0)) = 0
fchownat(AT_FDCWD, "/var/lib/yum/yumdb/l/5fdbe753a75779754e10414648989d7579bbd2ef-libcap-2.22-8.el7-x86_64", 0, 0, 0) = 0
read(0, "var/lib/yum/yumdb/l/5fdbe753a757"..., 512) = 512
read(0, "113 path=var/lib/yum/yumdb/l/5fd"..., 113) = 113
read(0, "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"..., 399) = 399
read(0, "var/lib/yum/yumdb/l/5fdbe753a757"..., 512) = 512
lstat("/var/lib/yum/yumdb/l/5fdbe753a75779754e10414648989d7579bbd2ef-libcap-2.22-8.el7-x86_64", {st_dev=makedev(253, 0), st_ino=8655889, st_mode=S_IFCHR|000, st_nlink=1, st_uid=0, st_gid=0, st_blksize=4096, st_blocks=0, st_rdev=makedev(0, 0), st_atime=1535967974 /* 2018-09-03T12:46:14.120000000+0300 /, st_atime_nsec=120000000, st_mtime=1535967974 / 2018-09-03T12:46:14.120000000+0300 /, st_mtime_nsec=120000000, st_ctime=1535967974 / 2018-09-03T12:46:14.120000000+0300 */, st_ctime_nsec=120000000}) = 0
lstat("/var/lib/yum/yumdb/l/5fdbe753a75779754e10414648989d7579bbd2ef-libcap-2.22-8.el7-x86_64/.wh.checksum_data", 0xc4205cfa38) = -1 ENOTDIR (Not a directory)
mknodat(AT_FDCWD, "/var/lib/yum/yumdb/l/5fdbe753a75779754e10414648989d7579bbd2ef-libcap-2.22-8.el7-x86_64/checksum_data", S_IFCHR|000, makedev(0, 0)) = -1 ENOTDIR (Not a directory)
write(2, "not a directory", 15) = 15

@CajuM
Copy link
Author

CajuM commented Sep 3, 2018

I think the problem is here:
https://github.com/docker/docker-ce/blob/master/components/engine/pkg/archive/archive.go#L895
and here:
https://github.com/docker/docker-ce/blob/master/components/engine/pkg/archive/archive_linux.go#L64

When docker-untar sees a whiteout file it calls mknod on the original path, even if it does not have a parent, which gives an ENOTDIR in our case

@cyphar
Copy link
Member

cyphar commented Sep 3, 2018

Okay, so they've started writing the internal format to the disk rather than emulating what the format tells you to do. That's definitely worrying (and weirdly non-extensible) -- and I'm also quite worried that they're going to start adding overlayfs whiteouts into their images despite this violating the OCI specification (because the new code currently supports that). How do they handle filesystems that don't use an internal format -- like btrfs or devicemapper? That's just weird.

(Our solution is the same though.)

@cyphar cyphar modified the milestone: 0.4.2 Sep 5, 2018
@cyphar
Copy link
Member

cyphar commented Sep 8, 2018

After discussing this with some other folks it has begun to look like Docker is no longer even attempting to be OCI-compatible with their image format (which is 'funny' given that "ability to easily convert from Docker images" was one of the design restrictions of the OCI image specification).

But I will do my best to figure out precisely what they want us to do...

@cyphar
Copy link
Member

cyphar commented Sep 9, 2018

Can you take a look at #258 which should fix the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants