Support sharing image on the host for layer block mode #560
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChengyuZhu6
force-pushed
the
layer_block
branch
from
659424d
to
9de4ab8
Compare
ChengyuZhu6
changed the title
ChengyuZhu6
force-pushed
the
layer_block
branch
6 times, most recently
from
9719d39
to
a80586b
Compare
|
/cc @jiangliu |
ChengyuZhu6
force-pushed
the
layer_block
branch
2 times, most recently
from
9700477
to
ffb6713
Compare
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #560 +/- ##
==========================================
- Coverage 33.63% 33.52% -0.12%
==========================================
Files 65 65
Lines 8259 8287 +28
==========================================
Hits 2778 2778
- Misses 5166 5194 +28
Partials 315 315
|
ChengyuZhu6
force-pushed
the
layer_block
branch
3 times, most recently
from
b224a87
to
00ceb1c
Compare
Introduce a function to get the path of layer disk file. Fixes: containerd#559 Signed-off-by: ChengyuZhu6 <[email protected]>
ChengyuZhu6
force-pushed
the
layer_block
branch
3 times, most recently
from
685b2e1
to
723b3b2
Compare
jiangliu
reviewed
Dec 7, 2023
pkg/tarfs/tarfs.go
Outdated
| } else if !wholeImage != perLayer { | ||
| } | ||
|
|
||
| if exportDisk && !perLayer && !wholeImage && !withVerity { |
There was a problem hiding this comment.
Could you please help to add a comment about what's this condition for?
There was a problem hiding this comment.
By design，the function ExportBlockData() should not be called for rw layer. So could you please help to confirm the call stack?
There was a problem hiding this comment.
Could you please help to add a comment about what's this condition for?
Fixed.
There was a problem hiding this comment.
if !exportDisk && !withVerity {
return updateFields, nil
} else if !wholeImage != perLayer {
// Special handling for `layer_block` mode
if exportDisk && !withVerity && !perLayer {
labels[label.NydusLayerBlockInfo] = ""
updateFields = append(updateFields, "labels."+label.NydusLayerBlockInfo)
}
return updateFields, nil
}
jiangliu
reviewed
Dec 7, 2023
snapshot/mount_option.go
Outdated
| @@ -151,10 +151,14 @@ func (o *snapshotter) mountWithKataVolume(ctx context.Context, id string, overla | |||
|
|
|||
| func (o *snapshotter) mountWithProxyVolume(rafs rafs.Rafs) ([]string, error) { | |||
| options := []string{} | |||
| source := "" | |||
There was a problem hiding this comment.
how about simplifying it as :
source := rafs.Annotations[label.CRIImageRef]
1a02642
to
24d2230
Compare
When the layer_block mode is enabled, the remote snapshot does not contain the `containerd.io/snapshot/nydus-layer-block` label, which prevents the creation of a kata volume. Therefore, we need to add the `containerd.io/snapshot/nydus-layer-block` label to the remote snapshot. Signed-off-by: ChengyuZhu6 <[email protected]>
Introduce a function to prepare KataVirtualVolume for both guest pull image and host sharing image modes to eliminate redundant code. Signed-off-by: ChengyuZhu6 <[email protected]>
ChengyuZhu6
force-pushed
the
layer_block
branch
from
24d2230
to
5a3efca
Compare
jiangliu
reviewed
Dec 7, 2023
| if len(info) > 0 { | ||
| dmverity, err := parseTarfsDmVerityInfo(info) | ||
|
|
||
| options = append(options, opt) |
There was a problem hiding this comment.
It would be better to return here，to protect from a abnormal case where both NydusImageBlockInfo and NydusLayerBlockInfo are set.
jiangliu
approved these changes
Dec 7, 2023
|
An kata volume option takes about 500-bytes as And mount options are limited to 4096 bytes by containerd, so seems only max 8-layers may be supported by this way. You have test an image with 7 layers, so could you please help to test an image with more layers? |
ChengyuZhu6
force-pushed
the
layer_block
branch
from
5a3efca
to
f8f4416
Compare
Sure, I’d be happy to help. |
Support to handle kata volume for layer block and reconstruct functions about handling kata virtual volumes. Signed-off-by: ChengyuZhu6 <[email protected]>
The sandbox image (pause image) name cannot be stored in annotations by containerd, so snapshotter is unable to retrieve it. This means that the source field of `KataVirtualVolume` has to be set to "" to support guest pull in proxy mode now. However, once containerd fixes this bug (containerd/containerd#9419), I think the nydus snapshotter could be able to get the sandbox image name from annotations directly. I recommend that we can support to obtain the image name through "containerd.io/snapshot/cri.image-ref" in snapshotter. If this value is empty, the source should be set to "", otherwise it should be set to the image name. Signed-off-by: ChengyuZhu6 <[email protected]>
ChengyuZhu6
force-pushed
the
layer_block
branch
from
f8f4416
to
4473328
Compare
|
@jiangliu I try to pull options: the decoded data: In kata log: |
|
Seems strange，need to confirm whether containerd has a 4K limitation for annotations。 |
Sure. |
Containerd has a 4096-byte limit for labels (https://github.com/containerd/containerd/blob/a68efb1bad631a7a6c37953af791abf1d551790f/api/services/snapshots/v1/snapshots.proto#L49). However, this limit does not apply to the mounts returned by the snapshotter. Each mount can have an arbitrary length. (https://github.com/containerd/containerd/blob/a68efb1bad631a7a6c37953af791abf1d551790f/api/types/mount.proto) |
imeoer
approved these changes
Dec 12, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Nydus snapshotter supports both image block and layer block modes, with or without dm-verity. However, for confidential containers, nydus-snapshotter only supports image block mode, with or without dm-verity. Therefore, we need to extend nydus-snapshotter to support layer block mode, with or without dm-verity, for confidential containers as well.
options example for pulling
docker.io/library/nginx:latestdecoded data:
Fixes: #559