GoogleContainerTools / GoogleContainerTools/skaffold

Bug in `tryImportMissing` behaviour for derived images

Open
#9,462 1 comment 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

Whenever a base image (required) changes, the derived image (requiring) should be rebuilt, if there has been structural changes to the base image.

### Actual behavior

When `tryImportMissing` is enabled with `variant: AbbrevTreeSha` this doesn't always happen, when it should.
Changes to base image directory will result in changes to the git tree sha, which then will generate a new tag for the base image and trigger a rebuild. The derived image directory, and git tree sha, will remain the same.

`tryImportMissing` will cause the derived image runner to lookup the remote digest based solely on the tag for the derived image, on this [line](https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/build/cache/lookup.go?plain=1#L174) and then populate the cache with it [here](https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/build/cache/lookup.go?plain=1#L180).
Since the `tag` for derived image hasn't changed it will be an existing image, without the base image changes.

Next, `lookupRemote` is run [here](https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/build/cache/lookup.go?plain=1#L94) which will look for the same tag we previously passed on [this line](https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/build/cache/lookup.go?plain=1#L123) and compare it with the digest of itself [here](https://github.com/GoogleContainerTools/skaffold/blob/main/pkg/skaffold/build/cache/lookup.go?plain=1#L125).

This is superfluous/wrong, because we are looking up the exact same image twice and comparing it against itself.

This results in a "found" image, which is then appended to the local cache, effectively poisoning it against any future changes to the base image, until the derived image directory changes.

This applies to `gitCommit` and `inputDigest` strategies as well, as it will retag an image, instead of rebuilding it.

I don't have a fix in mind for this as of yet, but would love to work on a PR to fix it, if I could get some help with it :)

### Information

- Skaffold version:un `v2.12.0`
- Operating system: Ubuntu 22.04.4 LTS
- Installed via: Downloaded binary
- Contents of skaffold.yaml:

```yaml
apiVersion: skaffold/v4beta10
kind: Config
build:
artifacts:
- image: base
context: images/base/base/
- image: default
context: images/apps/default/
requires:
- image: base
alias: BASE
tagPolicy:
gitCommit:
variant: AbbrevTreeSha
platforms:
- linux/amd64
local:
useDockerCLI: true
useBuildkit: true
tryImportMissing: true
```

### Steps to reproduce the behavior

1. git clone https://github.com/pomyslowynick/skaffold_bug
2. `skaffold build --default-repo=`
3. Introduce some changes to base image, like adding a `RUN` step to the Dockerfile
4. Commit changes
5. `skaffold build --default-repo=`
6. Change the base image, you can set image to `ubuntu:latest` instead of `alpine`
7. `skaffold build --default-repo=`

Derived image won't be rebuilt, but base image will.

Clearing the cache and then setting `tryImportMissing: false` will "fix" it.

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.