Backlink walking should not be needed to determine layers for
current build and should be safe to skip. This improves performance
of provenance creation for certain builds.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Check that the result returned from the frontend
matches the user request conventions and show a
warning if it doesn't.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
20 sec stall on an platter old hdd is not unexpected.
Nor is 20s write on a low-tier cloud environment drive unexpected.
300s should however be enough to tell the difference between crappy hardware and actual failure.
fixes#4327
Signed-off-by: Dennis Haney <davh@davh.dk>
In the case for Windows, this line at
frontend/dockerfile/dockerfile2llb/convert.go#L1142
```go
dest += string(filepath.Separator)
```
was adding the `\\` to a path that is already normalized
to unix-format, hence ending up with dest paths like
`/\\` for `C:\\` and `/test\\` for `C:\\test\\`.
the src paths are well normalized too at ~L1290.
This change removes the block of code and instead
does the "/" appending using the keepSlash logic
that is in system.NormalizePath called in
pathRelativeToWorkingDir() function before.
fixes#4696
Signed-off-by: Anthony Nandaa <profnandaa@gmail.com>
The trace recorder is now a separate entity and does not wrap another
span exporter. It is added as another span processor to the underlying
tracer provider so the two can be disconnected from each other and this
removes the need to use `detect.Exporter()` to find the TraceRecorder
along with the need to invoke flush manually.
The trace recorder is wrapped with a simple span processor instead of
the batch one. That makes the flushing irrelevant for this purpose.
The trace recorder itself is also modified to avoid leaking goroutines
and respecting context cancellations.
This is part of a general refactor of the detect package to separate the
buildkit-specific functionality from the external exporter detection.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Follow-up to ce332e1952 - this didn't
resolve the access to sysSampler.Close, we need to stub this one out as
well!
Signed-off-by: Justin Chadwell <me@jedevc.com>
This is more versatile function that works for any source,
not just images.
It can be used together with a policy that switches
between input and output source as well as for adding
additional metadata for other sources in the future.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
These test all of the new behavior:
- Checks for old default no content cache
- Checks for old read-only and no-output allowed content cache
- Checks for new root selector allowed content cache
- Checks for new caller options that allow enabling/disabling it
Signed-off-by: Justin Chadwell <me@jedevc.com>
This allows LLB-directed content-based cache enablement for each mount.
Some mounts may not be explicitly unabled (because it would be unsafe) -
for these cases we explicitly error out.
Signed-off-by: Justin Chadwell <me@jedevc.com>
These mounts are actually safe, as suggested by Erik on slack:
> Is it correct that this wouldn’t be a problem in the case where the
> selector of the mount is just “/“? Because then there’s no “hidden”
> files.
>
> If so, maybe there’s a path to enabling content cache for rw mounts
> that are from “/“. Then you could also get the same end behavior by
> not using selectors and instead always copying the subdir you want to
> mount to scratch.
This patch adds a check for this case, and explicitly enables
content-based cache for these cases.
Co-authored-by: Erik Sipsma <erik@sipsma.dev>
Signed-off-by: Justin Chadwell <me@jedevc.com>
This refactors the content-based cache to be that little bit tidier. In
addition to adding comments that explain *why* we're even bothering,
this restructures the code to avoid being unclear.
To explain the changes in a little more detail (since it's not
abundantly clear why this translation is valid), the initial condition
looks like:
if (!m.Readonly || m.Dest == pb.RootMount) && m.Output != -1 {
deps[m.Input].NoContentBasedHash = true
We can apply De Morgan's law recursively to invert the condition and the
result:
deps[m.Input].NoContentBasedHash = true
if (m.Readonly && m.Dest != pb.RootMount) || m.Output == -1 {
deps[m.Input].NoContentBasedHash = false
With all the juggling of NoContentBasedCache, we invert the variable
name to be ContentBasedCache (and invert everywhere it's used as well):
if (m.Readonly && m.Dest != pb.RootMount) || m.Output == -1 {
deps[m.Input].ContentBasedHash = true
This reads a bit easier, but now we split this into two separate
branches for readability (and so we can comment each one in more detail
separately):
if m.Readonly && m.Dest != pb.RootMount {
deps[m.Input].ContentBasedHash = true
}
if m.Output == -1 {
deps[m.Input].ContentBasedHash = true
}
While this has been the behavior for ages, I think it makes sense to
deliberately this behavior slightly. It doesn't make sense to me that we
should only disallow read-only root mounts, but no-output root mounts
are allowed - there's no reason these shouldn't behave identically, by
splitting these out:
if m.Readonly {
deps[m.Input].ContentBasedHash = true
}
if m.Output == -1 {
deps[m.Input].ContentBasedHash = true
}
if m.Dest == pb.RootMount {
deps[m.Input].ContentBasedHash = false
}
There is a small chance that this is a breaking change for some users,
however, 1. SkipOutput (-1) is very rare and not often used in the wild
(except for dockerfiles, where it's only used for non-root mounts), and 2.
will only cause a cache miss.
Signed-off-by: Justin Chadwell <me@jedevc.com>
Ensure interactive calls validate same conditions that
the build requests do. Refactor of the build side is to ensure
we use the same validation function for both cases. There
was no validation issue with the LLB validation.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit d1970522d7145be5f4a1f1a028b1910bb527126c)
Running interactive container APIs was done by giving
the gateway implementation access to worker controller
directly, but it should be passed with a build job instead.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit 0971dffaab93d91e51af984b44c745b35b3c5b4d)
The solver has a Close method to shutdown the scheduler, which releases
a goroutine. We should call it on shutdown.
While in the area, we can also close the sysSampler.
Signed-off-by: Justin Chadwell <me@jedevc.com>
A cache ID should not have any impact on whether or not a step should be
re-run any more than the content of that cache does (or rather,
doesn't).
Signed-off-by: Brian Goff <cpuguy83@gmail.com>
While debugging around, I found some instances of os.ErrNotExist
appearing in the logs from the history API - however, no stack traces
were available, since they're not wrapped.
This patch just adds wrapping for them, with some additional debbuging
info.
Signed-off-by: Justin Chadwell <me@jedevc.com>
This is possible with llb.Scratch in a multi-platform build. We were
accidentally discarding the nil entries in Refs.
Instead, we just skip over nil refs. This isn't ideal - we *should* be
able generate provenance for a nil ref. However, 1. this isn't handled
for llb.Scratch as a single-platform result, and 2. this is tricky,
since the ResultProxy is nil at this point (so there's no ID to match
on). This should be ok for a quick fix, but we can come back and fix
this later.
Signed-off-by: Justin Chadwell <me@jedevc.com>
We can derive exporter ids from their place in the exporter array in a
SolveRequest - this removes the need to manually generate and handle
multiple sets of IDs.
Signed-off-by: Justin Chadwell <me@jedevc.com>
This patch adds support for multiple exporters at the control API, and
propogates the resulting required changes through the client and the
solver.
A few notable changes:
- Each exporter instance now has an associated identifier
- Build records in the build history now have multiple possible
descriptors to built content
- Exporter responses are all merged together (like we currently do with
multiple cache exporters). We likely will need to revisit this design
later, since now cache exporters do not line up one-to-one with
exporters.
For backwards compatability, new clients will continue to produce
requests that contain the now deprecated exporter fields, as well as the
new ones. New servers will attempt to use deprecated fields if they
are present.
Co-authored-by: a-palchikov <deemok@gmail.com>
Co-authored-by: fahed dorgaa <fahed.dorgaa@gmail.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
This preps for the case where a single exporter source does not map
neatly onto a single inline cache - such as introducing multiple
exporters.
We also introduce laziness here - each exporter chooses to attempt
extracting the inline cache on demand, which ensures that we avoid
creating inline caches for exporters that do not support them.
Co-authored-by: a-palchikov <deemok@gmail.com>
Co-authored-by: fahed dorgaa <fahed.dorgaa@gmail.com>
Signed-off-by: Justin Chadwell <me@jedevc.com>
This exposes the otel metrics through the environment variables for the
otel collector or through the prometheus `/metrics` endpoint through the
debug address.
This also adds metrics for the grpc endpoints from the opentelemetry
libraries.
For the opentelemetry collector, the metric temporality is delta which
reduces the amount of data that goes over the wire overall. Prometheus
metrics are cumulative because prometheus metrics require that.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
