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

Acb/update buildkit #31

Merged
merged 107 commits into from
Nov 6, 2023
Merged

Acb/update buildkit #31

merged 107 commits into from
Nov 6, 2023

Conversation

alexcb
Copy link
Contributor

@alexcb alexcb commented Oct 31, 2023

No description provided.

tonistiigi and others added 30 commits September 27, 2023 17:35
"Merged edges" is an optimization in the solve graph where two different active LLB edges can be combined into one after cache key computation finds that they generated equivalent cache chain.

In that case, one edge is released and is set to point to another. The reference count for the second one is increased.

An issue was discovered where an edge that was already pointing to another edge became a target to the third one. The current implementation did not handle the case where an edge that already had a different target itself became a target edge as well. This resulted in an inconsistent graph state where edges that were thought to be released could get scheduled again.

Instead of setting the same edge value to two different maps, the new logic is to chain the edges into a linked list that should handle multiple levels of targets. This slightly increases the number of lookups, but "merged edges" case is very rare anyway, and a couple of extra pointer dereferences do not affect build speed.

Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
The output window was previously hard-coded to a height of 6;
this patch makes it configurable at run-time by setting the
BUILDKIT_TTY_LOG_LINES environment variable.

Signed-off-by: Burt Holzman <burt@fnal.gov>
Mirrors in `RegistryConfig.Mirrors` can be specified using a full URL
(schema, trailing slashes) but registries in the input map are keyed by
their hostname.

Previous code used the mirror URL as key which resulted in an empty
`RegistryConfig` being passed to the `fillInsecureOpts` function and
didn't set the insecure options.

Use Host part of the parsed registry as a key instead.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
If both Insecure and PlainHTTP is requested for the host, use a
transport that falls back to HTTP in case of an HTTP request to a HTTPS
client error.

This also changes the order - before that an HTTP connection was
attempted first. Now an HTTPS connection with insecure TLS check will be
attempted first and will only fallback to HTTP if the former fails.

This fixes push to an insecure HTTPS-only registry.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
It's no longer needed to return multiple hosts.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
Fixes #4108

Signed-off-by: Mark Yen <mark.yen@suse.com>
This responds to review feedback from
moby/buildkit#4308 (review)

Signed-off-by: Mark Yen <mark.yen@suse.com>
Signed-off-by: Erik McKelvey <Erik.McKelvey.is@gmail.com>
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
.dockerignore relates more to build context than to the Dockerfile
frontend, so it makes more sense to document the syntax for this file
where we describe how build contexts work.

Signed-off-by: David Karlsson <35727626+dvdksn@users.noreply.github.com>
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Dockerfile (containerd v1.6): bump up containerd to 1.6.24
docs: mv .dockerignore help to context docs
Follow-up to 38040ae.

Messages returned by fsutil.Stat are guaranteed to return a PathError
wrapped by a stack trace (since it calls os.Lstat). However, as these
error messages are printed, they include the temporary directory for the
mounted reference which is not useful to the caller.

On an error, we can restore the filename in the PathError to the
requested filename, as also seen in os.DirFS.

Signed-off-by: Justin Chadwell <me@jedevc.com>
util/resolver: Perform Insecure HTTPS + HTTP fallback in one `docker.RegistryHost`
SCP-style URLs take the following format [1]:

	[user@]server:project.git

These are used and handled by git, so we should have a full-parser for
these, that return a custom URL type.

[1]: https://git-scm.com/book/en/v2/Git-on-the-Server-The-Protocols

Signed-off-by: Justin Chadwell <me@jedevc.com>
This resolves a regression introduced in
50e75e3. In this previous patch, I'd
incorrectly assumed that scp-like URLs can express a subset of
"standard"-URLs and so we can always safely convert them for
consistency. This isn't true - the URL "git@example.com:foo" should be
resolved to the home directory of the host, however, the converted URL
"ssh://git@example.com/foo" will be resolved to the root of the host.

To resolve this, we need to not perform this conversion. However, we
also need preserve the behaviour of firm distinction between SCP and
normal URL types (so as to keep proper port parsing).

To do this, we add a new GitURL type to the gitutil package. This new
type contains all useful fields shared in common between the standard
libraries url package and our custom scp-style url parsing package. This
keeps the previous property of a single clean interface to all GitURLs,
while also ensuring that we preserve the original URL to pass to the Git
CLI (making sure we strip fragments out, which are used as
buildkit-level metadata).

As a side-effect of this, the client-side calling code for parsing
git urls is simplified (so we don't have to do fragment wrangling at
every call point).

Signed-off-by: Justin Chadwell <me@jedevc.com>
Before this change, all platforms that loosely match the provided
platform will be fetched even though we only care about 1 of them.
As an example when linux/amd64 is requested it will also fetch linux/386
because it is a compatible architecture.
This means extra round trips to the registry, potentially even for
content that doesn't exist in the remote.

This is especially a problem when resolve mode is prefer-local because
we'll have the index locally but most likely only one manifest.
In this case we'll end up reaching out to the registry to fetch the
other manifests unncessarily.

With this change instead of fetching all matching platforms it chooses
only the best matching platform.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
ResolveImageConfig: Only fetch best matching config
Signed-off-by: Alex Couture-Beil <alex@mofo.ca>
solver: protect against nil rres upon errors
full diff: containerd/continuity@v0.4.1...v0.4.2

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
full diff: stretchr/testify@v1.8.3...v1.8.4

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
jedevc and others added 27 commits October 23, 2023 12:48
Signed-off-by: Justin Chadwell <me@jedevc.com>
This patch modifies the function signature of the FSSync provider to
take an fsutil.FS instead of a simple raw path resolved to the client's
root filesystem.

Internally, we were already creating an fsutil.FS to Send to the
buildkit server, however, this abstraction didn't reach the session
attachable parameters, so we couldn't provide our own custom FS
implementation.

The rationale behind this change is to allow providing more abstract
custom filesystem implementations to a BuildKit client. This way, we can
start to build from filesystems that might not be on disk - for example,
we could use our Static filesystem implementation in tests to prevent
creating lots of temporary directories, or we could use our Merge
filesystem implementation to allow easily creating variants of a single
context.

Signed-off-by: Justin Chadwell <me@jedevc.com>
This completes propogating the fsutil.FS abstraction into the SolveOpt,
deprecating the old LocalDirs.

Since this is entirely a golang-level abstraction, we could potentially
investigate just removing the old LocalDirs directly.

Signed-off-by: Justin Chadwell <me@jedevc.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
Now we error out if there is any clash between LocalDirs and
LocalMounts.

Signed-off-by: Justin Chadwell <me@jedevc.com>
fixes data corruption with zstd output in "best"

- 1.17.2 diff: klauspost/compress@v1.17.1...v1.17.2
- full diff: klauspost/compress@v1.16.3...v1.17.2

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Signed-off-by: 蝦米 <me@jhdxr.com>
client: modify SolveOpt to take fsutil.FS objects
vendor: github.com/klauspost/compress v1.17.2
llbsolver: fix possible deadlock in history listen
solver: fix issue with double merged edges
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
When running buildkitd in a rootless environment but as root, buildkitd
misinterprets how it is being run and loads the config file from the
home directory instead of the global location in `/etc/buildkitd`.

This happens when buildkit is being run in docker and docker itself is
being run as rootless. The buildkit daemon sees the user remapping and
assumes that the remapping belongs to itself rather than to docker.

This causes it to load the wrong configuration file as buildkitd is
still being run as "root" but a remapped root that docker created.

Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This allows callers to pass in a non-default build context without
creating and passing in a dockerui.Client.

Signed-off-by: Aaron Lehmann <alehmann@netflix.com>
Signed-off-by: Kohei Tokunaga <ktokunaga.mail@gmail.com>
server: prohibit more than MaxConcurrentStreams handlers from running at once
(CVE-2023-44487).

In addition to this change, applications should ensure they do not leave running
tasks behind related to the RPC before returning from method handlers, or should
enforce appropriate limits on any such work.

- grpc/grpc-go@v1.56.2...v1.56.3

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
…ss-docker

buildkitd: use default config file location when run as root in rootless
vendor: google.golang.org/grpc v1.56.3
Signed-off-by: CrazyMax <crazy-max@users.noreply.github.com>
@alexcb alexcb merged commit c7c1f7a into earthly-main Nov 6, 2023
37 of 54 checks passed
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.