GoogleContainerTools / GoogleContainerTools/skaffold

The `--cache-artifacts=false` flag provoking race-condition and leads to the deletion of wrong images

Open
#9,251 0 comments 1 reaction 0 assignees View on GitHub
Dominant language
Go
Stars
15.9k
Forks
1.7k
Avg merge
3d 9h
Merged PRs (30d)
10

Description

### Expected behavior

The `--cache-artifacts=false` flag works as specified in the docs.

### Actual behavior

Some percent of the `skaffold run --cache-artifcats=false` end up with error `ErrImagePull`. For our project broken build happens in 10% of cases. Depends on the project.

### Information

- Skaffold version: v2.7.1
- Operating system: Linux, Ubuntu 20.04.6
- Installed via: GitHub Releases
- Contents of skaffold.yaml: Not provided

### Steps to reproduce the behavior

1. Slow down image prune in the code.
2. Run `skaffold run --cache-artifcats=false` many times.

In the file `pkg/skaffold/build/local/local.go` there is a code:
```
func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact) build.ArtifactBuilder {
if b.prune {
b.localPruner.asynchronousCleanupOldImages(ctx, []string{a.ImageName})
}
builder := build.WithLogFile(b.buildArtifact, b.muted)
return builder
```

It eventually calls `cleanup` with `sync = false`:
```
func (p *pruner) cleanup(ctx context.Context, sync bool, artifacts []string) {
toPrune := p.collectImagesToPrune(ctx, artifacts)
if len(toPrune) == 0 {
return
}

if sync {
err := p.runPrune(ctx, toPrune)
if err != nil {
log.Entry(ctx).Debugf("Failed to prune: %v", err)
}
} else {
go func() {
err := p.runPrune(ctx, toPrune)
if err != nil {
log.Entry(ctx).Debugf("Failed to prune: %v", err)
}
}()
}
}
```

If there are a lot of fat docker images, then the `runPrune` will take some time to delete them.

Meanwhile the rebuild process is running concurrently and might to finish image rebuild _before_ the `runPrune` call completes. If that happens you might see events in wrong order:
- First rebuild the docker image by id
- Second delete the docker image by id

If there were no changes in the docker file then the id of the image have not been changed. And the Skaffold deleted it. Due to that the project will not be able to start due to missing image ( `ErrImagePull`).

It is possible to run the prune with `sync = true` to fix that:
```
diff --git a/pkg/skaffold/build/local/local.go b/pkg/skaffold/build/local/local.go
index 368894b36..768a5d570 100644
--- a/pkg/skaffold/build/local/local.go
+++ b/pkg/skaffold/build/local/local.go
@@ -32,7 +32,7 @@ import (
// its checksum. It streams build progress to the writer argument.
func (b *Builder) Build(ctx context.Context, out io.Writer, a *latest.Artifact) build.ArtifactBuilder {
if b.prune {
- b.localPruner.asynchronousCleanupOldImages(ctx, []string{a.ImageName})
+ b.localPruner.synchronousCleanupOldImages(ctx, []string{a.ImageName})
}
builder := build.WithLogFile(b.buildArtifact, b.muted)
return builder
```

But it kills the performance, because docker is slow to delete the images.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.