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

skaffold ko builder does not detect changes to go:embed files #7836

Open
jsok opened this issue Sep 2, 2022 · 5 comments
Open

skaffold ko builder does not detect changes to go:embed files #7836

jsok opened this issue Sep 2, 2022 · 5 comments

Comments

@jsok
Copy link

jsok commented Sep 2, 2022

Expected behavior

Making a change to a file embedded by the embed package should cause skaffold to produce a new image.

Actual behavior

skaffold build instead logs:

Checking cache...
 - app: Found remotely

and no new image is produced.

Information

  • Skaffold version: 1.39.2
  • Operating system: macOS & Linux
  • Installed via: Homebrew
  • Contents of skaffold.yaml:
apiVersion: skaffold/v2beta28
kind: Config
build:
  artifacts:
    - image: app
      ko:
        dir: .
        main: ./app/cmd
        env:
          - CGO_ENABLED=0
        flags:
          - -buildvcs=false
          - -trimpath
          - -v
        ldflags:
          - -buildid=
          - -extldflags="static"
          - -s
          - -w

Steps to reproduce the behavior

  1. clone https://github.com/jsok/skaffold-ko-embed-bug
  2. Run a local registry: go run github.com/google/go-containerregistry/cmd/registry@latest -port 1338
  3. skaffold build -d localhost:1338
Generating tags...
 - app -> localhost:1338/app:latest
Some taggers failed. Rerun with -vdebug for errors.
Checking cache...
 - app: Not found. Building
Starting build...
Building [app]...
No matching credentials were found, falling back on anonymous
Using base gcr.io/distroless/static:nonroot@sha256:1f580b0a1922c3e54ae15b0758b5747b260bd99d39d40c2edb3e7f6e2452298b for github.com/jsok/skaffold-ko-embed-bug
Using build config app for github.com/jsok/skaffold-ko-embed-bug
Building github.com/jsok/skaffold-ko-embed-bug for linux/amd64
Publishing localhost:1338/app:latest
Published localhost:1338/app@sha256:b772b3a87a43fb1c2708ee9651ce345776ea80fab364972b15a412d87e36f646
Build [app] succeeded
  1. Make an edit to hello.txt (the embedded file)
  2. skaffold build -d localhost:1338
Generating tags...
 - app -> localhost:1338/app:latest
Some taggers failed. Rerun with -vdebug for errors.
Checking cache...
 - app: Found Remotely

Fixes considered

Adding dependencies to the skaffold.yaml like:

  - image: app
    ko:
      dependencies:
        paths:
        - '**/*.go'
        - hello.txt

Does fix the problem, but this is tedious to maintain for larger projects.

ko natively does not exhibit this behaviour, it picks up embedded file changes and produces new images correctly.

jsok added a commit to jsok/skaffold-ko-embed-bug that referenced this issue Sep 2, 2022
@jsok
Copy link
Author

jsok commented Sep 2, 2022

Digging into this a bit, it seems that by default skaffold only consider Go files:

Paths: []string{"**/*.go"},

The information about embedded files is readily available via go list or via golang.org/x/tools which skaffold already depends on, see https://pkg.go.dev/golang.org/x/[email protected]/go/packages#Package

> go list -json . | jq ."EmbedPatterns"
[
  "hello.txt"
]

So a potential solution here would be to introspect the packages and automatically add embed patterns to the dependencies list?

@halvards
Copy link
Contributor

halvards commented Sep 2, 2022

Thanks for raising this, and for the repro repo.

The dependencies defaults were chosen to be conservative and not fail builds due to patterns that don't match any files. However, if embedded files are not present, then go build fails, so that's an argument for including them by default.

We'll look into this, and also whether it should apply to other builders - e.g., buildpacks when building Go apps.

@halvards
Copy link
Contributor

halvards commented Sep 2, 2022

Also, @jsok, since you're in Sydney - feel free to drop by here and say hi 😄 https://www.meetup.com/devops-sydney/events/287892691/

@timuthy
Copy link

timuthy commented Sep 23, 2022

@halvards, I just found this Github issue because of a similar problem I faced with static assets. I'd appreciate if you could discuss considering them as well.

timuthy added a commit to timuthy/gardener that referenced this issue Sep 23, 2022
Static assets as well as embedded files should be maintained as
dependencies for Skaffold, so that every time such a file changes
the image is re-built accordingly.
See GoogleContainerTools/skaffold#7836.
gardener-prow bot pushed a commit to gardener/gardener that referenced this issue Sep 23, 2022
Static assets as well as embedded files should be maintained as
dependencies for Skaffold, so that every time such a file changes
the image is re-built accordingly.
See GoogleContainerTools/skaffold#7836.
@halvards
Copy link
Contributor

Thanks for the comment @timuthy.

To watch static assets you can specify kodata/**/* as a watch pattern. The ko builder also supports syncing static assets (without image rebuilds) in dev mode: https://skaffold.dev/docs/pipeline-stages/builders/ko/#file-sync

ialidzhikov added a commit to ialidzhikov/gardener-extension-registry-cache that referenced this issue Oct 3, 2023
Currently, the ko builder does not detect changes to `go:embed` files (see GoogleContainerTools/skaffold#7836). This makes impossible the development of some scripts that are used later by `go:embed`.
This PR inspires from gardener/gardener#6735 and gardener/gardener#6781 and does the same setup for the registry-cache extension.
ialidzhikov added a commit to ialidzhikov/gardener-extension-registry-cache that referenced this issue Oct 5, 2023
Currently, the ko builder does not detect changes to `go:embed` files (see GoogleContainerTools/skaffold#7836). This makes impossible the development of some scripts that are used later by `go:embed`.
This PR inspires from gardener/gardener#6735 and gardener/gardener#6781 and does the same setup for the registry-cache extension.
dimitar-kostadinov pushed a commit to ialidzhikov/gardener-extension-registry-cache that referenced this issue Oct 6, 2023
Currently, the ko builder does not detect changes to `go:embed` files (see GoogleContainerTools/skaffold#7836). This makes impossible the development of some scripts that are used later by `go:embed`.
This PR inspires from gardener/gardener#6735 and gardener/gardener#6781 and does the same setup for the registry-cache extension.
gardener-prow bot pushed a commit to gardener/gardener-extension-registry-cache that referenced this issue Oct 6, 2023
* skaffold: Rebuild when there are changes to `go:embed` files

Currently, the ko builder does not detect changes to `go:embed` files (see GoogleContainerTools/skaffold#7836). This makes impossible the development of some scripts that are used later by `go:embed`.
This PR inspires from gardener/gardener#6735 and gardener/gardener#6781 and does the same setup for the registry-cache extension.

* Add skaffold dependency

---------

Co-authored-by: Dimitar Kostadinov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants