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

Calculate checksum when installing image layers #8249

Closed
wants to merge 4 commits into from

Conversation

jlhawn
Copy link
Contributor

@jlhawn jlhawn commented Sep 26, 2014

Compute TarSum of image layer data when installing
  • Adds a new Checksum field to the current image format to store the result of the tarsum of the file system changes.
Remove jsonData argument from image.StoreImage
  • The argument specified the json data to save to disk when registering a new image into the image graph. If it is nil, then the given image is serialized to json and that is written by default. This default behavior is sufficient because the given image was originally deserialzed
    from this jsonData to begin with.
Remove unused "image_set" job
  • A quick grep of the code base shows that the "image_set" job registered by the TagStore is not used.

work towards #8093

@dmp42 dmp42 added this to the 1.3.0 milestone Sep 26, 2014
@@ -32,6 +33,7 @@ type Image struct {
Config *runconfig.Config `json:"config,omitempty"`
Architecture string `json:"architecture,omitempty"`
OS string `json:"os,omitempty"`
Checksum string `json:"checksum"`
Copy link
Contributor

Choose a reason for hiding this comment

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

do we want this information to be pushed on the registry & available on docker inspect ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This will be pushed to the registry as part of the image json and available when inspecting an image and requesting raw json. We do want it available as it is very important to the goals for #8093

@vieux
Copy link
Contributor

vieux commented Sep 26, 2014

Lots of deletions, I like it!

@jlhawn
Copy link
Contributor Author

jlhawn commented Sep 26, 2014

@vieux @crosbymichael as a proof of backwards compatibility, I've pushed my own build of busybox:

docker pull jlhawn/busybox

you should be able to pull with docker 1.2 and manually check /var/lib/docker/graph/<img_id>/json for the new checksum field.

Edit: if you're going to check the image IDs, here are the IDs that you should get after pulling (in order of base to leaf layer):

b7d3e77360a6ecaf24e4051b841a3b228f24ec7fff104de0335daa9f558244fc
59dc2274ebfe4ced700b26e1ace5be46b0240599cc3f2d137b2d481730919e6d
6e43fbcf56f737ffee152b125308ec99dad92f81585d4b7f035e63e48f33238c
b1f177363e4f0cb0c6cf7c3121b809bbe6f4640980ae8868df218ce0c5dcb32b

and their respective tarsums:

tarsum+sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
tarsum+sha256:ba1c12fd12e7cef83cc08f453335d57723993a7563207218853d21c358a63c61
tarsum+sha256:17165fde06d415685a1232234b86ff836e0faed296656a9328bf08b0f1cb997f
tarsum+sha256:ba1c12fd12e7cef83cc08f453335d57723993a7563207218853d21c358a63c61

@jlhawn
Copy link
Contributor Author

jlhawn commented Sep 27, 2014

wait... Wait a minute..... WAIT! 3 of those tarsums should be the same.... They should all be the tarsum of an empty change set, i.e., e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855. This ba1c12fd12e7cef83cc08f453335d57723993a7563207218853d21c358a63c61 nonsense must be from the .wh..wh.* files that AUFS uses. darn...

@jlhawn
Copy link
Contributor Author

jlhawn commented Sep 27, 2014

@dmcgowan I think this issue will affect how/when we compute the checksums for layers. Computing it on commit/load/import/pull should be okay, but we'll also need to compute it when doing a push/save/export. This is because the tarsum of the layer will change from one graphdriver to the next. exemplified above, AUFS generates the .wh..wh.* metadata/hardlink files that the other graphdrivers will ignore when installing, so if they have pulled/imported/loaded layers that were generated by an AUFS driver, when they go to push/export/save the layers again they will have a different checksum. So I guess the safest thing to do is compute the tarsum whenever a layer is entering or leaving the system?

@vieux
Copy link
Contributor

vieux commented Sep 29, 2014

ping @jlhawn

@vieux vieux modified the milestones: 1.4, 1.3.0 Sep 29, 2014
@tiborvass
Copy link
Contributor

Reping @jlhawn

@jlhawn
Copy link
Contributor Author

jlhawn commented Oct 21, 2014

hi. I starting working on this a bit more as of yesterday. I think there may be a safe way for the aufs driver to export/import a layer without exporting/importing the .wh..wh.* stuff. I'm going to have to test it out to make sure links still work between layers.

@jlhawn jlhawn force-pushed the image_v2 branch 2 times, most recently from f837656 to c5e77f3 Compare October 23, 2014 21:07
@jlhawn
Copy link
Contributor Author

jlhawn commented Oct 24, 2014

@vieux @tiborvass @dmcgowan I'm running into a few details/issues that I am unsure of how to resolve and are major blockers for me.

  1. I've got the AUFS Driver's Diff() and ApplyDiff methods to ignore .wh..wh.* and I've tested creating hard links between layers and it appears to not really matter if we include the .wh..wh.plink folder when exporting an AUFS layer as the hard linked files are copied up anyway, and since other graph drivers explicitly do not import this folder and just copy up the hardlinked files (which appear to already be done by AUFS). While I'm pretty sure this is safe, I'm not 100% certain.
  2. The way the VFS Driver creates a new layer given a parent is by just running cp -a <src_dir> <dst_dir>. However, I've noticed that the archive behavior does not behave as is described. It should preserve modified times of all files/directories but this does not happen when it copies symbolic links from the base image. All of the symbolic links get new modified times and therefore will appear as changes when we export the new layer using the VFS driver's Diff() method. I'm not sure if this is a bug in the cp utility or if it's meant to behave this way (I've tested it on busybox, ubuntu, mac os x, and tiny core linux and all implementations have this issue), but either way it means, again, that even though we generate the same build/content for an image, the checksums computed using tarsum will be different depending on the storage driver you are using because the different storage drivers handle create/import/export of layers differently.

EDIT: no cp -a symlink modified time issue on ubuntu

@dmcgowan
Copy link
Member

For issue 2, tested on my Fedora machine and cp -a preserved the modified timestamps.

Kernel: 3.16.6
GNU CoreUtils: 8.21

@jlhawn
Copy link
Contributor Author

jlhawn commented Oct 25, 2014

@dmcgowan and I have discussed some possible solutions to this issue that would affect Docker in different ways depending on how we decide to get around the above system/driver dependent checksum.

  1. We compute checksums when pulling a layer, i.e., the image manifest says it has checksum abcd, so as you pull the layer, compute its tarsum and ensure that the archive has the same checksum as the one from the manifest. Next the call to driver.ApplyDiff(imgID, parentID, layerArchive) would compute the tarsum of the layer again before it returns so that you now have the checksum of the layer as it exists on your local storage driver since it may be different (as explained above), then you use this checksum to put in the image JSON that you end up storing locally since it matches the actual content you'd get from the storage driver for that layer. There are a couple of issues with this though. The first is time performance related as we would be doing even more TarSum computations which take a significant amount of time depending on the size of the layer and the storage driver being used. This would make pull/commit appear slower to users of Docker. Second, it precludes us from keeping the original signed manifest which we've used to pull an image. Since the checksums for the layers we ended up storing may not match the original checksums in the image manifest, any images that are build which base off of this original image will not be able to include the original manifest when exporting this new signed image because the layer checksums aren't guaranteed to match, so you'll no longer be able to prove that your new image is based on the official ubuntu base or rhel or anything else.
  2. Another option is to keep around the compressed tar.gz archive which was pulled/imported/loaded in another location on the filesystem, e.g., /var/lib/docker/layer_archives/. When pulling a v2 image, the pull would immediately write it to disk at that location, then extract it to the storage driver while computing the tarsum. It then no longer matters what storage driver you use when you decide to export the layer again because you still have the original, compressed, archive. And you can continue to include the signed manifest of this image nested an any other image manifests you produced which use that image as a base: you still have the archives which match the checksums in the base image manifest. What are the issues with this approach? Extra disk space is now required for docker to keep these archives around, but they would be gzipped archives so disk usage per imported layer would not double, but probably multiply by 1.3-1.4x depending on the layer content.

Personally, I prefer option 2 over option 1 because disk space is cheap and the extra disk space utilized by option 2 could be minimized if we add features to make it easier for users to clean up untagged/unnecessary images, and limit the cases when we save the extra archives to only importing of layers. This option also does not introduce extra invocations of tarsum or preclude us from adding nested/base image manifests for image provenance.

Josh Hawn added 4 commits October 27, 2014 09:55
Adds a new `Checksum` field to the current image format
to store the result of the tarsum of the file system changes.

Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
The argument specified the json data to save to disk when registering
a new image into the image graph. If it is nil, then the given image
is serialized to json and that is written by default. This default
behavior is sufficient because the given image was originally deserialzed
from this jsonData to begin with.

Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
A quick grep of the code base shows that the "image_set" job
registered by the TagStore is not used.

Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
Also includes Checksum field in `docker inspect` output for images.
Correcly close layerData decompress/reader wrapper when storing images.
Remove imgJSON from call to graph.Register when pulling an image.

Docker-DCO-1.1-Signed-off-by: Josh Hawn <josh.hawn@docker.com> (github: jlhawn)
@jlhawn
Copy link
Contributor Author

jlhawn commented Oct 27, 2014

I'm breaking this PR up into multiple PRs so I may continue work on other issues. I've also thought of a potential way around the 'shell out to cp' issue and I'll submit a PR with that.

@jlhawn
Copy link
Contributor Author

jlhawn commented Oct 30, 2014

Hey all, I'm closing this issue for now in lieu of various other PRs that I've been submitting. I'll submit a proposal for the option 2 that I outlined above.

@jlhawn jlhawn closed this Oct 30, 2014
@jlhawn jlhawn deleted the image_v2 branch July 31, 2015 18:04
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

5 participants