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

Local docker builder should support using the previously built image as a cache source #6809

Closed
artemkoru opened this issue Nov 6, 2021 · 10 comments · Fixed by #6904
Closed
Labels
area/build area/cache area/local build/docker help wanted We would love to have this done, but don't have the bandwidth, need help from contributors kind/feature-request priority/p2 May take a couple of releases

Comments

@artemkoru
Copy link
Contributor

artemkoru commented Nov 6, 2021

I want to make cache of docker’s layers «builder» stage using the cache-from flag, like this #3831.

I can do like this:

apiVersion: skaffold/v2beta24
kind: Config
metadata:
  name: multistage-build-and-cache
profiles:
  # Development environment
  - name: dev
    activation:
      - env: GIT_BRANCH_PREFIX=develop
    build:
      artifacts:
        - image: multistage-build-and-cache-builder
          context: .
          hooks:
            after:
              - command:
                  - sh
                  - -cx
                  - |
                    docker tag $SKAFFOLD_IMAGE multistage-build-and-cache:develop-builder-cache
                    docker push multistage-build-and-cache:develop-builder-cache
          docker:
            dockerfile: Dockerfile
            target: builder
            cacheFrom:
              - "multistage-build-and-cache:develop-builder-cache"
            buildArgs:
              BUILDKIT_INLINE_CACHE: "1"
        - image: multistage-build-and-cache
          context: .
          docker:
            dockerfile: Dockerfile
      local:
        useBuildkit: true
      tagPolicy:
        envTemplate:
          template: "develop-{{.GIT_COMMIT}}"

It works fine, but we also have dynamic environments. In this case the skaffold.yaml should look like this:

apiVersion: skaffold/v2beta24
kind: Config
metadata:
  name: multistage-build-and-cache
profiles:
  # Dynamic environment for any git branch
  - name: branch
    activation:
      - env: GIT_BRANCH_PREFIX=feature
      - env: GIT_BRANCH_PREFIX=bugfix
    build:
      artifacts:
        - image: multistage-build-and-cache-builder
          context: .
          hooks:
            after:
              - command:
                  - sh
                  - -cx
                  - |
                    docker tag $SKAFFOLD_IMAGE $SKAFFOLD_IMAGE_REPO/multistage-build-and-cache:$GIT_BRANCH_DASH-builder-cache
                    docker push $SKAFFOLD_IMAGE_REPO/multistage-build-and-cache:$GIT_BRANCH_DASH-builder-cache
          docker:
            dockerfile: Dockerfile
            target: builder
            cacheFrom:
              # THE PROBLEM IS HERE: Using env-vars unsupported in skaffold v1.34.0
              - "{{.SKAFFOLD_DEFAULT_REPO}}/multistage-build-and-cache:{{.GIT_BRANCH_DASH}}-builder-cache"
            buildArgs:
              BUILDKIT_INLINE_CACHE: "1"
        - image: multistage-build-and-cache
          context: .
          docker:
            dockerfile: Dockerfile
      local:
        useBuildkit: true
      tagPolicy:
        envTemplate:
          template: "{{.GIT_BRANCH_DASH}}-{{.GIT_COMMIT}}"

If I try to use this configuration, I got the error:

getting imageID for "{{.SKAFFOLD_DEFAULT_REPO}}/multistage-build-and-cache:{{.GIT_BRANCH_DASH}}-builder-cache": getting imageID for {{.SKAFFOLD_DEFAULT_REPO}}/multistage-build-and-cache:{{.GIT_BRANCH_DASH}}-builder-cache: Error response from daemon: no such image: {{.SKAFFOLD_DEFAULT_REPO}}/multistage-build-and-cache:{{.GIT_BRANCH_DASH}}-builder-cache: invalid reference format

I would suggest to allow using environment variables in the parameter build.artifacts[*].docker.cacheFrom. It solves this problem.
What do you think about this? Thanks.

As I understand these changes are enough artemkoru@7af8afd. I can create PR, if it's ok.

Sample project here: https://github.com/artemkoru/multistage-build-and-cache

P.S. Also will be great if we will can add some additional docker-tags to each artifact, which was suggested here #1269

@briandealwis
Copy link
Member

@artemkoru would you mind taking a look at the solution implemented in #5903 for GCB? It seems more straightforward and less error-prone.

@artemkoru
Copy link
Contributor Author

artemkoru commented Nov 11, 2021

Hi, @briandealwis!

Unfortunately we can't use GCB! Our pipelines works on Github Actions, also we don't use self-hosted custom runner (We can't use "local cache").

We try to use this Github action for caching Docker layers. But we are faced with the problem, which is that the size of the cache (docker images) every time increases more and more. The problem describes here. We tried some of the solutions suggested there but ended up giving up.

I understand that using the Skaffold build hook for adding additional tags looks a little strange. But it's temporary solution and I can't do this via Skaffold any other way. Add some additional docker-tags to each artifact will be better solution when it will come.

All I want to suggest it's adding support to allow using environment variables in the parameter build.artifacts[*].docker.cacheFrom. As I see it does not contradict the Skaffold approaches. I think it will be helpful for other users in similar cases.

Will be grate if you accept these changes. Thanks!

@briandealwis
Copy link
Member

Sorry @artemkoru I meant implement the equivalent handling for the docker builder too.

@artemkoru
Copy link
Contributor Author

@briandealwis Thanks for your comment i didn't understand it.

I try to check it with this skaffold.yaml:

apiVersion: skaffold/v2beta25
kind: Config
metadata:
  name: multistage-build-and-cache
build:
  artifacts:
    - image: builder
      context: .
      docker:
        dockerfile: Dockerfile
        target: builder
        cacheFrom:
          - builder
          #- artemkoru/builder:latest #we don't use latest tags
        buildArgs:
          BUILDKIT_INLINE_CACHE: "1"
    - image: app
      context: .
      requires:
        - image: builder
          alias: BUILDER
      docker:
        dockerfile: Dockerfile
  local:
    useBuildkit: true
  tagPolicy:
    gitCommit:
      ignoreChanges: true

My Dockerfile:

FROM busybox as builder

# very log build command here
RUN sleep 15

COPY somefile .

FROM busybox

COPY --from=builder /somefile / 

I always get this warning and error messages:

WARN[0003] cacheFrom image couldn't be pulled: builder   subtask=-1 task=DevLoop
...
=> ERROR importing cache manifest from builder
...

Full logs:

$ skaffold build
Generating tags...
 - builder -> artemkoru/builder:8b64a0f
 - app -> artemkoru/app:8b64a0f
Checking cache...
 - builder: Not found. Building
 - app: Not found. Building
Starting build...
Building [builder]...
WARN[0003] cacheFrom image couldn't be pulled: builder   subtask=-1 task=DevLoop
[+] Building 17.3s (11/11) FINISHED                                                                   
 => [internal] load build definition from Dockerfile                                             0.0s
 => => transferring dockerfile: 176B                                                             0.0s
 => [internal] load .dockerignore                                                                0.0s
 => => transferring context: 2B                                                                  0.0s
 => [internal] load metadata for docker.io/library/busybox:latest                                0.0s
 => ERROR importing cache manifest from builder                                                  1.8s
 => [builder 1/3] FROM docker.io/library/busybox                                                 0.0s
 => [internal] load build context                                                                0.0s
 => => transferring context: 72B                                                                 0.0s
 => [auth] library/builder:pull token for registry-1.docker.io                                   0.0s
 => [builder 2/3] RUN sleep 15                                                                  15.3s
 => [builder 3/3] COPY somefile .                                                                0.0s
 => exporting to image                                                                           0.0s
 => => exporting layers                                                                          0.0s
 => => writing image sha256:30b503a677d4e9df2cf3ea3edb28642d32ef88d12dfd69ff05b69bee999c39d9     0.0s
 => => naming to docker.io/artemkoru/builder:8b64a0f                                             0.0s
 => exporting cache                                                                              0.0s
 => => preparing build cache for export                                                          0.0s
------
 > importing cache manifest from builder:
------
The push refers to repository [docker.io/artemkoru/builder]
69988592065a: Preparing
330186a249cf: Preparing
d94c78be1352: Preparing
69988592065a: Layer already exists
d94c78be1352: Layer already exists
330186a249cf: Pushed
8b64a0f: digest: sha256:e6b08595aefb5a6d53d144c0972b6516770f63983b8f52a84203a4f456401854 size: 941
Building [app]...
[+] Building 0.2s (9/9) FINISHED                                                                      
 => [internal] load build definition from Dockerfile                                             0.0s
 => => transferring dockerfile: 37B                                                              0.0s
 => [internal] load .dockerignore                                                                0.0s
 => => transferring context: 2B                                                                  0.0s
 => [internal] load metadata for docker.io/library/busybox:latest                                0.0s
 => [internal] load build context                                                                0.0s
 => => transferring context: 29B                                                                 0.0s
 => CACHED [builder 1/3] FROM docker.io/library/busybox                                          0.0s
 => CACHED [builder 2/3] RUN sleep 15                                                            0.0s
 => CACHED [builder 3/3] COPY somefile .                                                         0.0s
 => [stage-1 2/2] COPY --from=builder /somefile /                                                0.0s
 => exporting to image                                                                           0.0s
 => => exporting layers                                                                          0.0s
 => => writing image sha256:12ed3333a07d0202ff0369e7bfc09caa59bf34103e312317f621217797270c0c     0.0s
 => => naming to docker.io/artemkoru/app:8b64a0f                                                 0.0s
The push refers to repository [docker.io/artemkoru/app]
69988592065a: Preparing
d94c78be1352: Preparing
d94c78be1352: Layer already exists
69988592065a: Layer already exists
8b64a0f: digest: sha256:40c751811abb855f77b7703c20bd507051aca232c824f66305fc08815442a15e size: 733   

If I run it again, clearing all local caches, the runtime is equivalent. What am I doing wrong?

And the second question: if we will use the customTemplate image tagger how will it find the previous image tag?

Thanks!

@briandealwis briandealwis added area/build area/cache area/local build/docker help wanted We would love to have this done, but don't have the bandwidth, need help from contributors kind/feature-request priority/p2 May take a couple of releases labels Nov 13, 2021
@briandealwis
Copy link
Member

Oops I meant that if you were interested then you could try implementing a fix for the docker builder using the same approach used in the gcb build.

@artemkoru
Copy link
Contributor Author

@briandealw sorry for the delay, as I understand your approach is using the same image tag in the cache that will be built and this doesn't solve my problems that I described above, because any time the image tag will be different. Or am I misunderstanding something?

@briandealwis
Copy link
Member

With #5903, the GCB build will replace an image in the cacheFrom that matches an artifact image name with the actual tagged image. So if we "port" this to the local Docker builder, then we could rewrite your example as (eliding some unnecessary detail):

    build:
      artifacts:
        - image: multistage-build-and-cache-builder
          docker:
            dockerfile: Dockerfile
            target: builder
            cacheFrom:
              - multistage-build-and-cache-builder

Then the multistage-build-and-cache-builder would be expanded to include the default-repo (if specified) and the image tags. The advantage here is that it works correctly regardless of whether the user used --default-repo or not, or if they over-ride the tag policy with --tag XXXX.

Let us know if you'd like to take a stab at bringing this over to the local Docker builder. In the meantime, I'll retitle this issue.

@briandealwis briandealwis changed the title [Feat] Multistage Docker Build Cache Local docker builder should support using the previously built image as a cache source Nov 16, 2021
@artemkoru
Copy link
Contributor Author

Yes, I understand it!

Let's imagine that what you propose has already been implemented and consider how it works.

skaffold.yaml;

apiVersion: skaffold/v2beta25
kind: Config
metadata:
  name: multistage-build-and-cache
build:
  artifacts:
    - image: multistage-build-and-cache-builder
      context: .
      docker:
        dockerfile: Dockerfile
        target: builder
        cacheFrom:
          - multistage-build-and-cache-builder
        buildArgs:
          BUILDKIT_INLINE_CACHE: "1"
  local:
    useBuildkit: true
  tagPolicy:
    gitCommit:
      ignoreChanges: true

Initial state:
Docker image repo is empty.
SKAFFOLD_DEFAULT_REPO=artemkoru

Case 1

I made a git-commit (let's imagine the hash will be like this aaaaaa) and then ran skaffold build.
Skaffold expands the cacheFrom image multistage-build-and-cache-builder to artemkoru/multistage-build-and-cache-builder:aaaaaa, tries to pull it as cacheFrom (but docker repo is empty), then builds the image without cache and pushes it. In this case everything is fine.

Then, I changed the app code and made an other git-commit (it's hash is bbbbbb) and then ran skaffold build.
Skaffold expands the cacheFrom image multistage-build-and-cache-builder to artemkoru/multistage-build-and-cache-builder:bbbbbb, tries to pull it as cacheFrom (but again, the image doesn't exist, there it only image artemkoru/multistage-build-and-cache-builder:aaaaaa in the docker repo). It is the problem.

Case 2

I know you wrote that you use only latest tags and use a separate docker registry for each environment. In this case I can change tagPolicy as follows:

tagPolicy:
  envTemplate:
    template: "{{.ENV}}"

where ENV is an environment variable containing the name of the current environment.

In this case I will get a static docker tag for each environment. And your solution will work fine. But we have another problem: if I try rollback (e.g. helm rollback) the deployment will be failed, because the image with this tag was overwritten and the sha256 hash won't match. Also I am unable to deploy the previously builded artifacts, the same reason.

To solve this problem, you can propose to implement overrides "tagpolicy" per artifact (not multiple tags per artifact).

Summary

I am going to try implement your solution soon, I would also be grateful if you tell me what you think about the problems that I described above. Thanks!

@briandealwis
Copy link
Member

@artemkoru In Case 1, I think you're confusing a hash (which is computed based on the source contents) with the tag. But since the tag is computed on each build, it's possible that generated tag may differ on the each pass and the cache may not work. For example, if first building from a pristine repo and then making a change, the gitCommit tag for the second iteration will have a -dirty marker. You could use a profile, activated on the dev or debug commands, to set the tagger to a consistent development-only tag.

Using a development-time tagging policy would also avoid the issue in Case 2. But IMHO you shouldn't be relying on image tags, which are mutable: your Helm chart should be referencing the image by its digest. That way a rollback will get exactly the image that was used previously, regardless of the tag.

@Saran33
Copy link

Saran33 commented Oct 21, 2023

@artemkoru In Case 1, I think you're confusing a hash (which is computed based on the source contents) with the tag. But since the tag is computed on each build, it's possible that generated tag may differ on the each pass and the cache may not work. For example, if first building from a pristine repo and then making a change, the gitCommit tag for the second iteration will have a -dirty marker. You could use a profile, activated on the dev or debug commands, to set the tagger to a consistent development-only tag.

Using a development-time tagging policy would also avoid the issue in Case 2. But IMHO you shouldn't be relying on image tags, which are mutable: your Helm chart should be referencing the image by its digest. That way a rollback will get exactly the image that was used previously, regardless of the tag.

If using a CI/CD pipeline to build/push/deploy only on pushes to main, then I think it should be fine to use use commits instead of hashes. It's also a bit easier to reason about in the artifact registy, as opposed to using "latest" tags. In our case, we have a Kubernetes Job run from a manual GitHub worfklow dispatch on main. It uses the most recent commit as the image tag when installing a Helm chart for the job (not installed via Skaffold). This tag references an image already built and pushed in a separate deployment workflow. How could we set this image tag if using a digest? The process would become quite involved. Moreover, I can't think of any good reason why a dirty commit would be used as a tag when deploying to a remote cluster. We only use "latest" tags for a local Skaffold profile.

+1 on being able to use the gitCommit tag policy in conjunction with the change in #6904. For instance, if we could use environment variables in the parameter build.artifacts[*].docker.cacheFrom as suggested by @artemkoru, it would allow us to use git rev-parse --short HEAD~1 to cache from an image tagged with the previous commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/build area/cache area/local build/docker help wanted We would love to have this done, but don't have the bandwidth, need help from contributors kind/feature-request priority/p2 May take a couple of releases
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants