Skip to content

nodes metadata JSON ouput#162

Merged
crazy-max merged 4 commits intodocker:masterfrom
crazy-max:nodes-output
Sep 22, 2022
Merged

nodes metadata JSON ouput#162
crazy-max merged 4 commits intodocker:masterfrom
crazy-max:nodes-output

Conversation

@crazy-max
Copy link
Member

@crazy-max crazy-max commented Sep 18, 2022 •
edited
Loading

At the moment endpoint, status, flags and platforms outputs are only return for the very first note being registered for the builder.

This PR adds a new output nodes that returns metadata JSON of all registered nodes and also adds new fields available since Buildx 0.9:

[
  {
    "name": "builder-06133552-7a7d-4d4f-a011-fccc08c3ae500",
    "endpoint": "unix:///var/run/docker.sock",
    "driver-opts": [
      "image=moby/buildkit:master",
      "network=host"
    ],
    "status": "running",
    "buildkitd-flags": "--allow-insecure-entitlement security.insecure --allow-insecure-entitlement network.host",
    "buildkit": "81b6ff2",
    "platforms": "linux/amd64,linux/amd64/v2,linux/amd64/v3,linux/386"
  }
]

This change is primarily for future support of appending nodes to a builder, so the output is relevant.

endpoint, status, flags and platforms outputs are deprecated and will be removed in next major release.

@crazy-max crazy-max force-pushed the nodes-output branch 3 times, most recently from 26f290a to b0194e0 Compare September 18, 2022 00:29
@crazy-max crazy-max mentioned this pull request Sep 18, 2022
Merged
@crazy-max crazy-max marked this pull request as ready for review September 19, 2022 08:48
tonistiigi
tonistiigi reviewed Sep 19, 2022
View reviewed changes
Copy link
Member

@tonistiigi tonistiigi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could maybe see status and platforms being a little bit useful in some cases. If the goal is to handle the cases with multiple nodes(not very common in CI) then maybe we could just combine the values for these fields?

jedevc
jedevc reviewed Sep 20, 2022
View reviewed changes
Copy link
Contributor

@jedevc jedevc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could maybe see status and platforms being a little bit useful in some cases. If the goal is to handle the cases with multiple nodes(not very common in CI) then maybe we could just combine the values for these fields?

Agreed, especially makes sense for platforms, I like the idea of merging each node's together for this if we can.

@crazy-max
Copy link
Member Author

crazy-max commented Sep 20, 2022 •
edited
Loading

I could maybe see status and platforms being a little bit useful in some cases. If the goal is to handle the cases with multiple nodes(not very common in CI) then maybe we could just combine the values for these fields?

Agreed, especially makes sense for platforms, I like the idea of merging each node's together for this if we can.

This would not give the expected result as we can define preferred platforms even if those are not supported on specified nodes:

  {
    "nodes": [
      {
        "name": "builder-13dbff8e-3be6-4024-9aa5-2c75782f1cf10",
        "endpoint": "docker-container://buildx_buildkit_builder-ba6a88cb-fe10-4412-876e-66458fe474180",
        "status": "running",
        "platforms": "linux/amd64,linux/amd64/v2,linux/amd64/v3,linux/amd64/v4,linux/arm64,linux/riscv64,linux/ppc64le,linux/s390x,linux/386,linux/mips64le,linux/mips64,linux/arm/v7,linux/arm/v6"
      },
      {
        "name": "aws_graviton2",
        "endpoint": "tcp://***:1234",
        "driver-opts": [
          "cacert=/home/runner/.docker/buildx/creds/builder-13dbff8e-3be6-4024-9aa5-2c75782f1cf1/cacert_***-1234.pem",
          "cert=/home/runner/.docker/buildx/creds/builder-13dbff8e-3be6-4024-9aa5-2c75782f1cf1/cert_***-1234.pem",
          "key=/home/runner/.docker/buildx/creds/builder-13dbff8e-3be6-4024-9aa5-2c75782f1cf1/key_***-1234.pem"
        ],
        "status": "running",
        "platforms": "darwin/arm64*,linux/arm64*,linux/arm/v5*,linux/arm/v6*,linux/arm/v7*,windows/arm64*"
      },
      {
        "name": "linuxone_s390x",
        "endpoint": "tcp://***:1234",
        "driver-opts": [
          "cacert=/home/runner/.docker/buildx/creds/builder-13dbff8e-3be6-4024-9aa5-2c75782f1cf1/cacert_***-1234.pem",
          "cert=/home/runner/.docker/buildx/creds/builder-13dbff8e-3be6-4024-9aa5-2c75782f1cf1/cert_***-1234.pem",
          "key=/home/runner/.docker/buildx/creds/builder-13dbff8e-3be6-4024-9aa5-2c75782f1cf1/key_***-1234.pem"
        ],
        "status": "running",
        "platforms": "linux/s390x*"
      }
    ],
    "name": "builder-13dbff8e-3be6-4024-9aa5-2c75782f1cf1",
    "driver": "remote"
  }

For example here aws_graviton2 supported platforms are:

time="2022-09-19T21:12:20Z" level=info msg="found worker \"klrqif3uuxy84t69eo3eef89n\",
  labels=map[org.mobyproject.buildkit.worker.executor:oci
    org.mobyproject.buildkit.worker.hostname:7c099955516d
    org.mobyproject.buildkit.worker.network:host
    org.mobyproject.buildkit.worker.oci.process-mode:sandbox
    org.mobyproject.buildkit.worker.snapshotter:overlayfs
  ],
  platforms=[linux/arm64 linux/arm/v7 linux/arm/v6]"

But I define the following ones as preferred:

      -
        name: Set up Docker Buildx
        uses: crazy-max/docker-setup-buildx-action@append
        with:
          driver: remote
          endpoint: docker-container://buildx_buildkit_${{ steps.builder.outputs.name }}0
          append: |
            - name: aws_graviton2
              endpoint: tcp://${{ secrets.AWS_ARM64_HOST }}:1234
              platforms: darwin/arm64,linux/arm64,linux/arm/v5,linux/arm/v6,linux/arm/v7,windows/arm64
            - name: linuxone_s390x
              endpoint: tcp://${{ secrets.LINUXONE_S390X_HOST }}:1234
              platforms: linux/s390x

See #165 and https://github.com/crazy-max/docker-setup-buildx-action/blob/append/docs/advanced/append-nodes.md about the append input

@crazy-max crazy-max mentioned this pull request Sep 20, 2022
Merged
@crazy-max
Copy link
Member Author

crazy-max commented Sep 21, 2022

As discussed keep platforms output but return preferred ones if set or all available.

@crazy-max crazy-max force-pushed the nodes-output branch 4 times, most recently from 78c8b9a to 8af1ec8 Compare September 21, 2022 14:27
@crazy-max
nodes metadata JSON ouput
aa0df6f 
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max
return driver opts and buildkit version from nodes metadata
69d3837 
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max
add tests for inspect parsing
c6ec880 
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max
update generated content
ec8fe6a 
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max crazy-max merged commit d5234d6 into docker:master Sep 22, 2022
@crazy-max crazy-max deleted the nodes-output branch September 22, 2022 09:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tonistiigi tonistiigi Awaiting requested review from tonistiigi

1 more reviewer

@jedevc jedevc jedevc approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@crazy-max @tonistiigi @jedevc