Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug in tryImportMissing behaviour for derived images #9462

Open
pomyslowynick opened this issue Jul 2, 2024 · 0 comments
Open

Bug in tryImportMissing behaviour for derived images #9462

pomyslowynick opened this issue Jul 2, 2024 · 0 comments

Comments

@pomyslowynick
Copy link

pomyslowynick commented Jul 2, 2024

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 and then populate the cache with it here.
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 which will look for the same tag we previously passed on this line and compare it with the digest of itself here.

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:
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=<some-registry>
  3. Introduce some changes to base image, like adding a RUN step to the Dockerfile
  4. Commit changes
  5. skaffold build --default-repo=<some-registry>
  6. Change the base image, you can set image to ubuntu:latest instead of alpine
  7. skaffold build --default-repo=<some-registry>

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

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant