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

Tracking issue for health start interval #45897

Open
10 of 12 tasks
cpuguy83 opened this issue Jul 6, 2023 · 15 comments · Fixed by #45965
Open
10 of 12 tasks

Tracking issue for health start interval #45897

cpuguy83 opened this issue Jul 6, 2023 · 15 comments · Fixed by #45965
Labels
area/builder/buildkit Issues affecting buildkit area/cli area/swarm kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny
Milestone

Comments

@cpuguy83
Copy link
Member

cpuguy83 commented Jul 6, 2023

Description

The API was updated in #40894 but other changes are required:

After the 25.0 major release:

  • Update default start interval to be non-zero?
@cpuguy83 cpuguy83 added status/0-triage kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny labels Jul 6, 2023
@cpuguy83 cpuguy83 added this to the 25.0.0 milestone Jul 6, 2023
@cpuguy83 cpuguy83 added kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. area/swarm area/cli area/builder/buildkit Issues affecting buildkit and removed status/0-triage labels Jul 6, 2023
@thaJeztah
Copy link
Member

Ignore my previous comment (removed), it was before coffee

@s4ke
Copy link
Contributor

s4ke commented Jul 16, 2023

This looks cool and useful, just leaving this here though:

I looked through the PRs and didn't see anything about stack file support. Is this something that was overlooked by chance?

@cpuguy83
Copy link
Member Author

@s4ke I added an item to the list

@glours
Copy link

glours commented Jul 18, 2023

FYI @cpuguy83, we won't be able to update the Compose to support the new StartInterval attribute until the 25.0.0 is released as we use the HealthConfig struct to pass all the health check info to the engine.

@zamlz
Copy link

zamlz commented Jul 25, 2023

If I'm understanding the above statements correctly, does this mean the --start-interval flag for the HEALTHCHECK command in Dockerfiles will only be available in v25.0.0?

@cpuguy83
Copy link
Member Author

@zamlz It should be available with the most recent dockerfile release. Set this at the top of your dockerfile:

# syntax=docker.io/docker/dockerfile-upstream:1.6.0

However docker will not honor it until v25.0.0.

@zamlz
Copy link

zamlz commented Jul 26, 2023

@cpuguy83 interesting, so is there a point to using that then if docker won't honor it? I added the line on my end, and the error message is still the same.

Follow up question, why does the latest Dockerfile reference docs say it does support it then? https://docs.docker.com/engine/reference/builder/#healthcheck

@cpuguy83
Copy link
Member Author

the error message is still the same.

What error message?

why does the latest Dockerfile reference docs say it does support it then?

Because the dockerfile parser does support it and it will be embedded in the image config.

so is there a point to using that then if docker won't honor it?

Only that it will be ready when docker v25 is released.

@zamlz
Copy link

zamlz commented Jul 26, 2023

What error message?

@cpuguy83 I'm sorry, I didn't realize I never mentioned it in my first message. Please refer to the message below.

Error response from daemon: dockerfile parse error on line 18: unknown flag: start-interval

@zamlz
Copy link

zamlz commented Jul 26, 2023

But in my system it does appear that the dockerfile parser does not support it.
Could it be because I'm on an older version? My client and engine are both are 24.0.2

@cpuguy83
Copy link
Member Author

@zamlz What does this do for you?

docker build -<< EOF
# syntax=docker/dockerfile:1.6
FROM busybox:latest
HEALTHCHECK --start-interval=10s --interval=5m CMD echo hello
EOF

@zamlz
Copy link

zamlz commented Jul 28, 2023

@cpuguy83 that looks like it works fine.

docker build -<< EOF
# syntax=docker/dockerfile:1.6
FROM busybox:latest
HEALTHCHECK --start-interval=10s --interval=5m CMD echo hello
EOF
[+] Building 8.8s (7/7) FINISHED                                                                                                                                                                                              docker:default
 => [internal] load .dockerignore                                                                                                                                                                                                       0.0s
 => => transferring context: 2B                                                                                                                                                                                                         0.0s
 => [internal] load build definition from Dockerfile                                                                                                                                                                                    0.0s
 => => transferring dockerfile: 150B                                                                                                                                                                                                    0.0s
 => resolve image config for docker.io/docker/dockerfile:1.6                                                                                                                                                                            6.5s
 => docker-image://docker.io/docker/dockerfile:1.6@sha256:ac85f380a63b13dfcefa89046420e1781752bab202122f8f50032edf31be0021                                                                                                              0.8s
 => => resolve docker.io/docker/dockerfile:1.6@sha256:ac85f380a63b13dfcefa89046420e1781752bab202122f8f50032edf31be0021                                                                                                                  0.0s
 => => sha256:9d9c93f4b00be908ab694a4df732570bced3b8a96b7515d70ff93402179ad232 11.80MB / 11.80MB                                                                                                                                        0.5s
 => => sha256:ac85f380a63b13dfcefa89046420e1781752bab202122f8f50032edf31be0021 8.40kB / 8.40kB                                                                                                                                          0.0s
 => => sha256:657fcc512c7369f4cb3d94ea329150f8daf626bc838b1a1e81f1834c73ecc77e 482B / 482B                                                                                                                                              0.0s
 => => sha256:a17ee7fff8f5e97b974f5b48f51647d2cf28d543f2aa6c11aaa0ea431b44bb89 1.27kB / 1.27kB                                                                                                                                          0.0s
 => => extracting sha256:9d9c93f4b00be908ab694a4df732570bced3b8a96b7515d70ff93402179ad232                                                                                                                                               0.3s
 => [internal] load metadata for docker.io/library/busybox:latest                                                                                                                                                                       1.0s
 => [1/1] FROM docker.io/library/busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79                                                                                                                 0.4s
 => => resolve docker.io/library/busybox:latest@sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79                                                                                                                 0.0s
 => => sha256:a416a98b71e224a31ee99cff8e16063554498227d2b696152a9c3e0aa65e5824 1.46kB / 1.46kB                                                                                                                                          0.0s
 => => sha256:3f4d90098f5b5a6f6a76e9d217da85aa39b2081e30fa1f7d287138d6e7bf0ad7 2.22MB / 2.22MB                                                                                                                                          0.2s
 => => sha256:3fbc632167424a6d997e74f52b878d7cc478225cffac6bc977eedfe51c7f4e79 2.29kB / 2.29kB                                                                                                                                          0.0s
 => => sha256:023917ec6a886d0e8e15f28fb543515a5fcd8d938edb091e8147db4efed388ee 528B / 528B                                                                                                                                              0.0s
 => => extracting sha256:3f4d90098f5b5a6f6a76e9d217da85aa39b2081e30fa1f7d287138d6e7bf0ad7                                                                                                                                               0.1s
 => exporting to image                                                                                                                                                                                                                  0.0s
 => => exporting layers                                                                                                                                                                                                                 0.0s
 => => writing image sha256:1cf7b305df857cc4b095b8cd295541ea24ac64e4c0c8dddaafe31b4141f3df66   

But I forgot to mention an important thing about my environment. Because of bugs in the latest buildx, we have to use the legacy environment.(These bugs have been reported and fixed, they just haven't made it to upstream in distro package managers yet).

Once we disable buildx and use the legacy builder, we run into issues.

DOCKER_BUILDKIT=0 docker build -<< EOF
# syntax=docker/dockerfile:1.6
FROM busybox:latest
HEALTHCHECK --start-interval=10s --interval=5m CMD echo hello
EOF
DEPRECATED: The legacy builder is deprecated and will be removed in a future release.
           BuildKit is currently disabled; enable it by removing the DOCKER_BUILDKIT=0
           environment-variable.

Sending build context to Docker daemon  2.048kB
Error response from daemon: dockerfile parse error on line 3: unknown flag: start-interval

@thaJeztah
Copy link
Member

thaJeztah commented Jul 28, 2023

Yes, the classic builder wouldn't support it (at least not for 24.0.x - for 25.0 I guess it still would have to be implemented, but given the deprecation, it's not decided yet).

w.r.t. the syntax;

# syntax=docker/dockerfile:1.6

Generally, I'd recommend using docker/dockerfile:1 (not 1.6) as that will always provide you the latest stable (1.x) release of the dockerfile syntax. Specifying a minor version (such as 1.6) means you're pinning to that version, and won't get updates (1.7, 1.8 once they arrive);

# syntax=docker/dockerfile:1

FROM yourimage:latest
# etc..

@zamlz
Copy link

zamlz commented Jul 28, 2023

I see, thanks for the clarification @thaJeztah. Just curious, is there an expected version in which the legacy builder will be fully deprecated?

Another quick question, is it recommended to add # syntax=docker/dockerfile:1 to every Dockerfile's header? My understanding of the default behaviour of Dockerfiles are that it will use the latest stable version anyway? Please let me know if I'm wrong here.

@thaJeztah
Copy link
Member

is there an expected version in which the legacy builder will be fully deprecated?

No, we haven't set a date for that. We still need to support the classic builder for Windows, as there's no support for native Windows containers/images yet in BuildKit. But consider it mostly on "life support"; there's no active development happening on the classic builder, so it'll mostly be (critical) bugfixes. That's not to say we fully froze it, so if it's possible to improve (or desirable for compatibility reasons), we may apply some patches, but if we do so, we'll look closely if it doesn't bring considerable maintenance-cost in doing so.

Another quick question, is it recommended to add # syntax=docker/dockerfile:1 to every Dockerfile's header?

I think the docs could be somewhat clearer on this, but yes, I would recommend starting your Dockerfiles with a syntax directive (https://docs.docker.com/build/dockerfile/frontend/).

My understanding of the default behaviour of Dockerfiles are that it will use the latest stable version anyway? Please let me know if I'm wrong here.

The default is to use the Dockerfile parser that's included with the BuildKit version that's included (compiled in) in the Daemon (e.g. for v24.0.5, the version of BuildKit included is; https://github.com/moby/moby/blob/v24.0.5/vendor.mod#L59). However, that version is not trivial to match with an exact version of the Dockerfile syntax (both are released from the BuildKit code repository, but follow their own release cycle).
The default (compiled-in) Dockerfile parser allows for docker build to work in offline (network-isolated) environments, but the Dockerfile parser version is not updated automatically, so it may not be "latest".

When # syntax=docker/dockerfile:1 directive instructs BuildKit to download the latest version of the Dockerfile (dockerfile:1) parser from Docker Hub before continuing the build. This of course needs a networking connection, but also allows for the syntax to be updated (including bugfixes) without updating the Engine (daemon) or BuildKit itself. That means you can have the latest syntax, even if the Daemon you're using is not managed by you (e.g. a hosted builder).

For example:

docker build -<<'EOF'
# syntax=docker/dockerfile:1
FROM scratch
EOF

In the output, you can see resolve image config for docker.io/docker/dockerfile:1 (check fo the latest digest for the dockerfile:1 syntax), and CACHED docker-image://docker.io/docker/dockerfile:1@sha256:39b85bbfa7536a5feceb7372a0817649ecb2724562a38360f4d6a7782a409b14 (it resolved to sha256:39b85bbfa7536a5feceb7372a0817649ecb2724562a38360f4d6a7782a409b14 and that version was already downloaded, so doesn't have to be downloaded from Docker Hub)

[+] Building 0.8s (5/5) FINISHED                                                                                    docker:desktop-linux
 => [internal] load build definition from Dockerfile                                                                                0.0s
 => => transferring dockerfile: 79B                                                                                                 0.0s
 => [internal] load .dockerignore                                                                                                   0.0s
 => => transferring context: 2B                                                                                                     0.0s
 => resolve image config for docker.io/docker/dockerfile:1                                                                          0.7s
 => CACHED docker-image://docker.io/docker/dockerfile:1@sha256:39b85bbfa7536a5feceb7372a0817649ecb2724562a38360f4d6a7782a409b14     0.0s
 => => resolve docker.io/docker/dockerfile:1@sha256:39b85bbfa7536a5feceb7372a0817649ecb2724562a38360f4d6a7782a409b14                0.0s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/builder/buildkit Issues affecting buildkit area/cli area/swarm kind/enhancement Enhancements are not bugs or new features but can improve usability or performance. kind/feature Functionality or other elements that the project doesn't currently have. Features are new and shiny
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants