search

containerd / cri

Moved to https://github.com/containerd/containerd/tree/master/pkg/cri . If you wish to submit issues/PRs, please submit to https://github.com/containerd/containerd
https://github.com/containerd/containerd/tree/master/pkg/cri
Apache License 2.0
900 stars 348 forks source link

[release/1.4] Add manifest digest annotation for snapshotters #1609

Closed shahzzzam closed 4 years ago

shahzzzam commented 4 years ago

Signed-off-by: Samarth Shah samarthmshah@gmail.com

We leverage stargz-based snapshotter and have implemented our own Filesystem interface. We need the image digest to get some metadata for the image itself.

k8s-ci-robot commented 4 years ago

Hi @shahzzzam. 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 /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available [here](https://git.k8s.io/community/contributors/guide/pull-requests.md). If you have questions or suggestions related to my behavior, please file an issue against the [kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:) repository.
shahzzzam commented 4 years ago

@ktock @cpuguy83 @estesp @thaJeztah can you help me understand which commits i need to cherry-pick into CRI 1.4 to unblock myself on https://github.com/containerd/containerd/pull/4740

ktock commented 4 years ago

Seems you are picking right commits. 57f541b is needed if you want to enable the remote snapshotter without facing the issue of label size limitation (https://github.com/containerd/stargz-snapshotter/issues/144). (but this patch is already merged into the master branch of containerd/containerd)

shahzzzam commented 4 years ago

@ktock I see. @estesp mentioned https://github.com/containerd/containerd/pull/4740#issuecomment-729295050 https://github.com/containerd/containerd/pull/4740#issuecomment-729295050 that I need this in release/1.4 of CRI first, then vendor it out to containerd version/1.4. So we need all the commits related to the change e398fb to go into release/1.4

shahzzzam commented 4 years ago

Why is the CI failing?

ktock commented 4 years ago

@shahzzzam https://github.com/containerd/cri/pull/1607

estesp commented 4 years ago

We should get #1607 merged today and then you can rebase to get a clean CI run.

thaJeztah commented 4 years ago

@shahzzzam https://github.com/containerd/cri/pull/1607 was merged, can you try rebasing?

shahzzzam commented 4 years ago

Thanks! It's passing the CIs @thaJeztah @estesp