Spegel does not advertise content loaded using `ctr image import`

[brandond]

Spegel version

All since #888

Kubernetes distribution

k3s, rke2

Kubernetes version

n/a

CNI

n/a

Describe the bug

• When spegel gets a ContentCreate event from containerd, it does not advertise the layer content if the event lacks a Digest:
  

  spegel/pkg/oci/containerd.go

  Lines 369 to 372 in decaf73

  	// Just advertise the image if it is a tag reference.
  	if img.Digest == "" {
  	return []OCIEvent{{Type: CreateEvent, Reference: img.Reference}}, nil
  	}

  
  This is a problem, as the ImageCreate events generated by containerd when an image is imported from tarball only contain the tag, not the digest. There is no event generated for the image digest, so the image digest and layer content digests are never advertised.
• When containerd imports an image instead of pulling it, it does not add containerd.io/distribution.source.<registry>=<name> labels to the content. These labels are only added by the docker remote. This prevents the content from being properly tracked by spegel, as it applies label-based filters to image content:
  

  spegel/pkg/oci/containerd.go

  Lines 561 to 578 in decaf73

  	func contentLabelsToReferences(l map[string]string, dgst digest.Digest) ([]Reference, error) {
  	refs := []Reference{}
  	for k, v := range l {
  	if !strings.HasPrefix(k, labels.LabelDistributionSource) {
  	continue
  	}
  	ref := Reference{
  	Registry: strings.TrimPrefix(k, labels.LabelDistributionSource+"."),
  	Repository: v,
  	Digest: dgst,
  	}
  	refs = append(refs, ref)
  	}
  	if len(refs) == 0 {
  	return nil, fmt.Errorf("no distribution source labels found for %s", dgst)
  	}
  	return refs, nil
  	}

Combined, these two issues make it so that spegel fails to advertise everything except the tag when content is imported instead of pulled.

[brandond]

Related:

• Image digests pulled by ctr can't be seen through crictl containerd/containerd#8483

[phillebaba]

Containerd seems to behave very differently when images are pulled with CRI vs other methods. It is hard to cover every single use case, when labels are applied differently. I do think that the addition of integration tests for different Containerd versions in #1097 is going to help a lot. Not only have I been dealing with different Containerd versions, but also different ways images makes its way in.

I am fine with fixing this as long as it can be covered in the integration tetsts.

[brandond]

Yeah, unfortunately I don't see a way around this other than going back to the old way of not ignoring tags, and always walking images to find content digests. A bunch of the stuff that you have come to rely on for efficient discovery of resources to advertise is managed by the CRI service.

[brandond]

FYI, we are currently planning to work around this with additional code on the K3s side to manually create digests and label layer content - by walking the image - when K3s imports images from tarballs. But that will not help anyone that uses ctr or nerdctl to import content.

[phillebaba]

I have considered different options to work around this. Either by adding labels on my own or creating my own separate index for this. This was something that really just worked by mistake, as none of the tests actually covered the import flow. My use case for Spegel was always to serve images pulled by CRI, and the e2e tests only cover that use case.

Now that there are actual tests specific to Containerd behavior it would be possible to define expected behavior for non CRI fetching of images. Any new solution to cover the use case would be worthless without tests to stop future regression, so that had to be prioritized. Now that it is done we can discuss possible future solutions. I am personally on the fence when it comes to writing labels to Containerd. Hoping that I can get some time soon to work on this.

[brandond]

It's not terrible to do, it's just a bummer that containerd put this in cri instead of core image/content store.

I agree that spegel shouldn't be pushing digests and labels back into the store, of whatever client is loading the images does not do so.

Would you consider going back to the old image walking behavior, or perhaps at least making that an option?

[phillebaba]

I did some local tests of functionality. Turns out ctr pull is not the issue, it is only ctr import which does not apply the labels to the content. There does not seem to be an option to apply labels to content during import sadly. There are three options to solve this. Either the importer will be responsible for adding labels to content, we make changes upstream in Containerd, or lastly we add some backwards compatible solution that can work around the lack of labels.

Either way it needs to be added to the integration tests to ensure future functionality.

[brandond]

Interesting that it works with pull; I admit I didn't test that bit myself but was just looking at the code in containerd and it seemed to be all in the CRI service.

I think pull and import both have the issue with not creating a digest for the tag?

I wonder if upstream would accept a PR to fix both of these. Could open an issue and see how they respond, I guess.

[pog-charlesinglese]

I'm definitely interested in a remedy to this bug, as it is impacting our deployments using k3s+spegel.

[brandond]

We've already fixed it on the k3s side, at least for images loaded from airgap tarballs.

I do need to call this out in the docs though, we still say that you can import stuff with ctr and expect it to work.