docker / setup-buildx-action

GitHub Action to set up Docker Buildx
https://github.com/marketplace/actions/docker-setup-buildx
Apache License 2.0
952 stars 149 forks source link

default context issue with self hosted runner #311

Closed lifeofmoo closed 2 months ago

lifeofmoo commented 6 months ago

Contributing guidelines

I've found a bug, and:

Description

Explicitly setting the context to either:

       - name: Build and push
         uses: docker/build-push-action@v5
         with:
           context: "{{defaultContext}}"

or 
       - name: Build and push
         uses: docker/build-push-action@v5
         with:
           context: .

results in this error.

Creating a new builder instance

  /usr/local/bin/docker buildx create --name builder-f2d40647-7bfd-4310-a7a4-7f3dd40aff91 --driver docker-container --buildkitd-flags --allow-insecure-entitlement security.insecure --allow-insecure-entitlement network.host --use
  ERROR: could not create a builder instance with TLS data loaded from environment. Please use `docker context create <context-name>` to create a context for current environment and then create a builder instance with context set to <context-name>

Expected behaviour

I expect the default context to be used as it's mentioned a few times in the ENV log output.

image

Actual behaviour

I can only get my workflow to run and successfully push to an AWS ECR, if I remove the above and use this instead:

      - name: Set up Docker Context for Buildx
        id: buildx-context
        run: |
          docker context create builders

      - name: Set up Docker Buildx
        uses: docker/setup-buildx-action@v3
        with:
          version: latest
          endpoint: builders

      # - name: Build and push
      #   uses: docker/build-push-action@v5
      #   with:
      #     context: "{{defaultContext}}"

I got the inspiration from this similar issue.

Is anyone able to explain why this approach works please? It's more for my understanding and ability to debug issues in future.

Regards,

Benny

Repository URL

No response

Workflow run URL

No response

YAML workflow

N/A

Workflow logs

No response

BuildKit logs

No response

Additional info

No response

crazy-max commented 3 months ago

same as https://github.com/docker/setup-buildx-action/issues/105#issuecomment-910774354

crazy-max commented 3 months ago

I'm looking at making context creation automatic with this action if TLS data are detected.

crazy-max commented 2 months ago

@lifeofmoo Can you test my changes from #341 please and let me know if it works for you?:

      -
        name: Set up Docker Buildx
        uses: crazy-max/docker-setup-buildx-action@docker-context-tls
davidfrickert commented 2 months ago

hey @crazy-max , i have the same issue, your PR did not work for me, but very close!

diff here: https://github.com/crazy-max/docker-setup-buildx-action/compare/docker-context-tls...davidfrickert:docker-setup-buildx-action:docker-context-tls of a working action (at least for my setup)

and also log of my context info:

context info: {
  "Name": "default",
  "Metadata": {},
  "Endpoints": {
    "docker": {
      "Host": "tcp://localhost:2376",
      "SkipTLSVerify": false
    }
  },
  "TLSMaterial": {
    "docker": [
      "ca.pem",
      "cert.pem",
      "key.pem"
    ]
  },
  "Storage": {
    "MetadataPath": "<IN MEMORY>",
    "TLSPath": "<IN MEMORY>"
  }
}

I do not have TLSData in the endpoints so your PR's action doesn't create the context, so in my commit i replaced that with checking contextInfo.Storage.TLSPath == "<IN MEMORY>". Not sure if this is the way to go but it works for me in Gitea actions!

(ofc ignore the changes from debug to info i just changed that because i couldn't get the debug to work)

Edit: probably it's TLSMaterial.docker that needs to be looked at

crazy-max commented 2 months ago

diff here: crazy-max/docker-setup-buildx-action@docker-context-tls...davidfrickert:docker-setup-buildx-action:docker-context-tls of a working action (at least for my setup)

The right condition in Buildx is on default context with TLS data loaded: https://github.com/docker/buildx/blob/d130f8ef0aea99ed9dc53c808e0bc77a8fc622c0/builder/builder.go#L489

Not Storage.TLSPath that is always set to <IN MEMORY> for default context even if TLS is not used: https://github.com/docker/cli/blob/393de5f44f250e83b473ad7aeb9bd6d94f21e8c9/cli/command/defaultcontextstore.go#L199

davidfrickert commented 2 months ago

diff here: crazy-max/docker-setup-buildx-action@docker-context-tls...davidfrickert:docker-setup-buildx-action:docker-context-tls of a working action (at least for my setup)

The right condition in Buildx is on default context with TLS data loaded: https://github.com/docker/buildx/blob/d130f8ef0aea99ed9dc53c808e0bc77a8fc622c0/builder/builder.go#L489

Not Storage.TLSPath that is always set to <IN MEMORY> for default context even if TLS is not used: https://github.com/docker/cli/blob/393de5f44f250e83b473ad7aeb9bd6d94f21e8c9/cli/command/defaultcontextstore.go#L199

i see.. but in my case i get the exact error in the code line you linked but i do not have TLSData on the context. maybe this does not show up in docker context inspect but is instead fetched in some other way?

crazy-max commented 2 months ago

i see.. but in my case i get the exact error in the code line you linked but i do not have TLSData on the context. maybe this does not show up in docker context inspect but is instead fetched in some other way?

Ok after looking a bit more at it, we need to check for TLSMaterial as well. Here is a repro with a compose file:

services:
  docker:
    image: docker:dind
    privileged: true
    volumes:
      - certs:/certs

  cli:
    image: docker:cli
    depends_on:
      - docker
    volumes:
      - certs:/certs

volumes:
  certs:
$ docker compose run --rm cli buildx create --use
ERROR: could not create a builder instance with TLS data loaded from environment. Please use `docker context create <context-name>` to create a context for current environment and then create a builder instance with context set to <context-name>
crazy-max commented 2 months ago

@davidfrickert Should work with latest changes from https://github.com/docker/setup-buildx-action/pull/341 if you want to give it a try, thanks!

      -
        name: Set up Docker Buildx
        uses: crazy-max/docker-setup-buildx-action@docker-context-tls
davidfrickert commented 2 months ago

@davidfrickert Should work with latest changes from https://github.com/docker/setup-buildx-action/pull/341 if you want to give it a try, thanks!

Amazing will give it a try later today!

Probably out of scope for this issue but would it be possible to cache builders to make use of the cache?

crazy-max commented 2 months ago

Probably out of scope for this issue but would it be possible to cache builders to make use of the cache?

I guess you mean the local state where build cache is saved, then no for now, see https://github.com/docker/setup-buildx-action/pull/138

Amazing will give it a try later today!

Thanks!

davidfrickert commented 2 months ago

Probably out of scope for this issue but would it be possible to cache builders to make use of the cache?

I guess you mean the local state where build cache is saved, then no for now, see #138

Actually meant creating a named context and builder (if not exists of course), and not deleting it at the end, so that it can be reused in later executions and it can use the cache. It was what I was doing with a shell script before moving to use this (patched) action.

davidfrickert commented 2 months ago

@davidfrickert Should work with latest changes from #341 if you want to give it a try, thanks!

      -
        name: Set up Docker Buildx
        uses: crazy-max/docker-setup-buildx-action@docker-context-tls

Tested in my Gitea Actions setup, works nicely!

Log snippet:

::endgroup::
::group::Buildx version
[command]/usr/local/bin/docker buildx version
github.com/docker/buildx v0.13.1 788433953af10f2a698f5c07611dddce2e08c7a0
::endgroup::
::group::Creating temp docker context (TLS data loaded in default one)
[command]/usr/local/bin/docker context create buildx-a313545e-849a-482a-9527-ec29f5769afb
buildx-a313545e-849a-482a-9527-ec29f5769afb
Successfully created context "buildx-a313545e-849a-482a-9527-ec29f5769afb"
Setting builder endpoint to buildx-a313545e-849a-482a-9527-ec29f5769afb context
::endgroup::
::group::Creating a new builder instance
[command]/usr/local/bin/docker buildx create --name builder-18d60bde-79ed-4053-b215-83f54f3898d8 --driver docker-container --buildkitd-flags --allow-insecure-entitlement security.insecure --allow-insecure-entitlement network.host --platform linux/amd64 --use buildx-a313545e-849a-482a-9527-ec29f5769afb
builder-18d60bde-79ed-4053-b215-83f54f3898d8
::endgroup::
::group::Booting builder
[command]/usr/local/bin/docker buildx inspect --bootstrap --builder builder-18d60bde-79ed-4053-b215-83f54f3898d8
crazy-max commented 2 months ago

Probably out of scope for this issue but would it be possible to cache builders to make use of the cache?

I guess you mean the local state where build cache is saved, then no for now, see #138

Actually meant creating a named context and builder (if not exists of course), and not deleting it at the end, so that it can be reused in later executions and it can use the cache. It was what I was doing with a shell script before moving to use this (patched) action.

In this case I suggest to create the builder manually on your self-hosted runner so any build running will use it:

# create docker context without tls data
$ docker context create buildxctx
# create the builder using created docker context and use it by default
$ docker buildx create --name mybuilder --use --bootstrap buildxctx

Tested in my Gitea Actions setup, works nicely!

Awesome! Do you have a public link to your pipeline btw?

davidfrickert commented 2 months ago

Probably out of scope for this issue but would it be possible to cache builders to make use of the cache?

I guess you mean the local state where build cache is saved, then no for now, see #138

Actually meant creating a named context and builder (if not exists of course), and not deleting it at the end, so that it can be reused in later executions and it can use the cache. It was what I was doing with a shell script before moving to use this (patched) action.

In this case I suggest to create the builder manually on your self-hosted runner so any build running will use it:

# create docker context without tls data
$ docker context create buildxctx
# create the builder using created docker context and use it by default
$ docker buildx create --name mybuilder --use --bootstrap buildxctx

Tested in my Gitea Actions setup, works nicely!

Awesome! Do you have a public link to your pipeline btw?

sorry, no, it is a private repo. but i can share the pipeline YAML here:

name: Build and Push docker images

on:
  push:
    branches:
      - 'master'
  pull_request:

# heavily inspired from https://docs.docker.com/build/ci/github-actions/multi-platform/#distribute-build-across-multiple-runners

jobs:
  build:
    strategy:
      matrix:
        arch: [amd64, arm64]
    runs-on: ${{ matrix.arch }}
    steps:
      - name: Checkout
        uses: actions/checkout@v4

      - name: Login to Docker Hub
        uses: docker/login-action@v3
        with:
          username: ${{ vars.DOCKERHUB_USERNAME }}
          password: ${{ secrets.DOCKERHUB_TOKEN }}

      - name: Set up buildx
        id: buildx
        uses: crazy-max/docker-setup-buildx-action@docker-context-tls
        with:
          platforms: linux/${{ matrix.arch }}
          cleanup: true

      - name: Get docker image before build
        id: pre-build
        run: |
          make echo-image | sed 's/^/image=/'     >> "$GITHUB_OUTPUT"          

      - name: Build and push by digest
        id: build
        uses: docker/build-push-action@v6
        with:
          builder: ${{ steps.buildx.outputs.name }}
          tags: ${{ steps.pre-build.outputs.image }}
          outputs: type=image,name=${{ steps.pre-build.outputs.image }},push-by-digest=true,name-canonical=true,push=true

      - name: Export digest
        run: |
          mkdir -p /tmp/digests-${{ github.run_id }}-${{ github.run_attempt }}
          digest="${{ steps.build.outputs.digest }}"
          touch "/tmp/digests-${{ github.run_id }}-${{ github.run_attempt }}/${digest#sha256:}"                    

      - name: Upload digest
        uses: christopherhx/gitea-upload-artifact@v4
        with:
          name: digests-linux-${{ matrix.arch }}
          path: /tmp/digests-${{ github.run_id }}-${{ github.run_attempt }}/*
          if-no-files-found: error
          retention-days: 1

  merge:
    runs-on: host
    needs:
      - build
    steps:
      - name: Checkout
        uses: actions/checkout@v4

      - name: Download digests
        uses: christopherhx/gitea-download-artifact@v4
        with:
          path: /tmp/digests-merge-${{ github.run_id }}-${{ github.run_attempt }}
          pattern: digests-*
          merge-multiple: true

      - name: Set up buildx
        id: buildx
        uses: crazy-max/docker-setup-buildx-action@docker-context-tls
        with:
          cleanup: true

      - name: Get docker image before build
        id: pre-build
        run: |
          make echo-image | sed 's/^/image=/' >> "$GITHUB_OUTPUT"          

      - name: Docker meta
        id: meta
        uses: docker/metadata-action@v5
        with:
          images: ${{ steps.pre-build.outputs.image }}

      - name: Login to Docker Hub
        uses: docker/login-action@v3
        with:
          username: ${{ vars.DOCKERHUB_USERNAME }}
          password: ${{ secrets.DOCKERHUB_TOKEN }}

      - name: Create manifest list and push
        working-directory: /tmp/digests-merge-${{ github.run_id }}-${{ github.run_attempt }}
        run: |
          docker buildx imagetools create $(jq -cr '.tags | map("-t " + .) | join(" ")' <<< "$DOCKER_METADATA_OUTPUT_JSON") \
            $(printf '${{ steps.pre-build.outputs.image }}@sha256:%s ' *)                    

      - name: Inspect image
        run: |
          docker buildx imagetools inspect ${{ steps.pre-build.outputs.image }}:${{ steps.meta.outputs.version }}