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

buildinfo: named input contexts support #2654

Merged
merged 3 commits into from
Feb 23, 2022

Conversation

crazy-max
Copy link
Member

follow-up #2476

Signed-off-by: CrazyMax crazy-max@users.noreply.github.com

@crazy-max
Copy link
Member Author

crazy-max commented Feb 17, 2022

First example based on https://github.com/docker/buildx/blob/master/docs/reference/buildx_bake.md#pinning-alpine-image

# Dockerfile
FROM alpine
RUN echo "Hello world"
# docker-bake.hcl
target "app" {
    contexts = {
        alpine = "docker-image://alpine:3.13"
    }
}
$ docker buildx bake --metadata-file metadata.json --set *.args.foo=bar --set *.args.bar=foo app
$ cat metadata.json
{
  "containerimage.buildinfo": {
    "frontend": "dockerfile.v0",
    "attrs": {
      "build-arg:bar": "foo",
      "build-arg:foo": "bar",
      "context:alpine": "docker-image://alpine:3.13",
      "filename": "Dockerfile"
    },
    "sources": [
      {
        "type": "docker-image",
        "ref": "docker.io/library/alpine:3.13",
        "pin": "sha256:026f721af4cf2843e07bba648e158fb35ecc876d822130633cc49f707f0fc88c"
      }
    ]
  }
}

@crazy-max
Copy link
Member Author

crazy-max commented Feb 17, 2022

Second example https://github.com/docker/buildx/blob/master/docs/reference/buildx_bake.md#using-a-secondary-source-directory

# Dockerfile

FROM scratch AS src

FROM busybox
COPY --from=src . /src
RUN ls -al /src
# docker-bake.hcl
target "app" {
  contexts = {
    src = "../ctx-src"
  }
}
$ docker buildx bake --metadata-file metadata.json --set *.args.foo=bar --set *.args.bar=foo app
$ cat metadata.json
{
  "containerimage.buildinfo": {
    "frontend": "dockerfile.v0",
    "attrs": {
      "build-arg:bar": "foo",
      "build-arg:foo": "bar",
      "context:src": "local:src",
      "filename": "Dockerfile"
    },
    "sources": [
      {
        "type": "docker-image",
        "ref": "busybox",
        "alias": "docker.io/library/busybox@sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb",
        "pin": "sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb"
      }
    ]
  }
}

@crazy-max
Copy link
Member Author

crazy-max commented Feb 17, 2022

Third example https://github.com/docker/buildx/blob/master/docs/reference/buildx_bake.md#using-a-result-of-one-target-as-a-base-image-in-another-target

baseapp.Dockerfile is missing on buildx docs

# baseapp.Dockerfile
FROM busybox
WORKDIR /src
# Dockerfile
FROM baseapp
RUN echo "Hello world"
# docker-bake.hcl

target "base" {
    dockerfile = "baseapp.Dockerfile"
}

target "app" {
    contexts = {
        baseapp = "target:base"
    }
}
$ docker buildx bake --metadata-file metadata.json --set *.args.foo=bar --set *.args.bar=foo app
$ cat metadata.json
{
  "app": {
    "containerimage.buildinfo": {
      "frontend": "dockerfile.v0",
      "attrs": {
        "build-arg:bar": "foo",
        "build-arg:foo": "bar",
        "context:baseapp": "input:0-base",
        "filename": "Dockerfile"
      }
    }
  },
  "base": {
    "containerimage.buildinfo": {
      "frontend": "dockerfile.v0",
      "attrs": {
        "build-arg:bar": "foo",
        "build-arg:foo": "bar",
        "filename": "baseapp.Dockerfile"
      },
      "sources": [
        {
          "type": "docker-image",
          "ref": "busybox",
          "alias": "docker.io/library/busybox@sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb",
          "pin": "sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb"
        }
      ]
    }
  }
}

I can see this attr is filtered out:

"input-metadata:0-base": "{\"containerimage.config\":\"eyJhcmNoaXRlY3R1cmUiOiJhbWQ2NCIsIm9zIjoibGludXgiLCJyb290ZnMiOnsidHlwZSI6ImxheWVycyIsImRpZmZfaWRzIjpbInNoYTI1NjpkMzE1MDVmZDUwNTBmNmI5NmNhMzI2OGQxZGI1OGZjOTFhZTU2MWRkZjE0ZWFhYmM0MWQ2M2VhMmVmOGMxYzZkIl19LCJoaXN0b3J5IjpbeyJjcmVhdGVkIjoiMjAyMi0wMi0wNFQyMToyMDoxMi4zMTg5MTc4MjJaIiwiY3JlYXRlZF9ieSI6Ii9iaW4vc2ggLWMgIyhub3ApIEFERCBmaWxlOjFjODUwN2UzZTliMjJiOTc3OGYyZWRiYjk1MDA2MWUwNmJkZTZhMWY1M2I2OWUxYzYxMDI1MDAyOWMzNzNiNzIgaW4gLyAifSx7ImNyZWF0ZWQiOiIyMDIyLTAyLTA0VDIxOjIwOjEyLjQ5Nzc5NDgwOVoiLCJjcmVhdGVkX2J5IjoiL2Jpbi9zaCAtYyAjKG5vcCkgIENNRCBbXCJzaFwiXSIsImVtcHR5X2xheWVyIjp0cnVlfSx7ImNyZWF0ZWRfYnkiOiJXT1JLRElSIC9zcmMiLCJjb21tZW50IjoiYnVpbGRraXQuZG9ja2VyZmlsZS52MCJ9XSwiY29uZmlnIjp7IkVudiI6WyJQQVRIPS91c3IvbG9jYWwvc2JpbjovdXNyL2xvY2FsL2JpbjovdXNyL3NiaW46L3Vzci9iaW46L3NiaW46L2JpbiJdLCJDbWQiOlsic2giXSwiV29ya2luZ0RpciI6Ii9zcmMiLCJPbkJ1aWxkIjpudWxsfX0=\"}"

Should we merge input-metadata:0-base (base) with the app buildinfo or change the context:baseapp value to input:base to link them?

@crazy-max

This comment was marked as outdated.

@crazy-max
Copy link
Member Author

crazy-max commented Feb 19, 2022

@tonistiigi Last commit adds deps key as discussed but we can put this one in a follow-up if you prefer. Here is an example:

FROM busybox
RUN echo "Hello World" > /hello

Build this image and push to the registry:

$ docker buildx build --metadata-file metadata.json --platform linux/amd64,linux/arm64 --tag crazymax/busybox:buildinfo --push .

Build info in image config should look like this:

$ docker pull crazymax/busybox:buildinfo
$ docker image save crazymax/busybox:buildinfo | tar -ax --exclude 'manifest.json' --wildcards '*.json' -O | jq -r '."moby.buildkit.buildinfo.v1"' | base64 --decode | jq
{
  "frontend": "dockerfile.v0",
  "sources": [
    {
      "type": "docker-image",
      "ref": "docker.io/library/busybox:latest",
      "pin": "sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb"
    }
  ]
}

Create another Dockerfile that will use this image:

FROM crazymax/busybox:buildinfo
WORKDIR /src

Build info of crazymax/busybox:buildinfo should be added as dep:

$ docker buildx build --metadata-file metadata.json --build-arg foo=bar --build-arg bar=foo .
$ cat metadata.json
{
  "containerimage.buildinfo": {
    "frontend": "dockerfile.v0",
    "attrs": {
      "build-arg:bar": "foo",
      "build-arg:foo": "bar",
      "filename": "baseapp.Dockerfile"
    },
    "sources": [
      {
        "type": "docker-image",
        "ref": "docker.io/crazymax/busybox:buildinfo",
        "pin": "sha256:afa2baea8c14ae33d4430f42883836dab4770208c09d21706219daef3c044aa4"
      }
    ],
    "deps": {
      "crazymax/busybox:buildinfo": {
        "frontend": "dockerfile.v0",
        "sources": [
          {
            "type": "docker-image",
            "ref": "docker.io/library/busybox:latest",
            "pin": "sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb"
          }
        ]
      }
    }
  }
}

Here I use the original image ref as dep key (crazymax/busybox:buildinfo) if that sounds good to you. Maybe it could be the hash of Source block.

@tonistiigi
Copy link
Member

tonistiigi commented Feb 19, 2022

Deps map should not contain buildinfo for the sources, read from the config. The goal is not to gather all build sources recursively(and atm it is just one level of recursiveness anyway). Only for the buildinfo sent with inputs in docker/buildx#958 . The difference is that while for image sources we can get buildinfo from the image itself, for the inputs they don't exist anywhere. So if you rebuild from buildinfo you need to rebuild the deps as well and therefore need buildinfo for the deps.

I'm not sure how this looks like for multi-arch builds as well. I think atm the platform would appear in these keys and I'm not sure if that is correct or not. Feels wrong if you get different buildinfo depending on if you built for a single arch or for the one arch inside of the multi-arch image.

@crazy-max crazy-max changed the title buildinfo: build-context support buildinfo: named input contexts support Feb 19, 2022
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max
Copy link
Member Author

crazy-max commented Feb 19, 2022

@tonistiigi

Deps map should not contain buildinfo for the sources, read from the config. The goal is not to gather all build sources recursively(and atm it is just one level of recursiveness anyway). Only for the buildinfo sent with inputs in docker/buildx#958 . The difference is that while for image sources we can get buildinfo from the image itself, for the inputs they don't exist anywhere. So if you rebuild from buildinfo you need to rebuild the deps as well and therefore need buildinfo for the deps.

Yes was thinking it could be useful but that's out of scope agree and should be handled by others tooling. I removed that behavior.

I'm not sure how this looks like for multi-arch builds as well. I think atm the platform would appear in these keys and I'm not sure if that is correct or not. Feels wrong if you get different buildinfo depending on if you built for a single arch or for the one arch inside of the multi-arch image.

I've added a test for multiplatform and deps https://github.com/moby/buildkit/pull/2654/files#diff-7df700366904727b0358fe2ed234172b34a8d68782c1ef1c255fb8379db79c9bR385. Excpected output for buildinfo:

{
  "frontend": "dockerfile.v0",
  "attrs": {
    "context:base::linux/amd64": "input:base::linux/amd64",
    "context:base::linux/arm64": "input:base::linux/arm64"
  },
  "sources": [
    {
      "type": "docker-image",
      "ref": "docker.io/library/alpine@sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3",
      "pin": "sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3"
    },
    {
      "type": "docker-image",
      "ref": "docker.io/library/busybox:latest",
      "pin": "sha256:b69959407d21e8a062e0416bf13405bb2b71ed7a84dde4158ebafacfa06f5578"
    }
  ],
  "deps": {
    "base:linux/amd64": {
      "frontend": "dockerfile.v0",
      "sources": [
        {
          "type": "docker-image",
          "ref": "alpine",
          "alias": "docker.io/library/alpine@sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3",
          "pin": "sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3"
        }
      ]
    },
    "base:linux/arm64": {
      "frontend": "dockerfile.v0",
      "sources": [
        {
          "type": "docker-image",
          "ref": "alpine",
          "alias": "docker.io/library/alpine@sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3",
          "pin": "sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3"
        }
      ]
    }
  }
}

So yeah it's what you expected. We can discuss about that next week as well as bake keys.

@crazy-max crazy-max marked this pull request as ready for review February 20, 2022 20:47
@crazy-max crazy-max added this to the v0.10.0 milestone Feb 22, 2022
@tonistiigi
Copy link
Member

{
  "app": {
    "containerimage.buildinfo/linux/amd64": {
      "frontend": "dockerfile.v0",
      "attrs": {
        "build-arg:foo": "bar",
        "context:baseapp": "target:base",
        "context:baseapp::linux/amd64": "input:0-base::linux/amd64",
        "context:baseapp::linux/arm64": "input:0-base::linux/arm64",
        "filename": "Dockerfile"
      },
      "sources": [
        {
          "type": "docker-image",
          "ref": "docker.io/library/busybox@sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb",
          "pin": "sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb"
        }
      ],
      "deps": {
        "0-base::linux/amd64": {
          "frontend": "dockerfile.v0",
          "attrs": {
            "filename": "baseapp.Dockerfile"
          },
          "sources": [
            {
              "type": "docker-image",
              "ref": "busybox",
              "alias": "docker.io/library/busybox@sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb",
              "pin": "sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb"
            }
          ]
        },
        "0-base::linux/arm64": {
          "frontend": "dockerfile.v0",
          "attrs": {
            "filename": "baseapp.Dockerfile"
          },
          "sources": [
            {
              "type": "docker-image",
              "ref": "busybox",
              "alias": "docker.io/library/busybox@sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb",
              "pin": "sha256:afcc7f1ac1b49db317a7196c902e61c6c3c4607d63599ee1a82d702d249a0ccb"
            }
          ]
        }
      }
    },

Is what I see from bake atm. Under containerimage.buildinfo/linux/amd64 there should only be amd64 contexts/deps. Also "context:baseapp": "target:base", should either not be there or have the platform-specific value.

Ideally, we can clean up the names like input:0-base as well, but I think that is buildx side change.

Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@crazy-max
Copy link
Member Author

Is what I see from bake atm. Under containerimage.buildinfo/linux/amd64 there should only be amd64 contexts/deps. Also "context:baseapp": "target:base", should either not be there or have the platform-specific value.

Now platforms not tied to the metadata key (eg containerimage.buildinfo/linux/amd64) are filtered out for attrs and deps:

{
  "frontend": "dockerfile.v0",
  "attrs": {
    "context:base": "input:base"
  },
  "sources": [
    {
      "type": "docker-image",
      "ref": "docker.io/library/alpine@sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3",
      "pin": "sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3"
    },
    {
      "type": "docker-image",
      "ref": "docker.io/library/busybox:latest",
      "pin": "sha256:b69959407d21e8a062e0416bf13405bb2b71ed7a84dde4158ebafacfa06f5578"
    }
  ],
  "deps": {
    "base": {
      "frontend": "dockerfile.v0",
      "sources": [
        {
          "type": "docker-image",
          "ref": "alpine",
          "alias": "docker.io/library/alpine@sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3",
          "pin": "sha256:e7d88de73db3d3fd9b2d63aa7f447a10fd0220b7cbf39803c803f2af9ba256b3"
        }
      ]
    }
  }
}

Ideally, we can clean up the names like input:0-base as well, but I think that is buildx side change.

Yes it should be done on buildx.


// if platform is defined, only decode dependencies for that platform
if platform != "" && !strings.HasSuffix(k, "::"+platform) {
continue
Copy link
Member

Choose a reason for hiding this comment

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

As discussed offline, we should do a follow-up that handles the contexts without suffixes as well for multi-platform cases.

@tonistiigi tonistiigi merged commit b0e56cd into moby:master Feb 23, 2022
@crazy-max crazy-max deleted the buildinfo-contexts branch February 23, 2022 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants