- added clarity with error handling. added test to show issue.
- in manual testing, this fixes the issue and allows watch to run after rebuild
- added cleanup back in
- fixed issue where watch extnet rebuild test would start all containers listed in the fixture
Signed-off-by: kimdcottrell <me@kimdcottrell.com>
From the Go specification [1]:
"1. For a nil slice, the number of iterations is 0."
`len` returns 0 if the slice is nil [2]. Therefore, checking
`len(v) > 0` before a loop is unnecessary.
[1]: https://go.dev/ref/spec#For_range
[2]: https://pkg.go.dev/builtin#len
Signed-off-by: Eng Zer Jun <engzerjun@gmail.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>
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>
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>
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>
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>
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>
Attempting to fix the state of codecov action checks right now,
which are behaving very erratically.
Using the new functionality in Go 1.20 to merge multiple reports,
so now the unit & E2E coverage data reports are stored as artifacts
and then downloaded, merged, and finally uploaded to codecov as a
new job.
Additionally, add a `codecov.yml` config and try to turn down the
aggressiveness of it for CI checks.
Signed-off-by: Milas Bowman <milas.bowman@docker.com>
Nicolas De Loof
Ulysses Souza
Guillaume Lours
kimdcottrell
Eng Zer Jun
Matthieu MOREL
Milas Bowman
Sebastiaan van Stijn
Jes Cok