Update cimfs snapshotter & differ for new hcsshim interface #10033
Conversation
|
Hi @ambarve. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
ambarve
force-pushed
the
cimfs_layer_refactor
branch
from
be2ed19 to
e0f787b
Compare
script/setup/runhcs-version
Outdated
| @@ -1 +1 @@ | |||
| v0.12.0 | |||
| 1d406d0eac5573287ba7b46a04a58275410137ac | |||
There was a problem hiding this comment.
Is it possible to get a release with the changes? 2.0 doesn't seem too far off so it'd be nice to stay on a tag
There was a problem hiding this comment.
Hmm, I wasn't planning to get this in containerd 2.0 since the rc builds are already out. That's why we didn't get a release on hcsshim. Do we still need a tag if we don't want to get this in 2.0?
There was a problem hiding this comment.
Maybe I've forgotten the release process but I'd think if this got checked in anytime before we decide to tag 2.0 then it'd be part of the release
There was a problem hiding this comment.
Yeah, it does look like merging this right now will get it into 2.0. I have confirmed that getting this into 2.0 shouldn't be a problem.
There was a problem hiding this comment.
Gotcha.
I have confirmed that getting this into 2.0 shouldn't be a problem.
You mean the commit instead of a tag?
There was a problem hiding this comment.
We don't need to get this into 2.0, we can merge this after the final 2.0 tag is cut. At that time, we can merge with a commit ID of hcsshim and then when containerd is preparing for a 2.1 release we can make a separate hcsshim tag at that time and vendor that. Does that sound okay?
There was a problem hiding this comment.
Sounds fine to me at least, I'm not sure if any other maintainers would prefer us stay on a tag.
There was a problem hiding this comment.
I agree it would be best to keep a tagged release here.
|
Needs a rebase |
ambarve
force-pushed
the
cimfs_layer_refactor
branch
from
e0f787b to
abedcaf
Compare
ambarve
force-pushed
the
cimfs_layer_refactor
branch
2 times, most recently
from
ca0c82e to
6e1f0ee
Compare
ambarve
force-pushed
the
cimfs_layer_refactor
branch
from
6e1f0ee to
d3d91db
Compare
ambarve
force-pushed
the
cimfs_layer_refactor
branch
from
d3d91db to
874c5de
Compare
ambarve
force-pushed
the
cimfs_layer_refactor
branch
from
874c5de to
6c2b23d
Compare
|
@dcantah I was waiting to merge this before containerd 2.0 was released. Could you please take another look at this and approve if it looks good? |
hcsshim recently [updated](microsoft/hcsshim@1d406d0) the interface of APIs that are used for importing OCI layers. It now expects that the CimFS snapshotter mounts contain the full cim paths for parent layers. This change updates the cimfs differ & snapshotter to use that new interface. Signed-off-by: Amit Barve <[email protected]>
ambarve
force-pushed
the
cimfs_layer_refactor
branch
from
6c2b23d to
098d303
Compare
|
|
||
| const ( | ||
| // LayerCimPathFlag is the option flag used to represent the path at which a layer CIM must be stored. This | ||
| // flag is only included if an image layer is being extracted onto the snapshot i.e the snapshot key has an |
There was a problem hiding this comment.
This statement doesn't seem to be true, it looks like we always set LayerCimPathFlag on the mount?
hcsshim recently updated the interface of APIs that are used for importing OCI layers. Plus it now expects that the CimFS snapshotter mounts contain the full cim paths for parent layers. This change updates the cimfs differ & snapshotter to use that new interface.