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

ContainerConfig field missing with buildkit #2868

Closed
tjamet opened this issue Dec 7, 2020 · 4 comments
Closed

ContainerConfig field missing with buildkit #2868

tjamet opened this issue Dec 7, 2020 · 4 comments

Comments

@tjamet
Copy link

tjamet commented Dec 7, 2020

First of all, I am unsure the issue needs to be opened here, in moby or in buildkit. Feel free to redirect me wherever is needed.

Description

Steps to reproduce the issue:

  1. printf 'FROM scratch\nLABEL some=defined\n' | env DOCKER_BUILDKIT=1 docker build -t buildkit -
  2. printf 'FROM scratch\nLABEL some=defined\n' | env DOCKER_BUILDKIT=0 docker build -t legacy -
  3. docker inspect legacy | jq '.[0].ContainerConfig'
  4. docker inspect buildkit | jq '.[0].ContainerConfig'

Describe the results you received:

In the case of the legacy build, the ContainerConfig field is filled with the labels

{
  "Hostname": "9e7bccb52cc4",
  "Domainname": "",
  "User": "",
  "AttachStdin": false,
  "AttachStdout": false,
  "AttachStderr": false,
  "Tty": false,
  "OpenStdin": false,
  "StdinOnce": false,
  "Env": [
    "PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
  ],
  "Cmd": [
    "/bin/sh",
    "-c",
    "#(nop) ",
    "LABEL some=defined"
  ],
  "Image": "",
  "Volumes": null,
  "WorkingDir": "",
  "Entrypoint": null,
  "OnBuild": null,
  "Labels": {
    "some": "defined"
  }
}

In the case of buildkit, no label is provided

{
  "Hostname": "",
  "Domainname": "",
  "User": "",
  "AttachStdin": false,
  "AttachStdout": false,
  "AttachStderr": false,
  "Tty": false,
  "OpenStdin": false,
  "StdinOnce": false,
  "Env": null,
  "Cmd": null,
  "Image": "",
  "Volumes": null,
  "WorkingDir": "",
  "Entrypoint": null,
  "OnBuild": null,
  "Labels": null
}

Describe the results you expected:

I would have expected backward compatibility between both builds, or a deprecation notice in the API but didn't find anything.

This is triggered by a bug on a project I help maintain whalebrew/whalebrew#119 that, probably wrongly, uses the ContainerConfig field to inspect the image.

Output of docker version:

Client: Docker Engine - Community
 Cloud integration: 1.0.2
 Version:           19.03.13
 API version:       1.40
 Go version:        go1.13.15
 Git commit:        4484c46d9d
 Built:             Wed Sep 16 16:58:31 2020
 OS/Arch:           darwin/amd64
 Experimental:      false

Server: Docker Engine - Community
 Engine:
  Version:          19.03.13
  API version:      1.40 (minimum version 1.12)
  Go version:       go1.13.15
  Git commit:       4484c46d9d
  Built:            Wed Sep 16 17:07:04 2020
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          v1.3.7
  GitCommit:        8fba4e9a7d01810a393d5d25a3621dc101981175
 runc:
  Version:          1.0.0-rc10
  GitCommit:        dc9208a3303feef5b3839f4323d9beb36df0a9dd
 docker-init:
  Version:          0.18.0
  GitCommit:        fec3683

Output of docker info:

Client:
 Debug Mode: false
 Plugins:
  scan: Docker Scan (Docker Inc., v0.3.4)

Server:
 Containers: 0
  Running: 0
  Paused: 0
  Stopped: 0
 Images: 18
 Server Version: 19.03.13
 Storage Driver: overlay2
  Backing Filesystem: extfs
  Supports d_type: true
  Native Overlay Diff: true
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: runc
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 8fba4e9a7d01810a393d5d25a3621dc101981175
 runc version: dc9208a3303feef5b3839f4323d9beb36df0a9dd
 init version: fec3683
 Security Options:
  seccomp
   Profile: default
 Kernel Version: 5.4.39-linuxkit
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 6
 Total Memory: 1.941GiB
 Name: docker-desktop
 ID: Z3TQ:34B3:WYRO:Q23J:KEAU:3PSG:TSIF:CCTO:STDT:WIK5:G7Q6:RXYZ
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: gateway.docker.internal:3128
 HTTPS Proxy: gateway.docker.internal:3129
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  127.0.0.0/8
 Live Restore Enabled: false
 Product License: Community Engine

Additional environment details (AWS, VirtualBox, physical, etc.):

docker for mac

tjamet added a commit to whalebrew/whalebrew that referenced this issue Dec 8, 2020
buildkit built images are not providing the ContainerConfig we are
reading labels from. Instead, everything is populated in the Config field
of `inspect`.

We should eventually have better visibility about those fields in this
issue: docker/cli#2868

Meanwhile, we should support it the best way possible.

Benefit from having to do this change to mutualise the label parsing
logic so we are consistent in how we manage the label lookup and
encoding (prefer yaml and fallback to string when possible)

Fixes: #119
tjamet added a commit to whalebrew/whalebrew that referenced this issue Dec 8, 2020
buildkit built images are not providing the ContainerConfig we are
reading labels from. Instead, everything is populated in the Config field
of `inspect`.

We should eventually have better visibility about those fields in this
issue: docker/cli#2868

Meanwhile, we should support it the best way possible.

Benefit from having to do this change to mutualise the label parsing
logic so we are consistent in how we manage the label lookup and
encoding (prefer yaml and fallback to string when possible)

Fixes: #119
@thaJeztah
Copy link
Member

I think this is expected, because of how BuildKit is implemented. In the "non-buildkit" situation, a docker build was effectively a chain of docker run -> docker commit steps (the Dockerfile originated from originally being a shell script to automate steps).

As a side-effect, the classic builder includes the information of the container that was run for a step (and "committed" to an image); but the container config (besides cache-matching for the classic builder) is not needed for the image itself (the Config section is what's actually the image's configuration.

BuildKit takes a fully different approach, and for many steps won't be using a container (or at least, not a container per step), and because of that does not include this information.

I would have expected backward compatibility between both builds, or a deprecation notice in the API but didn't find anything.

Hm, yes, I can see it being useful to improve the description for the image inspect API endpoint. There's likely a couple of reasons for that (not an excuse; bear with me); one is that some types/struct are used for multiple purposes (but with optional fields, depending on where they're used). The other is that inspect endpoints were historically created mostly for debugging (check/inspect the low-level information that was stored on disk), but their fields could be an "implementation detail" (the docker daemon itself being the only consumer).

For images, the format was later formalised in the OCI image spec (https://github.com/opencontainers/image-spec/blob/v1.0.1/config.md)

While the images stored in the local image cache are OCI compliant, the specification allows for additional implementation-specific fields to be present (for reasons like the one described here);

Any extra fields in the Image JSON struct are considered implementation specific and MUST be ignored by any implementations which are unable to interpret them.

So, all of those are not an "excuse", and I agree that if possible, we should update the API description to clarify that this information is "optional" (and may not be present), and it's purpose when present (which is for the "classic" builder to perform cache-checking).

@tjamet With all of the above; is there specific information you need that is not present in the "formal" fields (defined in the OCI spec)?

@tjamet
Copy link
Author

tjamet commented Dec 14, 2020

Great, thanks for the cristal clear answer.

So var Whalebrew relies exclusively on labels, volumes, and entrypoints, i.e. the basics.
Although I didn't do a deep dig into it, I tend to think the rest of the needs are already covered

@xpader
Copy link

xpader commented Mar 25, 2024

Any plan to compatible buildkit with docker-compose-plugin ?

@thaJeztah
Copy link
Member

Closing this ticket, as the ContainerConfig field has been deprecated and removed for current API versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants