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

Q: Proper bake usage withh groups #117

Closed
Nithos opened this issue Mar 15, 2023 · 5 comments · Fixed by #174
Closed

Q: Proper bake usage withh groups #117

Nithos opened this issue Mar 15, 2023 · 5 comments · Fixed by #174
Labels
kind/enhancement New feature or request

Comments

@Nithos
Copy link
Contributor

Nithos commented Mar 15, 2023

Hi, I am just wondering what is the proper/best practice for using this action with multiple targets through a group.
As an example I have a repository that contains 3 subsystems that are controlled through a bakefile that trigger them.

Example

group "prod" {
  targets = ["A", "B", "C"]
}

I want to trigger this group through the action, but am unsure as to the best approach. I have the following two options but unsure which way is best, or if there is another option I have not considered.

NOTE: removed the surrounding boiler stuff such as credential logins, QEMU/BuildX setup etc.

  1. Simple Way
    Simply use the prod as the targets input into the action. This will trigger the build of all of them however does this consider:
    a) multi-platform builds
    b) proper caching using gha, since the group is used do they share the cache or do they overwrite each others?

Example

- name: Push Images using BuildX Bake
        uses: docker/bake-action@v2
        with:
          files: |
            ./docker-bake.hcl
             ${{ steps.meta.outputs.bake-file }}
          targets: prod
          push: true
          set: |
            *.cache-from=type=gha,scope=build-prod
            *.cache-to=type=gha,scope=build-prod,mode=max
  1. Manual separation and using a Matrix strategy
    Currently I am taking the group input, and using jq to extract the targets within it and generating a matrix strategy so that they are run in parallel and guarantee separate caches. This makes things a bit more complicated and a little harder to maintain (for instance already had an issue with the bake output changing on me and needing to update the jq extraction.
targets:
    name: Generate targets list from provided bake file
    runs-on: ubuntu-22.04
    outputs:
      matrix: ${{ steps.targets.outputs.matrix }}
    steps:
      # 1.1 - checkout the files
      - name: Checkout
        uses: actions/checkout@v3

      # 1.2 - Generate a matrix output of all the targets for the specified group
      - name: Create matrix
        id: targets
        run: |
          docker buildx bake ${{ inputs.group }} -f ${{ inputs.file }} --print
          TARGETS=$(docker buildx bake ${{ inputs.group }} -f ${{ inputs.file }} --print | jq -cr ".group.${{ inputs.group }}.targets")
          echo "matrix=$TARGETS" >> $GITHUB_OUTPUT

      # 1.3 (optional) - output the generated target list for verification
      - name: Show matrix
        run: |
          echo ${{ steps.targets.outputs.matrix }}

Then using that to build the matrix

# this job depends on the 'targets' job
    needs:
      - targets

    # 2.0 - Build a matrix strategy from the retrieved target list
    strategy:
      fail-fast: true
      matrix:
        target: ${{ fromJson(needs.targets.outputs.matrix) }}

And finally building the images of those targets

 - name: Push Images using BuildX Bake
        uses: docker/bake-action@v2
        with:
          files: |
            ./${{ inputs.file }}
             ${{ steps.meta.outputs.bake-file }}
          targets: ${{ matrix.target }}
          push: true
          set: |
            *.cache-from=type=gha,scope=build-${{ matrix.target }}
            *.cache-to=type=gha,scope=build-${{ matrix.target }},mode=max
@crazy-max
Copy link
Member

crazy-max commented Mar 15, 2023

  1. Manual separation and using a Matrix strategy

Looking at your use case this is the best option as it will run each target in a dedicated runner. With 1. it would run each target in // on the same runner which would take more resource and time.

You're also right about the cache. Better to have it scoped to each target.

We use this pattern often across our repos like: https://github.com/moby/buildkit/blob/2816a8328fb13b81bc26b6fd70c67be1ca86e5c6/.github/workflows/validate.yml#L23-L60

We also have a similar pattern but for platforms to build each platform on a dedicated runner to produce binaries: https://github.com/docker/buildx/blob/4a73abfd645457ee8819855cb1a337ab23f833dc/.github/workflows/build.yml#L55-L114

We discussed about this internally few days ago and we were thinking about a simple helper to generate the matrix instead of relying on jq (cc @tonistiigi). I think we could use a composite action inside this action to handle that case.

For grouped targets for example:

group "foo" {
  targets = ["A", "B", "C"]
}
      - name: Create matrix
        id: targets
        use: docker/bake-action/gen-matrix@v2
        with:
          name: foo
          field: targets

For platforms or other fields:

target "binaries-cross" {
  platforms = [
    "linux/amd64",
    "linux/arm/v7",
    "linux/arm64",
    "linux/ppc64le",
    "linux/riscv64",
    "linux/s390x"
  ]
}
      - name: Create matrix
        id: targets
        use: docker/bake-action/gen-matrix@v2
        with:
          name: foo
          field: platforms

Let me know what you think.

@Nithos
Copy link
Contributor Author

Nithos commented Mar 16, 2023

Thank you for the analysis. My thoughts were similar although from my limited testing I have not noticed a big difference in the build times between the two options, however 1 does not seem to hit the cache properly so definitely a no go for me and sticking with 2.

A helper would be much appreciated for sure, would isolate possible issues with output formats changing in the future (hopefully). Is that gen-matrix action currently available or just internal testing for now?

@crazy-max
Copy link
Member

crazy-max commented Mar 16, 2023

Is that gen-matrix action currently available or just internal testing for now?

Not yet, it's just a suggestion/proposal we could implement.

however 1 does not seem to hit the cache properly so definitely a no go for me and sticking with 2.

Yeah that's kinda hard in the same runner, you need to scope each known targets of the group like:

- name: Push Images using BuildX Bake
        uses: docker/bake-action@v2
        with:
          files: |
            ./docker-bake.hcl
             ${{ steps.meta.outputs.bake-file }}
          targets: prod
          push: true
          set: |
            A.cache-from=type=gha,scope=build-prod-A
            A.cache-to=type=gha,scope=build-prod-A,mode=max
            B.cache-from=type=gha,scope=build-prod-B
            B.cache-to=type=gha,scope=build-prod-B,mode=max
            C.cache-from=type=gha,scope=build-prod-C
            C.cache-to=type=gha,scope=build-prod-C,mode=max

I wonder if we could also generate caches inputs if a grouped target is being built.

@Nithos
Copy link
Contributor Author

Nithos commented Mar 16, 2023

Not yet, it's just a suggestion/proposal we could implement.

Great thank you

For the multi-scoped targets, yes I get that but that again requires us to basically make a matrix to know that this needs to be set up. And if we already have a matrix then might as well use the strategy to make it parallel.

One more thing, using the matrix strategy does not prevent some images being pushed and others not due to failure. i.e there is no sync feature to make sure that all targets succeed. Currently the workaround is to do it twice, once for building without push, and the second with push enabled (and hopefully cache is reused). Do you think that would ever change with the use case that a the target is just a group? Would that automatically sync the images?

@crazy-max
Copy link
Member

@Nithos Fyi opened #174

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants