Stop using legacy cache import/export in client - #2982
Conversation
This also fixes the bug where registry cache options with a missing ref would be silently discarded by the server-side component that turns the legacy settings into the new settings. Signed-off-by: Jonny Stoten <jonny.stoten@docker.com>
Signed-off-by: Jonny Stoten <jonny.stoten@docker.com>
Signed-off-by: Jonny Stoten <jonny.stoten@docker.com>
tonistiigi
left a comment
tonistiigi
left a comment
There was a problem hiding this comment.
Could you also update the proto type to add indication that these fields have been removed. Eg. rename from "Deprecated" suffix to "Removed" or add comment.
Not "removed" yet (until vNextNext) ? |
|
@AkihiroSuda These were already deprecated in v0.4 |
|
Yes, deprecated, but not removed and still in use by buildctl as of v0.10.x. |
Isn't it only used when buildctl v0.10 is talking with v0.3 daemon. I don't think we care about that case. |
Doesn't seem so 😥 https://github.com/moby/buildkit/blob/v0.10.3/client/solve.go#L467-L475 |
|
Yes, my thinking is that we remove that stuff from the client first (this PR) and then after the next release we can remove the daemon-side usage and rename to "Removed" suffix. |
Revert "vendor: docker, docker/cli v28.0.0-rc.1"
3 participants
Fixes #2974.
I've kept the server-side code for translating the legacy request for now so that this will continue to work with existing clients. We should be able to delete this once enough clients have been updated, perhaps in the following release?
I've also added some simple validation in the client for the
refattr on registry cache options, and some extra context around errors from theResolveCacheExporterFuncs.