cri-o/cri-o: server/image_pull.go; 130 LoC

Found a possible issue in [cri-o/cri-o](https://www.github.com/cri-o/cri-o) at [server/image_pull.go](https://github.com/cri-o/cri-o/blob/1c9b71163bc094e935b5d95ccd84cc953d63ffa5/server/image_pull.go#L124-L253)

Below is the message reported by the analyzer for this snippet of code. Beware that the analyzer only reports the first issue it finds, so please do not limit your consideration to the contents of the below message.

> range-loop variable img used in defer or goroutine at line 183

[Click here to see the code in its original context.](https://github.com/cri-o/cri-o/blob/1c9b71163bc094e935b5d95ccd84cc953d63ffa5/server/image_pull.go#L124-L253)

Click here to show the 130 line(s) of Go which triggered the analyzer.

```go
for _, img := range images {
var tmpImg imageTypes.ImageCloser
tmpImg, err = s.StorageImageServer().PrepareImage(&sourceCtx, img)
if err != nil {
// We're not able to find the image remotely, check if it's
// available locally, but only for localhost/ prefixed ones.
// This allows pulling localhost/ prefixed images even if the
// `imagePullPolicy` is set to `Always`.
if strings.HasPrefix(img, localRegistryPrefix) {
if _, err := s.StorageImageServer().ImageStatus(
s.config.SystemContext, img,
); err == nil {
pulled = img
break
}
}
log.Debugf(ctx, "error preparing image %s: %v", img, err)
tryIncrementImagePullFailureMetric(ctx, img, err)
continue
}
defer tmpImg.Close()

var storedImage *storage.ImageResult
storedImage, err = s.StorageImageServer().ImageStatus(s.config.SystemContext, img)
if err == nil {
tmpImgConfigDigest := tmpImg.ConfigInfo().Digest
if tmpImgConfigDigest.String() == "" {
// this means we are playing with a schema1 image, in which
// case, we're going to repull the image in any case
log.Debugf(ctx, "image config digest is empty, re-pulling image")
} else if tmpImgConfigDigest.String() == storedImage.ConfigDigest.String() {
log.Debugf(ctx, "image %s already in store, skipping pull", img)
pulled = img

// Skipped digests metrics
tryRecordSkippedMetric(ctx, img, tmpImgConfigDigest.String())

// Skipped bytes metrics
if storedImage.Size != nil {
counter, err := metrics.CRIOImagePullsByNameSkipped.GetMetricWithLabelValues(img)
if err != nil {
log.Warnf(ctx, "Unable to write image pull name (skipped) metrics: %v", err)
} else {
counter.Add(float64(*storedImage.Size))
}
}

break
}
log.Debugf(ctx, "image in store has different ID, re-pulling %s", img)
}

// Pull by collecting progress metrics
progress := make(chan imageTypes.ProgressProperties)
defer close(progress)
go func() {
for p := range progress {
if p.Event == imageTypes.ProgressEventSkipped {
// Skipped digests metrics
tryRecordSkippedMetric(ctx, img, p.Artifact.Digest.String())
}
if p.Artifact.Size > 0 {
log.Debugf(ctx, "ImagePull (%v): %s (%s): %v bytes (%.2f%%)",
p.Event, img, p.Artifact.Digest, p.Offset,
float64(p.Offset)/float64(p.Artifact.Size)*100,
)
} else {
log.Debugf(ctx, "ImagePull (%v): %s (%s): %v bytes",
p.Event, img, p.Artifact.Digest, p.Offset,
)
}

// Metrics for every digest
digestCounter, err := metrics.CRIOImagePullsByDigest.GetMetricWithLabelValues(
img, p.Artifact.Digest.String(), p.Artifact.MediaType,
fmt.Sprintf("%d", p.Artifact.Size),
)
if err != nil {
log.Warnf(ctx, "Unable to write image pull digest metrics: %v", err)
} else {
digestCounter.Add(float64(p.OffsetUpdate))
}

// Metrics for the overall image
nameCounter, err := metrics.CRIOImagePullsByName.GetMetricWithLabelValues(
img, fmt.Sprintf("%d", imageSize(tmpImg)),
)
if err != nil {
log.Warnf(ctx, "Unable to write image pull name metrics: %v", err)
} else {
nameCounter.Add(float64(p.OffsetUpdate))
}
}
}()

cgroup := ""

if s.config.SeparatePullCgroup != "" {
if !s.config.CgroupManager().IsSystemd() {
return "", errors.New("--separate-pull-cgroup is supported only with systemd")
}
if s.config.SeparatePullCgroup == "pod" {
cgroup = pullArgs.sandboxCgroup
} else {
cgroup = s.config.SeparatePullCgroup
if !strings.Contains(cgroup, ".slice") {
return "", fmt.Errorf("invalid systemd cgroup %q", cgroup)
}
}
}

_, err = s.StorageImageServer().PullImage(s.config.SystemContext, img, &storage.ImageCopyOptions{
SourceCtx: &sourceCtx,
DestinationCtx: s.config.SystemContext,
OciDecryptConfig: decryptConfig,
ProgressInterval: time.Second,
Progress: progress,
CgroupPull: storage.CgroupPullConfiguration{
UseNewCgroup: s.config.SeparatePullCgroup != "",
ParentCgroup: cgroup,
},
})
if err != nil {
log.Debugf(ctx, "error pulling image %s: %v", img, err)
tryIncrementImagePullFailureMetric(ctx, img, err)
continue
}
pulled = img
break
}

```

Leave a reaction on this issue to contribute to the project by classifying this instance as a **Bug** :-1:, **Mitigated** :+1:, or **Desirable Behavior** :rocket:
See the descriptions of the classifications [here](https://github.com/github-vet/rangeclosure-findings#how-can-i-help) for more information.

commit ID: 1c9b71163bc094e935b5d95ccd84cc953d63ffa5

Start in server/image_pull.go at lines 124-253, especially the range over images and the defer and goroutine reported at line 183. Review the analyzer finding and the surrounding image-pull flow first. Done means the reported range-loop capture issue is addressed without changing the intended pull and progress behavior, and the relevant project checks pass.

Written by the indexing model from the issue text.