It is now possible to send next and continue as separate signals. When
executing a build, the debug adapter will divide the LLB graph into
regions. Each region corresponds to an uninterrupted chain of
instructions. It will also record which regions depend on which other
ones.
This determines the execution order and it also determines what the
stack traces look like.
When continue is used, we will attempt to evaluate the last leaf node
(the head). If we push next, we will determine which digest would be the
next one to be processed.
In the future, this will be used to also support breakpoints.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Adds a simple implementation of the debug adapter that supports the very
basics of a debug adapter.
It supports the launch request, the configuration done request, the
creation of threads, stopping, resuming, and disconnecting from server.
It does not support custom breakpoints, stack traces, or variable
inspection yet. These are planned to be added in the future.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Some minor fixes to the printer and how bake invokes it. Bake previously
had a race condition that could result in the display not updating on an
error condition, but it was much rarer because the channel communication
was much closer. The refactor added a proxy for the status channel so
there was more of an opportunity to surface the race condition.
When bake exits with an error when reading the bakefiles, it doesn't
wait for the printer to finish so it is possible for the printer to
update the display after an error is printed. This adds an extra `Wait`
in a defer to make sure the printer is finished.
`Wait` has also been fixed to allow it to be called multiple times and
have the same behavior. Previously, it only waited for the done channel
once so only the first wait would block.
The `onclose` method is now called every time the display is paused or
stopped. That was the previous behavior and it's been restored here.
The display only gets refreshed if we aren't exiting. There's no point
in initializing another display if we're about to exit.
The metric writer attached to the printer was erroneously removed. It is
now assigned properly.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The package just causes the entire flow to be more complicated as build
has to pretend it doesn't know about debug options and the debugger has
to pretend it doesn't know about the build.
This abstraction has been difficult when integrating a DAP command into
this same workflow so I don't think this abstraction has much of a
value.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This changes the build handler to customize the behavior of evaluate
rather than onresult and also simplifies the `ResultHandle`. The
`ResultHandle` is now only valid within the gateway callback and can be
used to start containers from the handler.
`Evaluate` now executes inside of the gateway callback rather than
having a separate implementation that executes or re-invokes the build.
This keeps the gateway callback session open until the debugger has
returned.
The `ErrReload` for monitor has now been moved into the `build` package
and been renamed to `ErrRestart`. This is because it restarts the build
so the name makes a bit more sense. The actual use of this functionality
is still tied to the monitor reload.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This changes the progress printer's pause and unpause implementation to
be reentrant to prevent race conditions and it also allows the status
updates to be buffered when the display is paused.
The previous implementation mixed the pause implementation with the
finish implementation and could cause a send on closed channel panic
because it could close the status channel before it had finished being
used. Now, the status channel is not closed.
When the display is enabled, the status channel will be forwarded to an
internal channel that is used to display the updates. When the display
is paused, the status channel will have the statuses buffered in memory
to be sent when the progress display is resumed.
The `Unpause` method has also been renamed to `Resume`.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Removes all references to the controller and moves the remaining
sections of code to other packages.
Processes has been moved to monitor where it is used and the data
structs have been removed so buildflags is used directly. The controller
build function has been moved to the commands package.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This creates a `Monitor` type that keeps the global state between
monitor invocations and allows the monitor to exist during the build so
it can be utilized for callbacks.
The result handler is now registered with the monitor during the build
and `Run` will use the result if it is present and the configuration
intends the monitor to be invoked with the given result.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The build now happens in a loop and the monitor is run after every
build. The monitor can return `ErrReload` to signal to the main thread
that it should reload the build result.
This will be used in the future to move the monitor into a callback
rather than as a separate existence. It allows the monitor to not
control the build itself which now makes it possible to completely
remove the controller.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The monitor needs stdin to run and isn't compatible with loading a
context or dockerfile from stdin. We already disallow this combination
and, with the removal of the remote controller, there's no way to use
stdin during the build when invoke is configured.
This just removes the extra code to allow forwarding stdin to the build
when the monitor is configured to simplify that section of code.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The controller interface is removed and the local controller is used for
only the initial build, invoke, and rebuilds.
Process control has been moved to the monitor.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The `--keep-storage` flag was changed to `--reserved-space`. Before it was
changed to that name, it was changed to `--max-storage`. This flag never
made it into a release as the name was changed before release, but the
update to the flag in buildx forgot to update the deprecation notice.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
With the removal of the protobuf for the controller, there are no longer
any generated files. Remove the makefile targets and the associated
dockerfiles and bake targets.
This wasn't being included in CI because it wasn't part of the
`validate` target.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Remove the protobuf files associated with controller/errdefs.
This doesn't completely remove the type as the monitor still uses it as
a signal to start the monitor.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Broken in 11c84973ef. The section to skip
an empty input was accidentally removed when some code was refactored to
fix a separate issue.
This skips empty cache entries which allows disabling the `cache-from` and
`cache-to` entries from the command line overrides.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Annotations were not merged correctly. The overrides in `ArrValue` would
be merged, but the section of code setting them from the command line
did not include `annotations` in the list of available attributes so the
command line option was completely discarded.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Reverts the usage of rangefunc and attempts to keep the foundation of it
in for when we move to go 1.23. We have downstream dependencies that
aren't ready to move to go 1.23. We can likely move after go 1.24 is
released.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Update the buildflags cty code to handle unknown values. When hcl
decodes a value with an invalid variable name, it appends a diagnostic
for the error and then returns an unknown value so it can continue
processing the file and finding more errors.
The iteration code has now been changed to use a rangefunc from go 1.23
and it skips empty or unknown values. Empty values are valid when they
are skipped and unknown values will have a diagnostic for itself.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
`MarshalJSON` would not include the extra attributes because it iterated
over the target map rather than the source map.
Also fixes JSON unmarshaling for SSH and secrets. The intention was to
unmarshal into the struct, but `UnmarshalText` takes priority over the
default struct unmarshaling so it didn't work as intended.
Tests have been added for all marshaling and unmarshaling methods.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This changes how the composable attributes are implemented and provides
various fixes to the first iteration.
Cache-from and cache-to now no longer print sensitive values that are
automatically added. These automatically added attributes are added when
the protobuf is created rather than at the time of parsing so they will
no longer be printed. If they are part of the original configuration
file, they will still be printed.
Empty strings will now be skipped. This was the original behavior and
composable attributes removed this functionality accidentally. This
functionality is now restored.
This also expands the available syntax that works with each of the
composable attributes. It is now possible to interleave the csv syntax
with the object syntax without any problems. The canonical form is still
the object syntax and variables are resolved according to that syntax.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This allows using either the csv syntax or object syntax to specify
certain attributes.
This applies to the following fields:
- output
- cache-from
- cache-to
- secret
- ssh
There are still some remaining fields to translate. Specifically
ulimits, annotations, and attest.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Improves the error message when using an incorrect format for setting
labels. This includes the intended format directly in the error message
instead of assuming the user knows what the format is.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
When run in standalone mode, the environment variables
`DOCKER_BUILDX_CPU_PROFILE` and `DOCKER_BUILDX_MEM_PROFILE` will cause
profiles to be written by the CLI.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Integrates vtproto into buildx. The generated files dockerfile has been
modified to copy the buildkit equivalent file to ensure files are laid
out in the appropriate way for imports.
An import has also been included to change the grpc codec to the version
in buildkit that supports vtproto. This will allow buildx to utilize the
speed and memory improvements from that.
Also updates the gc control options for prune.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Similar to https://github.com/docker/buildx/pull/2716.
Use the file size rather than the proto size, raise the allowed limit to
the same value for consistency, and improve the error message to include
the limit in human units.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Raise the maximum size limit for the dockerfile and correct the size
check. The size check was intended to use the size attribute from the
file stat, but the original gogo version confused the `Size()`
method (which returned the size of the proto message) with the `Size`
attribute (which was named `Size_`).
During the conversion, we noticed the mistake but kept the incorrect
behavior for the sake of keeping the conversion simple.
This also raises the maximum limit because 512 kB is likely a bit too
conservative. The limit has been raised to 2 MB and the limit has been
included in the error message.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Removes gogo/protobuf from buildx and updates to a version of
moby/buildkit where gogo is removed.
This also changes how the proto files are generated. This is because
newer versions of protobuf are more strict about name conflicts. If two
files have the same name (even if they are relative paths) and are used
in different protoc commands, they'll conflict in the registry.
Since protobuf file generation doesn't work very well with
`paths=source_relative`, this removes the `go:generate` expression and
just relies on the dockerfile to perform the generation.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
It was possible for multiple status messages to be written at the same
time which caused some of the metric writer code to have a race
condition.
This code should be fast enough that it doesn't interrupt the display,
but some further work might be needed here.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This adds metrics for the bake command using a different method of
calculating the build identifier but with the same attributes otherwise.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Compile the regular expressions only on first use rather than implicitly
as part of the `init()` function of the package. This prevents taking a
speed hit on the initialization of the package regardless if this type
is used and moves it to the time when a regular expression is first
used.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This adds an alias for `--check` that causes it to behave the same as
`--call=check`. This is done using `BoolFunc` to call a function when
the option is seen and to set it to the correct value. This should allow
command line flags like `--check --call=targets` to work correctly (even
though they conflict) by making it so the first invocation sets the
print function to `check` and the second overwrites the first. This is
the expected behavior for these types of boolean flags.
`BoolFunc` itself is part of the standard library flags package, but
never seems to have made it into pflag possibly because it was added in
go 1.21.
https://pkg.go.dev/flag#FlagSet.BoolFunc
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Update buildkit dependency to v0.14.0-rc1. Update the tracing
infrastructure to use the new detect API which updates how the delegated
exporter is configured.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This metric records the number of times it sees a lint warning in the
progress stream and categorizes the number of times each rule has been
triggered. This will only record whether a lint warning was triggered
and not whether the linter was even used or which rules were present.
That information isn't presently part of the stream.
With this change, we might be reaching some of the limitations that
spying on the progress stream gives us for metrics and may want to
consider another way for the build to communicate metrics back to the
client.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This version of docker/cli has changes to remove compose-cli wrapper and
move all CLI metrics to OTEL.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
(cherry picked from commit 4fc4bc07ae)
This version of docker/cli has changes to remove compose-cli wrapper and
move all CLI metrics to OTEL.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The meter provider initialization that was located here has now been
moved to a common area in the docker cli. This upgrades our CLI version
and then uses this common code instead of our own version.
As a piece of additional functionality, the docker OTEL endpoint can now
be overwritten with `DOCKER_CLI_OTEL_EXPORTER_OTLP_ENDPOINT` for
testing.
This removes the OTLP exporter from the CLI that was previously locked
behind `BUILDX_EXPERIMENTAL`. I do plan for this to return, but as a
proper part of the `docker/cli` implementation rather than something
special with `buildx`.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This refactors the driver handle to initialize the tracer delegate
inside of the driver handle instead of the individual plugins.
This provides more uniformity to how the tracer delegate is created by
allowing the driver handle to pass additional client options to the
drivers when they create the client. It also avoids creating the tracer
delegate client multiple times because the driver handle will only
initialize the client once. This prevents some drivers, like the remote
driver, from accidentally registering multiple clients as tracer
delegates.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Removes the experimental label from progress metrics. User-metrics
themselves are still experimental so this is still blocked behind the
experimental flag, but this will allow the docker otlp endpoint to
receive these metrics.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This measures the amount of time spent idle during the build. This is
done by collecting the set of task times, determining which sections
contain gaps where no task is running, and aggregating that duration
into a metric.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This measures the duration of exec operations. It does not factor in
whether the operation was cached or not so this should include the
amount of time to determine whether an operation was cached.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This measures the transfer size and duration for image pulls along with
the time spent extracting the image contents.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Measure the transfer size and duration of context transfers for various
categories of local source transfers from the progress stream that's
returned during the build.
Local source transfers are split into one of four categories:
* context
* dockerfile
* dockerignore
* namedcontext
Named contexts that are different names will be categorized under the
same metric.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The `BUILDX_EXPERIMENTAL` check is removed from the docker otel
collector. We'll send metrics to the OTLP endpoint for docker desktop if
it is present and enabled regardless of experimental status.
The user-facing `OTEL` endpoints for enabling the metric reporting for
external use is still hidden behind the experimental flag. We'll likely
remove the experimental flag for this feature for v0.14.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This adds a build duration metric for the build command with attributes
related to the buildx driver, the error type (if any), and which options
were used to perform the build from a subset of the options.
This also refactors some of the utility methods used by the git tool to
determine filepaths into its own separate package so they can be reused
in another place.
Also adds a test to ensure the resource is initialized correctly and
doesn't error. The otel handler logging message is suppressed on buildx
invocations so we never see the error if there's a problem with the
schema url. It's so easy to mess up the schema url when upgrading OTEL
that we need a proper test to make sure we haven't broken the
functionality.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Add the service instance id to the resource attributes to prevent
downstream OTEL processors and exporters from thinking that the CLI
invocations are a single process that keeps restarting. The unique id
can be removed through downstream aggregation to prevent cardinality
issues, but we need some way to tell OTEL that it shouldn't reset the
counters.
Move the check for the experimental flag to its own package and then use
that invocation to prevent creating exporters so metrics are disabled
completely. This makes it so we don't have to check for the experimental
flag in every place we add metrics until we decide to make metrics
stable in general.
This also moves the OTEL initialization to a `util/metricutil` package
to be more consistent with the existing util naming and to differentiate
it from the upstream `metric` name. Using both `metrics` and `metric` as
import names was confusing since `metric` was an upstream dependency and
`metrics` was a local utility. `metricutil` matches with the existing
utilities and makes clear that it isn't a spelling mistake.
The record version metric has been removed since we weren't planning on
keeping that metric anyway and most of the information is now included
in the instrumentation library name and version. That function is
included as a utility in the `otel/sdk/metric` package to retrieve the
appropriate meter from the meter provider.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
Introduce a meter provider to the buildx cli that will send metrics to
the otel-collector included in docker desktop if enabled.
This will send usage metrics to the desktop application but also send
metrics to a user-provided otlp receiver endpoint through the standard
environment variables.
This introduces a single metric which is the cli count for build and
bake along with the command name and a few additional attributes.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
This adds status reporting for the remote driver so it shows the length
of time it is spending waiting for the connection. If the service is
already present, this logger isn't shown but it should help provide a
message to show the user why the build is stalled.
A timeout of 20 seconds has been added to the bootstrap.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
The `docker-container` driver relies on the default config file location
for buildkit when writing the configuration file. When run in a rootless
version of docker (dind), the default location is different.
Instead of trying to figure out where the appropriate default location
is, this just writes the files to the same location and sets the
`--config` parameter explicitly. This flag is placed first so a
user-specified config option in `--buildkitd-flags` will take precedence
over the implicit config parameter.
This also fixes the `--config` option with the rootless image.
Previously, the config directory was being copied in a way that rendered
`/etc` unreadable and the configuration file wasn't readable either. It
also wasn't copied to the correct place. Now, `--config` is used to
specify the directory, `/etc` isn't included in the copied archive (so
the permissions aren't overwritten), and the directory is set as world
readable to be readable from the rootless buildkit process`.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
If the boot command hung or took a long time, it blocked any read
operations (such as `buildx ls`).
When the boot happens, we no longer need to hold the file lock so we can
release it. Releasing multiple times is legal and causes whichever
release that is second to be a no-op so the defer is kept to ensure the
lock is released even when an error condition happens.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
An integration test for `docker buildx version` has been created. The
integration test checks that there is one line output, the output is
composed of three sections, and that these sections could feasibly be
the package path, version, and revision information.
The intention of the checks is to find obvious errors in the output like
the package path not existing or the version and revision being swapped.
It is not intended to assert that these values must be certain values
because it is assumed these values may vary depending on the build
process for buildx.
Signed-off-by: Jonathan A. Sternberg <jonathan.sternberg@docker.com>
