Previously, Sync() only checked for fs.ErrNotExist when classifying
paths into copy vs delete. Non-NotExist stat errors (e.g. EACCES,
EIO) caused the condition to be false, falling through to the else
clause which incorrectly treated the path as copyable. This masked
real errors and led to cryptic failures downstream.
Restructure the condition into a three-way branch:
- err == nil → copy
- ErrNotExist → delete
- other errors → return immediately with context
This follows the pattern already used by entriesForPath() in the
same file.
Fixes#13654
Signed-off-by: Lidang Jiang <lidangjiang@gmail.com>
Signed-off-by: Lidang-Jiang <lidangjiang@gmail.com>
Also update TestDefaultNetworkSettings:
Test that the network with the highest priority is returned as
"primary" network, and other networks as extra networks.
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
The go-multierror Group is just a shallow wrapper around sync.WaitGroup;
https://github.com/hashicorp/go-multierror/blob/v1.1.1/group.go#L5-L38
It does not limit concurrency, but handles synchronisation to collect
all errors (if any) in a go-multierror.
This patch replaces the go-multierror.Group for a sync.ErrGroup (which
is slightly easier to use, and does allow for limiting concurrency if
wanted), and a basic slice with mutex to collect the errors and to produce
a stdlib multi-error through errors.Join
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
This has not been the default for quite a while and required
setting an environment variable to revert back.
The tar implementation is more performant and addresses several
edge cases with the original `docker cp` version, so it's time
to fully retire it.
The scaffolding for multiple sync implementations remains to
support future experimentation here.
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>
Support services with scale > 1 for the tar watch sync.
Add a "lossy" multi-writer specific to pipes that writes the
tar data to each `io.PipeWriter`, which is connected to `stdin`
for the `tar` process being exec'd in the container.
The data is written serially to each writer. This could be
adjusted to do concurrent writes but that will rapidly increase
the I/O load, so is not done here - in general, 99% of the
time you'll be developing (and thus using watch/sync) with a
single replica of a service.
If a write fails, the corresponding `io.PipeWriter` is removed
from the active set and closed with an error.
This means that a single container copy failing won't stop
writes to the others that are succeeding. Of course, they will
be in an inconsistent state afterwards still, but that's a
different problem.
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>