containerd / containerd/containerd
CRI: pinned label never applied when pinned_images config value has no tag
- Dominant language
- Go
- Stars
- 21.3k
- Forks
- 4.1k
- Avg merge
- 2d 4h
- Merged PRs (30d)
- 92
Description
## Description
When a `pinned_images` config value has no explicit tag (e.g. `localhost/kubernetes/pause`), the pinned label (`io.cri-containerd.pinned=pinned`) is never written onto the image. This means kubelet sees `Pinned: false` from `ImageStatus` and will garbage collect the image under disk pressure.
## Root cause
`getLabels` (`internal/cri/server/images/image_pull.go#L421-L428`) does a raw string comparison between the config value and the image ref:
\`\`\`go
for _, pinned := range c.config.PinnedImages {
if pinned == name { // exact string match, no normalization
labels[crilabels.PinnedImageLabelKey] = crilabels.PinnedImageLabelValue
}
}
\`\`\`
But `name` at the call site has already been normalized through `ParseDockerRef`, which appends `:latest` to any tagless ref:
\`\`\`go
namedRef, err := distribution.ParseDockerRef(name)
ref := namedRef.String() // "localhost/kubernetes/pause" → "localhost/kubernetes/pause:latest"
labels := c.getLabels(ctx, ref) // comparison always fails
\`\`\`
So `"localhost/kubernetes/pause"` (config) never equals `"localhost/kubernetes/pause:latest"` (normalized ref). The same mismatch affects the `UpdateImage` path (called from `ImageCreate`/`ImageUpdate` events and `CheckImages` at startup), so the label is never applied via any code path.
## Regression
This was introduced in commit ad4c9f8a9deaaafea37e4afce0b0b2c0d128e436, which refactored from a single `SandboxImage` string to the generic `PinnedImages` list. The **old code correctly normalized** the config value before comparing:
\`\`\`go
// Old code — worked correctly
sandboxNamedRef, err := distribution.ParseDockerRef(configSandboxImage)
sandboxRef := sandboxNamedRef.String() // normalizes tagless ref → :latest
if sandboxRef == name { // apples-to-apples
labels[crilabels.PinnedImageLabelKey] = crilabels.PinnedImageLabelValue
}
\`\`\`
The refactor dropped the `ParseDockerRef` call on the config side.
## The test that masked it
The same commit updated the "without tag" test case but worked around the regression instead of fixing it — by adding the already-normalized form to `pinnedImages` so the exact match would succeed:
https://github.com/containerd/containerd/blob/ad4c9f8a9deaaafea37e4afce0b0b2c0d128e436/pkg/cri/server/images/image_pull_test.go#L505-L508
\`\`\`go
// Test is named "without tag" but passes because :latest is explicitly listed
pinnedImages: []string{"k8s.gcr.io/pause", "k8s.gcr.io/pause:latest"},
pullImageName: "k8s.gcr.io/pause:latest",
\`\`\`
The tagless entry `"k8s.gcr.io/pause"` matches nothing and is dead code. The correct test would be:
\`\`\`go
pinnedImages: []string{"k8s.gcr.io/pause"}, // tagless only, as a user would configure
pullImageName: "k8s.gcr.io/pause:latest", // normalized form
// expected: still pinned — this FAILS with current code
\`\`\`
## Impact
1. Image exists in the store and is used as the sandbox image — everything appears healthy
2. Kubelet's image GC sees `Pinned: false` from `ImageStatus`
3. Under disk pressure, kubelet removes the pause/sandbox image
4. Next pod creation: `LocalResolve` fails, pull from local registry fails, pod creation fails across the entire node
5. Failure is silent until disk pressure is triggered
## Affected code
- Broken `getLabels`: https://github.com/containerd/containerd/blob/bc69a52680912da0c1ad9b876b21db5f7c0db99c/internal/cri/server/images/image_pull.go#L421-L428
- Regression commit: https://github.com/containerd/containerd/commit/ad4c9f8a9deaaafea37e4afce0b0b2c0d128e436
## Suggested fix
Normalize each pinned image config value through `ParseDockerRef` before comparing, restoring the behavior that existed before the refactor:
\`\`\`go
func (c *CRIImageService) getLabels(ctx context.Context, name string) map[string]string {
labels := map[string]string{crilabels.ImageLabelKey: crilabels.ImageLabelValue}
for _, pinned := range c.config.PinnedImages {
normalizedPinned, err := docker.ParseDockerRef(pinned)
if err == nil && normalizedPinned.String() == name {
labels[crilabels.PinnedImageLabelKey] = crilabels.PinnedImageLabelValue
}
}
return labels
}
\`\`\`
## Version
Reproducible on current `main` (bc69a52680912da0c1ad9b876b21db5f7c0db99c). Present since ad4c9f8a9deaaafea37e4afce0b0b2c0d128e436 (merged during containerd v2 development).
Contributor guide
Assessment
This issue has not been assessed yet.