I missed this during a refactor and there wasn't test coverage.
Instead of adding more heavy-weight integration tests, I tried
to use `gomock` here to assert on the options objects after CLI
flag parsing. I think with a few more helpers, this could be a
good way to get a lot more combinations covered without adding
a ton of slow E2E tests.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
The `alpha watch` command current "attaches" to an already-running
Compose project, so it's necessary to run something like
`docker compose up --wait` first.
Now, we'll do the equivalent of an `up --build` before starting the
watch, so that we know the project is up-to-date and running.
Additionally, unlike an interactive `up`, the services are not stopped
when `watch` exits (e.g. via `Ctrl-C`). This prevents the need to start
from scratch each time the command is run - if some services are already
running and up-to-date, they can be used as-is. A `down` can always be
used to destroy everything, and we can consider introducing a flag like
`--down-on-exit` to `watch` or changing the default.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
- Removed the broken link in README.md file about `backward compatibility`.
- The section
`# About update and backward compatibility` was removed in an earlier PR (https://github.com/docker/compose/pull/10889/files).
Signed-off-by: Lokesh Kumar <lkumar94@gmail.com>
The big change here is to pass around an explicit `*BuildOptions` object
as part of Compose operations like `up` & `run` that may or may not do
builds. If the options object is `nil`, no builds whatsoever will be
attempted.
Motivation is to allow for partial rebuilds in the context of an `up`
for watch. This was broken and tricky to accomplish because various parts
of the Compose APIs mutate the `*Project` for convenience in ways that
make it unusable afterwards. (For example, it might set `service.Build = nil`
because it's not going to build that service right _then_. But we might
still want to build it later!)
NOTE: This commit does not actually touch the watch logic. This is all
in preparation to make it possible.
As part of this, a bunch of code moved around and I eliminated a bunch
of partially redundant logic, mostly around multi-platform. Several
edge cases have been addressed as part of this:
* `DOCKER_DEFAULT_PLATFORM` was _overriding_ explicitly set platforms
in some cases, this is no longer true, and it behaves like the Docker
CLI now
* It was possible for Compose to build an image for one platform and
then try to run it for a different platform (and fail)
* Errors are no longer returned if a local image exists but for the
wrong platform - the correct platform will be fetched/built (if
possible).
Because there's a LOT of subtlety and tricky logic here, I've also tried
to add an excessive amount of explanatory comments.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
If running `up` in foreground mode (i.e. not `-d`),
when exiting via `Ctrl-C`, Compose stops all the
services it launched directly as part of that `up`
command.
In one of the E2E tests (`TestUpDependenciesNotStopped`),
this was occasionally flaking because the stop
behavior was racy: the return might not block on
the stop operation because it gets added to the
error group in a goroutine. As a result, it was
possible for no services to get terminated on exit.
There were a few other related pieces here that
I uncovered and tried to fix while stressing this.
For example, the printer could cause a deadlock if
an event was sent to it after it stopped.
Also, an error group wasn't really appropriate here;
each goroutine is a different operation for printing,
signal-handling, etc. If one part fails, we don't
actually want printing to stop, for example. This has
been switched to a `multierror.Group`, which has the
same API but coalesces errors instead of canceling a
context the moment the first one fails and returning
that single error.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
The uuid package in distribution was created as a utility for the distribution
project itself, to cut down external dependencies (see [1][1]).
For compose, this has the reverse effect, as it now brings all the dependencies
of the distribution module with it.
This patch switches to the uuid generation to crypto/rand to produce a random
id. I was considering using a different uuid implementation, or docker's
"stringid.GenerateRandomID", but all of those are doing more than needed,
so keep it simple.
Currently, this change has little effect, because compose also uses the
distribution module for other purposes, but the distribution project is
in the process of moving the "reference" package to a separate module,
in which case we don't want to depend on the distribution module only for
the uuid package.
[1]: 36e34a55ad
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
* Use unique project name prefixes (some of these tests assert
on output using the project name as a magic string, so could
be impacted by other tests with the same project name prefix)
* Tear down port range project before starting to try and avoid
race conditions with the engine and port assignment
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
This is a good place to start introducing (local) exclusivity
to Compose. Now, when `alpha watch` launches, it will check for
the existence of a PID file in the user XDG runtime directory,
and create one if the existing one is stale or does not exist.
If the PID file exists and is valid, an error is returned and
Compose exits.
A slight tweak to the experimental remote Git loader has been
made to use the XDG package for consistency.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
We can't assume we receive container logs line by line. Some framework won't buffer output and will send char by char, and we also can receive looong lines which get buffered to 32kb and then cut into multiple logs.
This assumes we will catch container streams being closed before we receive a die event for container, which could be subject to race condition, but at least the impact here is minimal and the fix works for reproduction examples provided in linked issues.
Signed-off-by: Nicolas De Loof <nicolas.deloof@gmail.com>
The BuildKit dockerignore package was integrated in the patternmatcher
repository / module. This patch updates our uses of the BuildKit package
with its new location.
A small local change was made to keep the format of the existing error message,
because the "ignorefile" package is slightly more agnostic in that respect
and doesn't include ".dockerignore" in the error message.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
By default, `compose up` attaches to all services (i.e.
shows log output from every associated container). If
a service is specified, e.g. `compose up foo`, then
only `foo`'s logs are tailed. The `--attach-dependencies`
flag can also be used, so that if `foo` depended upon
`bar`, then `bar`'s logs would also be followed. It's
also possible to use `--no-attach` to filter out one
or more services explicitly, e.g. `compose up --no-attach=noisy`
would launch all services, including `noisy`, and would
show log output from every service _except_ `noisy`.
Lastly, it's possible to use `up --attach` to explicitly
restrict to a subset of services (or their dependencies).
How these flags interact with each other is also worth
thinking through.
There were a few different connected issues here, but
the primary issue was that running `compose up foo` was
always attaching dependencies regardless of `--attach-dependencies`.
The filtering logic here has been updated so that it
behaves predictably both when launching all services
(`compose up`) or a subset (`compose up foo`) as well
as various flag combinations on top of those.
Notably, this required making some changes to how it
watches containers. The logic here between attaching
for logs and monitoring for lifecycle changes is
tightly coupled, so some changes were needed to ensure
that the full set of services being `up`'d are _watched_
and the subset that should have logs shown are _attached_.
(This does mean faking the attach with an event but not
actually doing it.)
While handling that, I adjusted the context lifetimes
here, which improves error handling that gets shown to
the user and should help avoid potential leaks by getting
rid of a `context.Background()`.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Refactor to use a consistent code path for determining the build
args for a service image regardless of whether BuildKit or the
classic builder is being used.
After recent changes, these code paths had diverged, so the classic
builder was missing the proxy variables from the Docker client
config.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
This was left over from debugging, but we should not block.
OTel will handle the connection in the background.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Swap the default implementation now that batching is merged.
Keeping the `docker cp` based implementation around for the
moment, but it needs to be _explicitly_ disabled now by setting
`COMPOSE_EXPERIMENTAL_WATCH_TAR=0`.
After the next release, we should remove the `docker cp`
implementation entirely.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
If an optional dependency exits successfully (exit code of 0),
with a service condition of `service_completed_successfully`,
don't log a warning.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Milas Bowman
Rory
Nicolas De Loof
Guillaume Lours
Lokesh Kumar
Sebastiaan van Stijn
Bilal Khan
dependabot[bot]
Silvin Lubecki
relrelb