containerd / nydus-snapshotter

A containerd snapshotter with data deduplication and lazy loading in P2P fashion
https://nydus.dev/
Apache License 2.0
173 stars 100 forks source link

snapshot: fix error on proxy driver when switching different snapshotter #593

Closed ChengyuZhu6 closed 6 months ago

ChengyuZhu6 commented 6 months ago

Fixes: #592

ChengyuZhu6 commented 6 months ago

cc @imeoer

codecov[bot] commented 6 months ago

Codecov Report

Attention: Patch coverage is 0% with 54 lines in your changes are missing coverage. Please review.

Project coverage is 34.36%. Comparing base (b8ffddf) to head (8b98d6a). Report is 1 commits behind head on main.

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/593/graphs/tree.svg?width=650&height=150&src=pr&token=R3RX5WX6R1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd)](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/593?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd) ```diff @@ Coverage Diff @@ ## main #593 +/- ## ========================================== - Coverage 34.64% 34.36% -0.29% ========================================== Files 65 65 Lines 6552 6606 +54 ========================================== Hits 2270 2270 - Misses 3967 4021 +54 Partials 315 315 ``` | [Files](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/593?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd) | Coverage Δ | | |---|---|---| | [snapshot/process.go](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/593?src=pr&el=tree&filepath=snapshot%2Fprocess.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd#diff-c25hcHNob3QvcHJvY2Vzcy5nbw==) | `0.00% <0.00%> (ø)` | | | [snapshot/snapshot.go](https://app.codecov.io/gh/containerd/nydus-snapshotter/pull/593?src=pr&el=tree&filepath=snapshot%2Fsnapshot.go&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=containerd#diff-c25hcHNob3Qvc25hcHNob3QuZ28=) | `0.00% <0.00%> (ø)` | |
imeoer commented 6 months ago

@ChengyuZhu6 Thanks! I'll try to reproduce the bug and test the PR.

fidencio commented 6 months ago

This does work around the problem we've encountered when using guest-pull with Confidential Containers. More than working around the issue, it gives us enough time to work on containerd (which has a pace of release and adoption that can be slow to reach all the CSPs) fix without worrying much about which version of containerd will be used with Confidential Containers.

Huge thumbs up for having this one in!

fidencio commented 6 months ago

@ChengyuZhu6, I'd love to see, as part of the commit message, as much information as you provided in the issue. This will help us, later on, to figure out why those changes were made without having to come back to GitHub to do so.

stevenhorsman commented 6 months ago

FYI - I've also testing this code and quay.io/chengyu_zhu/nydus-snapshotter@sha256:4b5e333ecd27d7b630cbe42fe686f8b3c38df7727c99f4b5cef724be6f3926fa with peer pods and it seemed to work without the workaround of cleaning up the images and content on the node. Amazing work!