Skip to content

Commit 3d2e188

Browse files
fidenciok8s-infra-cherrypick-robot
fidencio
authored and
k8s-infra-cherrypick-robot
committed
cri: Use the runtimeHandler parameter in PullImage
The runtimeHandler parameter was added to PullImage() but never used. Instead, the code relied on an experimental annotation (io.containerd.cri.runtime-handler) passed in the pod sandbox config. This annotation was a workaround because CRI's PullImageRequest didn't include the runtime handler. However, since cri-api v0.29.0, the runtime handler is available in the API and passed as a parameter to PullImage(). For backward compatibility with CRI clients that don't yet pass the runtime handler parameter, we fall back to the annotation if the parameter is empty. The annotation-based fallback is deprecated and will be removed in containerd 2.5. Signed-off-by: Wedson Almeida Filho <[email protected]> Signed-off-by: Fabiano Fidêncio <[email protected]>
1 parent fb33d37 commit 3d2e188

3 files changed

Lines changed: 62 additions & 23 deletions

File tree

‎internal/cri/annotations/annotations.go‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,10 @@ const (
8888
// See https://github.com/containerd/containerd/issues/6657 and https://github.com/containerd/containerd/pull/6899 for details.
8989
// The value of this annotation should be the runtime for sandboxes.
9090
// e.g. for [plugins.cri.containerd.runtimes.runc] runtime config, this value should be runc
91-
// TODO: we should deprecate this annotation as soon as kubelet supports passing RuntimeHandler from PullImageRequest
91+
//
92+
// Deprecated: Since cri-api v0.29.0 (Kubernetes 1.29+), kubelet passes the RuntimeHandler
93+
// in PullImageRequest. This annotation is only used as a fallback for older CRI clients
94+
// and will be removed in containerd 2.5.
9295
RuntimeHandler = "io.containerd.cri.runtime-handler"
9396

9497
// WindowsHostProcess is used by hcsshim to identify windows pods that are running HostProcesses

‎internal/cri/server/images/image_pull.go‎

Lines changed: 28 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func (c *CRIImageService) PullImage(ctx context.Context, name string, credential
165165
return "", fmt.Errorf("failed to parse image_pull_progress_timeout %q: %w", c.config.ImagePullProgressTimeout, err)
166166
}
167167

168-
snapshotter, err := c.snapshotterFromPodSandboxConfig(ctx, ref, sandboxConfig)
168+
snapshotter, err := c.snapshotterFromPodSandboxConfig(ctx, ref, sandboxConfig, runtimeHandler)
169169
if err != nil {
170170
return "", err
171171
}
@@ -821,25 +821,41 @@ func (rt *pullRequestReporterRoundTripper) RoundTrip(req *http.Request) (*http.R
821821
return resp, err
822822
}
823823

824-
// Given that runtime information is not passed from PullImageRequest, we depend on an experimental annotation
825-
// passed from pod sandbox config to get the runtimeHandler. The annotation key is specified in configuration.
826-
// Once we know the runtime, try to override default snapshotter if it is set for this runtime.
824+
// snapshotterFromPodSandboxConfig returns the snapshotter to use for the given
825+
// runtime handler. If a runtime-specific snapshotter is configured, it will be
826+
// returned; otherwise the default snapshotter is used.
827+
//
828+
// The runtimeHandler parameter (from CRI PullImageRequest, available since cri-api v0.29.0)
829+
// takes precedence. If empty, we fall back to the experimental annotation for backward
830+
// compatibility with clients that don't pass the runtime handler.
831+
//
832+
// Deprecated: The annotation-based fallback (io.containerd.cri.runtime-handler) is
833+
// deprecated and will be removed in containerd 2.5.
834+
//
827835
// See https://github.com/containerd/containerd/issues/6657
828836
func (c *CRIImageService) snapshotterFromPodSandboxConfig(ctx context.Context, imageRef string,
829-
s *runtime.PodSandboxConfig) (string, error) {
837+
s *runtime.PodSandboxConfig, runtimeHandler string) (string, error) {
830838
snapshotter := c.config.Snapshotter
831-
if s == nil || s.Annotations == nil {
839+
if s == nil {
832840
return snapshotter, nil
833841
}
834842

835-
// TODO(kiashok): honor the new CRI runtime handler field added to v0.29.0
836-
// for image pull per runtime class support.
837-
runtimeHandler, ok := s.Annotations[annotations.RuntimeHandler]
838-
if !ok {
839-
return snapshotter, nil
843+
// If runtimeHandler parameter is empty, fall back to annotation for backward compatibility
844+
if runtimeHandler == "" {
845+
if s.Annotations == nil {
846+
return snapshotter, nil
847+
}
848+
var ok bool
849+
runtimeHandler, ok = s.Annotations[annotations.RuntimeHandler]
850+
if !ok {
851+
return snapshotter, nil
852+
}
853+
log.G(ctx).Warnf("Using deprecated annotation %q for runtime handler. "+
854+
"This will be removed in a future release (2.5). "+
855+
"Please update your CRI client to pass runtime handler in PullImageRequest.",
856+
annotations.RuntimeHandler)
840857
}
841858

842-
// TODO: Ensure error is returned if runtime not found?
843859
if c.runtimePlatforms != nil {
844860
if p, ok := c.runtimePlatforms[runtimeHandler]; ok && p.Snapshotter != snapshotter {
845861
snapshotter = p.Snapshotter

‎internal/cri/server/images/image_pull_test.go‎

Lines changed: 30 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -381,42 +381,62 @@ func TestSnapshotterFromPodSandboxConfig(t *testing.T) {
381381
tests := []struct {
382382
desc string
383383
podSandboxConfig *runtime.PodSandboxConfig
384+
runtimeHandler string
384385
expectedSnapshotter string
385386
expectedErr bool
386387
}{
387388
{
388389
desc: "should return default snapshotter for nil podSandboxConfig",
390+
runtimeHandler: "",
389391
expectedSnapshotter: defaultSnapshotter,
390392
},
391393
{
392-
desc: "should return default snapshotter for nil podSandboxConfig.Annotations",
394+
desc: "should return default snapshotter for empty runtimeHandler",
393395
podSandboxConfig: &runtime.PodSandboxConfig{},
396+
runtimeHandler: "",
394397
expectedSnapshotter: defaultSnapshotter,
395398
},
396399
{
397-
desc: "should return default snapshotter for empty podSandboxConfig.Annotations",
400+
desc: "should return default snapshotter for runtime not found",
401+
podSandboxConfig: &runtime.PodSandboxConfig{},
402+
runtimeHandler: "runtime-not-exists",
403+
expectedSnapshotter: defaultSnapshotter,
404+
},
405+
{
406+
desc: "should return snapshotter for existing runtime",
407+
podSandboxConfig: &runtime.PodSandboxConfig{},
408+
runtimeHandler: "existing-runtime",
409+
expectedSnapshotter: runtimeSnapshotter,
410+
},
411+
{
412+
desc: "should fall back to annotation when runtimeHandler is empty",
398413
podSandboxConfig: &runtime.PodSandboxConfig{
399-
Annotations: make(map[string]string),
414+
Annotations: map[string]string{
415+
annotations.RuntimeHandler: "existing-runtime",
416+
},
400417
},
401-
expectedSnapshotter: defaultSnapshotter,
418+
runtimeHandler: "",
419+
expectedSnapshotter: runtimeSnapshotter,
402420
},
403421
{
404-
desc: "should return default snapshotter for runtime not found",
422+
desc: "should prefer runtimeHandler parameter over annotation",
405423
podSandboxConfig: &runtime.PodSandboxConfig{
406424
Annotations: map[string]string{
407425
annotations.RuntimeHandler: "runtime-not-exists",
408426
},
409427
},
410-
expectedSnapshotter: defaultSnapshotter,
428+
runtimeHandler: "existing-runtime",
429+
expectedSnapshotter: runtimeSnapshotter,
411430
},
412431
{
413-
desc: "should return snapshotter provided in podSandboxConfig.Annotations",
432+
desc: "should return default when annotation has unknown runtime and runtimeHandler is empty",
414433
podSandboxConfig: &runtime.PodSandboxConfig{
415434
Annotations: map[string]string{
416-
annotations.RuntimeHandler: "existing-runtime",
435+
annotations.RuntimeHandler: "runtime-not-exists",
417436
},
418437
},
419-
expectedSnapshotter: runtimeSnapshotter,
438+
runtimeHandler: "",
439+
expectedSnapshotter: defaultSnapshotter,
420440
},
421441
}
422442

@@ -428,7 +448,7 @@ func TestSnapshotterFromPodSandboxConfig(t *testing.T) {
428448
Platform: platforms.DefaultSpec(),
429449
Snapshotter: runtimeSnapshotter,
430450
}
431-
snapshotter, err := cri.snapshotterFromPodSandboxConfig(context.Background(), "test-image", tt.podSandboxConfig)
451+
snapshotter, err := cri.snapshotterFromPodSandboxConfig(context.Background(), "test-image", tt.podSandboxConfig, tt.runtimeHandler)
432452
assert.Equal(t, tt.expectedSnapshotter, snapshotter)
433453
if tt.expectedErr {
434454
assert.Error(t, err)

0 commit comments

Comments
 (0)