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
support gzip on tarball.ImageFromPath #1858
base: main
Are you sure you want to change the base?
Conversation
2420a9b
to
4633861
Compare
@imjasonh @jonjohnsonjr PTAL |
@imjasonh PATL |
@jonjohnsonjr PTAL |
pkg/v1/tarball/image.go
Outdated
magicHeader := make([]byte, len(ggzip.MagicHeader)) | ||
n, err := f.Read(magicHeader) | ||
if n == 0 && err == io.EOF { | ||
return nil, errors.New("invalid tar header") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe the error should be reported as "invalid tar or tar.zip header" to match the context, as both formats are supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe the error should be reported as "invalid tar or tar.zip header" to match the context, as both formats are supported
done
Signed-off-by: zhangguanzhang <zhangguanzhang@qq.com>
4633861
to
43fddf9
Compare
do you have a chance to make a benchmark with following code: image, err := tarball.Image(func() (io.ReadCloser, error) {
file, _ := os.Open(imagePath)
// file.Seek(0, 0)
gzReader, err1 := gzip.NewReader(file)
return gzReader, err1
}, nil) The code above also supports the tar.gz format without changing the tarball code, but of very slow performance. I am wondering if they have any performance difference. |
I have been busy with work recently. You can compile two different binaries on your own computer and benchmark them to compare them. |
I did a test, but no luck for simple upload gzip image PS D:\code\go-containerregistry> .\crane-gzip.exe push C:\Users\Peter\image.tar.gz 127.0.0.1:10000/hello/crane-test:0.2
2024/04/13 18:42:34 existing blob: sha256:fe5ca62666f04366c8e7f605aa82997d71320183e99962fa76b3209fdfbb8b58
2024/04/13 18:42:34 existing blob: sha256:07a64a71e01156f8f99039bc246149925c6d1480d3957de78510bbec6ec68f7a
2024/04/13 18:42:34 existing blob: sha256:b02a7525f878e61fc1ef8a7405a2cc17f866e8de222c1c98fd6681aff6e509db
2024/04/13 18:42:34 existing blob: sha256:4aa0ea1413d37a58615488592a0b827ea4b2e48fa5a77cf707d0e35f025e613f
2024/04/13 18:42:34 existing blob: sha256:1e3d9b7d145208fa8fa3ee1c9612d0adaac7255f1bbc9ddea7e461e0b317805c
2024/04/13 18:42:34 existing blob: sha256:e8c73c638ae9ec5ad70c49df7e484040d889cca6b4a9af056579c3d058ea93f0
2024/04/13 18:42:34 existing blob: sha256:5627a970d25e752d971a501ec7e35d0d6fdcd4a3ce9e958715a686853024794a
2024/04/13 18:42:34 existing blob: sha256:7c881f9ab25e0d86562a123b5fb56aebf8aa0ddd7d48ef602faf8d1e7cf43d8c
2024/04/13 18:42:34 existing blob: sha256:804bcaae78ee5b94f93193de7a0c3c3f4c55e6f84a76f299c6c0628e35933d9d
2024/04/13 18:42:34 existing blob: sha256:a0eed15eed4498c145ef2f1883fcd300d7adbb759df73c901abd5383dda668e7
2024/04/13 18:42:35 existing blob: sha256:d481ac5b71a6be7b6f1b0715b57a0ee724143b98aa5da9b54a22f9ba2d6cfc98
2024/04/13 18:42:35 existing blob: sha256:fcb6f6d2c9986d9cd6a2ea3cc2936e5fc613e09f1af9042329011e43057f3265
2024/04/13 18:42:36 existing blob: sha256:86b2214f3e425fcd38954cad4285959f086bd6e9f11b07307e3eb82860224441
Error: PUT http://127.0.0.1:10000/v2/hello/crane-test/manifests/0.2: multiple errors returned: MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:54ad2ec71039b74f7e82f020a92a8c2ca45f16a51930d539b56973a18b8ffe8d; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:6fbdf253bbc2490dcfede5bdb58ca0db63ee8aff565f6ea9f918f3bce9e2d5aa; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:7bea6b893187b14fc0a759fe5f8972d1292a9c0554c87cbf485f0947c26b8a05; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:ff5700ec54186528cbae40f54c24b1a34fb7c01527beaa1232868c16e2353f52; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:d52f02c6501c9c4410568f0bf6ff30d30d8290f57794c308fe36ea78393afac2; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:e624a5370eca2b8266e74d179326e2a8767d361db14d13edd9fb57e408731784; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:1a73b54f556b477f0a8b939d13c504a3b4f4db71f7a09c63afbc10acb3de5849; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:d2d7ec0f6756eb51cf1602c6f8ac4dd811d3d052661142e0110357bf0b581457; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:4cb10dd2545bd173858450b80853b850e49608260f1a0789e0d0b39edf12f500; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:5185a177ceb5de87c52c72d5704ad35976a413b25e14a96149f917e8ed29aedc; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:f9e13b6fcbbe89bc58ca4479888d3a427b4fa468ea750fac2c9a47db7d923d13; MANIFEST_BLOB_UNKNOWN: blob unknown to registry; sha256:8247c33dd8dcfc2cedd8a00591c6809676cafb8e0da44b6dbc9aed8525b560e8 I am sure the file was a valid gzip docker image: $ file image.tar.gz
image.tar.gz: gzip compressed data, was "image.tar", last modified: Sat Apr 13 10:23:29 2024, from Unix, original size modulo 2^32 148959232
Peter@ASUS-TUF-GAMING MINGW64 ~
$ docker load -i image.tar.gz
Loaded image: registry.k8s.io/etcd:3.5.10-0 |
用linux下跑我单元测试里那个呢 |
I did a rebase on the main or your branch, all seem work fine now:) |
Why do you want this change? It doesn't make a lot of sense to me. The output of
Since this code currently re-opens the stream with every file access, this is going to be really really slow. It would be better for callers to decompress it themselves first. I have been experimenting with some code that would make this a little bit less slow, but I'd rather avoid complex stuff like that if we can. Can you describe why you need this? |
Thanks for reply.
The size difference will be larger after larger image compression. Maybe there is a performance problem in my implementation, but this is indeed a very common scenario. |
support gzip on