image-rs: add conformance test for image pulling#1480
Conversation
Add a test that pulls an image with umoci and with image-rs and then compares them gomtree. To avoid issues with the network, the test spins up a local registry and loads images from test_data/conformance_images into it. This PR does not add any test images (thus the test doesn't do anything). We can add images to the repo (pending some license discussion) or you can just run the test locally with your own test images. This also adds a script to pull a test image and put it in the directory with the right format. While umoci is a good reference implementation to compare to, it differs from image-rs in that it unpacks an image directly to the filesystem (while image-rs uses overlayfs). This leads to some differences even when the image is unpacked correctly. For example, overlayfs does not correctly report hardlink counts. Thus, we don't include nlink in our comparison. Overlayfs can also have some discrepancies in terms of size. For now, this is included in the comparison and these differences are reported. Further, it's not yet clear if the mtime of the root dir will be correct with overlayfs. This is also reported. This test is not yet ready to be required, but it is interesting and useful. Signed-off-by: Tobin Feldman-Fitzthum <tfeldmanfitz@nvidia.com>
Xynnn007
left a comment
Xynnn007
left a comment
There was a problem hiding this comment.
This is a unfinished review, until I realize the aim of the PR. Please correct me then I could better understand.
| let spec_output = Command::new("gomtree") | ||
| .args([ | ||
| "-c", | ||
| "-k", | ||
| "size,type,uid,gid,mode,link,time,sha256digest,xattr,xattrs", | ||
| "-p", | ||
| ]) | ||
| .arg(umoci_rootfs) | ||
| .output() | ||
| .expect("Failed to execute gomtree"); | ||
| assert!( | ||
| spec_output.status.success(), | ||
| "gomtree spec generation failed: {}", | ||
| String::from_utf8_lossy(&spec_output.stderr) | ||
| ); |
There was a problem hiding this comment.
Note that this requires that umoci and gomtree are both installed on the machine. We'd better do a test before call them.
| println!( | ||
| "Rootfs mismatch for {} (image-rs vs umoci):\n{}{}", | ||
| image_name, stdout, stderr | ||
| ); |
There was a problem hiding this comment.
The println usually does not work in test and can be ignored. return this as an error to the upper layer, and do panic/assert would be better
| if images.is_empty() { | ||
| eprintln!("No test images found, skipping"); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Up to now, I start to know that the aim of the PR is not to do comparation everytime we do cargo test, but provide a way for users to compare with umoci, right?
If this is the case, and also integrate "pull image + comparation" into CI is not urgent, I'd suggest to start from #1414. Then we could provide a document to help users to do pull + comparation work using the image-rs tool.
There was a problem hiding this comment.
Yes, at some point we might want to do explicit checks in every PR, but that requires figuring out a few things (which images do we want; can we add them to the repo; which attributes should we check; etc). For now, this is just for diagnostic purposes. I don't think we even necessarily need to merge, but I figured I would share what I have been using to test since we are working on image-rs. Might as well put it into the repo imo.
I don't think this is related to #1414 tho. It doesn't matter if we use image-rs as a crate or a binary. In fact, it may be better to test it as a crate, since that's what we mostly use.
There was a problem hiding this comment.
Yes. I agree with the part of how umoci pulls images and how we do comparation.
The issue is that "where we can get an image rootfs pulled by image-rs". you are using an integration test in the PR, while I usually directly change the code of the unit test.
Before this becomes a required component of CI check in a proper way, it might not be proper to get the code in.
Instead, a doc to document these steps can be a merge-able thing imo.
2 participants
Add a test that pulls an image with umoci and with image-rs and then compares them gomtree.
To avoid issues with the network, the test spins up a local registry and loads images from test_data/conformance_images into it. This PR does not add any test images (thus the test doesn't do anything). We can add images to the repo (pending some license discussion) or you can just run the test locally with your own test images. This also adds a script to pull a test image and put it in the directory with the right format.
I think we may want to generalize this local registry approach and use it for some of the other tests too. This could be handy for performance tests, for instance, which we should probably add. Anecdotally image-rs seems significantly slower than umoci.
While umoci is a good reference implementation to compare to, it differs from image-rs in that it unpacks an image directly to the filesystem (while image-rs uses overlayfs). This leads to some differences even when the image is unpacked correctly.
For example, overlayfs does not correctly report hardlink counts. Thus, we don't include nlink in our comparison. Overlayfs can also have some discrepancies in terms of size. For now, this is included in the comparison and these differences are reported.
Further, it's not yet clear if the mtime of the root dir will be correct with overlayfs. This is also reported.
This test is not yet ready to be required, but it's pretty handy.