* 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>
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>
* When waiting for dependencies, `select` on the context as well
as the ticker
* Write multiple progress events "transactionally" (i.e. hold the
lock for the duration to avoid other events being interleaved)
* Do not change "finished" steps back to "in progress" to prevent
flickering
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Move builder and nodes initialization code up, avoiding to recreate/load them for every service build.
Signed-off-by: Silvin Lubecki <silvin.lubecki@docker.com>
Adjust the debouncing logic so that it applies to all inbound file
events, regardless of whether they match a sync or rebuild rule.
When the batch is flushed out, if any event for the service is a
rebuild event, then the service is rebuilt and all sync events for
the batch are ignored. If _all_ events in the batch are sync events,
then a sync is triggered, passing the entire batch at once. This
provides a substantial performance win for the new `tar`-based
implementation, as it can efficiently transfer the changes in bulk.
Additionally, this helps with jitter, e.g. it's not uncommon for
there to be double-writes in quick succession to a file, so even if
there's not many files being modified at once, it can still prevent
some unnecessary transfers.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
We cannot guarantee the exact value of `CapEff` across
environments, and this test has started failing some places,
e.g. Docker Desktop, and now GitHub Actions (likely due to
a kernel upgrade on the runners or similar).
By setting `privileged: true` on the build, we're asking for
the `security.insecure` entitlement on the build. A safe
assumption is that will include `CAP_SYS_ADMIN`, which won't
be present otherwise, so mask the `CapEff` value and check
for that.
It's worth noting that realistically, the build won't even
be able to complete without the correct entitlement, since the
`Dockerfile` uses `RUN --security=insecure`, so this is really
an additional sanity check.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Just moving some code around in preparation for an alternative
sync implementation that can do bulk transfers by using `tar`.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
It's no longer used in docker/cli, and doesn't do anything other than
creating an empty struct, so replacing it (as we're planning to
deprecate that function)
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
When building, if images are being pushed, ensure that only
named images (i.e. services with a populated `image` field)
are attempted to be pushed.
Services without `image` get an auto-generated name, which
will be a "Docker library" reference since they're in the
format `$project-$service`, which is implicitly the same as
`docker.io/library/$project-$service`. A push for that is
never desirable / will always fail.
The key here is that we cannot overwrite the `<svc>.image`
field when doing builds, as we need to be able to check for
its presence to determine whether a push makes sense.
Fixes#10813.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Fix forthcoming via https://github.com/compose-spec/compose-go/pull/436
which addresses some symlink limitations. These can
actually effect other platforms but are most common
on macOS because the test creates temporary directories,
which are symlinked on macOS.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Lots of our phony Compose files launch pointless long-lived processes
so we can assert on state. However, this means they often don't respond
well to signals on their own, requiring Compose to timeout and kill
them when doing a `down`.
Add in lots of `init: true` where appropriate so that we don't block
for no reason while running E2E tests all over the place.
Additionally, a couple tests have gotten a cleanup so they don't leave
behind containers. I still want to build this into the framework in
the future, but this is easier for the moment and won't cause any
trouble in the future.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Add an end-to-end test that covers the core watch functionality,
i.e. CRUD on files & directories.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
* Run `down` before and after test to not leave around containers
* Kill the `wait` process that's waiting on `infinity`
* NOTE: If the test is actually working, this should exit once
the `down` happens, but this ensures that we kill everything
we start
I'd like to generalize more of this into the framework, but this
is a quick fix to prevent filling up CI machines with tons of
processes over time.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
closes#10783
Compose Spec mentions that default values for secrets is `0444` aka. world-readable permissions. However, the value was previously set to `0400`.
Signed-off-by: Shan Desai <shantanoo.desai@gmail.com>
As part of the fix for #10668, the logic was adjusted so that the
default (highest-priority) network is used in the `ContainerCreate`,
and then the remaining networks are connected via calls to
`NetworkConnect` before starting the container.
Unfortunately, `ServiceConfig::NetworksByPriority` is neither
deterministic nor stable when networks have the same priority.
It's non-deterministic because the order of networks from parsing
YAML is random, since they are loaded into a Go map (which have
random iteration order). Additionally, it's not using a `SortStable`
in `compose-go`, so even if the load order was predictable, it
still might produce different results.
While I look at improving `compose-go` here to prevent this from
tripping us up in the future, this fix looks at _all_ networks for
a service and ignores the "default" one now. Before, it would
always skip the first one in the slice since that _should_ have
been the "default".
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Engine API only allows at most one network to be connected as
part of the ContainerCreate API request. Compose will pick the
highest priority network.
Afterwards, the remaining networks (if any) are connected before
the container is actually started.
The big change here is that, previously, the highest-priority
network was connected in the create, and then disconnected and
immediately reconnected along with all the others. This was
racy because evidently connecting the container to the network
as part of the create isn't synchronous, so sometimes when Compose
tried to disconnect it, the API would return an error like:
```
container <id> is not connected to the network <network>
```
To avoid needing to disconnect and immediately reconnect, the
network config logic has been refactored to ensure that it sets
up the network config correctly the first time.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
