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

Re-enable TestApplyLayer and make it work #3207

Merged
merged 4 commits into from
Dec 17, 2013

Conversation

alexlarsson
Copy link
Contributor

This re-enables TestApplyLayer and makes it work by re-implementing ApplyLayer() using the archive/tar go tar support.

Rather than calling out to tar we use the golang tar parser
to directly extract the tar files. This has two major advantages:

1) We're able to replace an existing directory with a file in the
   new layer. This currently breaks with the external tar, since
   it refuses to recursively remove the destination directory in
   this case, and there are no options to make it do that.

2) We avoid extracting the whiteout files just to later remove them.
The default gnu tar format has no sub-second precision mtime support,
and the golang tar writer currently doesn't support that either.
This means if we export the changes from a container we will not
get zeron in the sub-second precision field when the change is applied.

This means we can't compare that to the original without getting a
spurious change. So, we detect this case by treating a case where the
seconds match and either of the two nanoseconds are zero as equal.
With the previous two changes we now pass this test.
@crosbymichael
Copy link
Contributor

ping @creack

@crosbymichael
Copy link
Contributor

@alexlarsson I'm getting a few test failures but I'll help look into these because this approach is very good and will allow us not remove some of the custom code in aufs. Thanks!

--- FAIL: TestMount (0.00 seconds)
        graph_test.go:28: open /tmp/docker-graph-440920701/vfs/dir/0583af602d9a8f4382f14ab327cf677e6d8e12f52ba58769cbb54863835efbf2/etc/postgres/postgres.conf: no such file or directory
--- FAIL: TestInterruptedRegister (0.00 seconds)
        graph_test.go:85: open /tmp/docker-graph-092756919/vfs/dir/4eb8654991357fa9d86635cef433f035c7e083e7a8368e93a53bc004aa3b5192/etc/postgres/postgres.conf: no such file or directory
--- FAIL: TestGraphCreate (0.00 seconds)
        graph_test.go:100: open /tmp/docker-graph-114180010/vfs/dir/f9c1a7501df45f15638268f43331c66b790d71d039efc0dde7bec43c841f31d1/etc/postgres/postgres.conf: no such file or directory
--- FAIL: TestRegister (0.00 seconds)
        graph_test.go:136: open /tmp/docker-graph-731734785/vfs/dir/d18ed898e24744393dc999c29c15931bc26cdcb6663615444c7316ac3ca838a5/etc/postgres/postgres.conf: no such file or directory
--- FAIL: TestDeletePrefix (0.00 seconds)
        graph_test.go:173: open /tmp/docker-graph-095803500/vfs/dir/818db50153c7a113fca1042676c5617b5f4919c8f38c80f1b70e4499ad2d766e/etc/postgres/postgres.conf: no such file or directory
--- FAIL: TestDelete (0.00 seconds)
        graph_test.go:188: open /tmp/docker-graph-328509147/vfs/dir/f996911d9a6564e36222134357e0ccde557d4ce9f4c85b39856ef2b7d06d1dcd/etc/postgres/postgres.conf: no such file or directory
--- FAIL: TestByParent (0.00 seconds)
        graph_test.go:273: Expected 2 children, found 0

The TestLookupImage test seems to use a layer that contains
/etc/postgres/postgres.conf, but not e.g. /etc/postgres.

To handle this we ensure that the parent directory always
exists, and if not we create it.
@alexlarsson
Copy link
Contributor Author

With this fix all the tests pass for me

@creack
Copy link
Contributor

creack commented Dec 16, 2013

LGTM /cc @crosbymichael

@crosbymichael
Copy link
Contributor

LGTM

crosbymichael added a commit that referenced this pull request Dec 17, 2013
Re-enable TestApplyLayer and make it work
@crosbymichael crosbymichael merged commit 124da33 into moby:master Dec 17, 2013
@alexlarsson
Copy link
Contributor Author

Ugh, seems this broke docker pull:
e9aa60c60128: Error pulling image (latest) from busybox, archive/tar: invalid tar header

Seems archive.tar doesn't do uncompression, I guess we need to wrap the tar reader with an uncompression stream. I'll have a look.

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.

None yet

3 participants