It looks like the intent is to keep track of the index in 'ret' where a
destination was written, but that's not what the current code is doing.
Signed-off-by: Aaron Lehmann <alehmann@netflix.com>
HTTP/2(RFC7540) defines :authority pseudo header includes the authority portion
of target URI but it must not include userinfo part (i.e. url.Host).
However, when TLS certificate specified, grpc-go requires it must match
with its servername specified for certificate validation.
Signed-off-by: Shingo Omura <everpeace@gmail.com>
This breaks the giant blob that was the diffApply function into two
separate parts, a differ and an applier, which results in more modular
code that should be easier to follow and easier to make any future
updates to. For example, if we want to optimize by allowing differ and
applier to run in parallel in the future, that's straightforward now.
There are also some fixes that weren't needed for MergeOp, but will be
for DiffOp, such as correctly handling the case where a deletion is
applied that is under parent directories which don't exist yet (the
correct behavior is, surprisingly, to create the parent directories as
that is what the image import/export code ends up doing).
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
Before this change, test cases were running with an env var that forces
the overlay differ to be on even when the native snapshotter was being
used, which resulted in failures. Now, that env var is skipped when
using the native snapshotter.
Additionally, this includes a related change to skip even trying to use
the overlay differ when the native snapshotter is in use. Previously,
the blob creation code first tried to use the overlay differ and then
failed and fell back to the double-walking differ. Now, it just jumps
right to the double-walking differ when the native snapshotter is in
use.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
Using an interface instead of a func is more flexible while achieving
the same effect. It allows you to succintly define a large number of
test cases as structs, as is common in table-driven testing.
A helper func is added that converts the existing test funcs into the
interface, so the change is fairly seamless.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
update fsutils to 61a57076b9b065af88eb10f699926d7e8793910c
which is required to pull in moby/moby#43047
Signed-off-by: Alex Couture-Beil <alex@earthly.dev>
switch to using newer MatchesUsingParentResults methods which were
introduced in https://github.com/moby/moby/pull/43037
Signed-off-by: Alex Couture-Beil <alex@earthly.dev>
When vendored into moby, the local exporter uses a user map that results
in all files being given at most 755 permissions. This change updates
the test to use permissions less than or equal to that to make tests
equivalent whether running w/ dockerd worker or any other type.
Additionally, add assertions that exported images also have the expected
contents, which helps ensures consistency between images created by
dockerd and those created by vanilla buildkit.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
Before this, if you try to get a ref with an equal blobchain in
GetByBlob but hit a missing provider, the error was just returned. While
we never expect this situation to happen (you shouldn't be able to hit
this line if you didn't already have providers for each blob in the
chain), it technically shouldn't fail the build as you can just continue
on without re-using the ref with equal blobchainID.
Now, we log this at error level but allow the build to continue.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
This fixes an issue where merge refs were incorrectly setting their
chain IDs to their last input's ID. This resulted in errors where
GetByBlob thought the merge ref and the final input ref were equivalent.
Now, merge refs have their chain IDs computed by digesting each blob in
the full chain.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
This change results in the overlay differ comparing files to determine
if they are actually part of the diff. This is needed to resolve
differences between the blobs created by the overlay differ and the
double-walking differ.
Before this change, the overlay differ always just assumed that if a
file was in the upperdir it must be part of the diff and included it as
an add or a modify change. However, there are situations in which files
can appear in the upperdir without having been modified or even opened.
For example, if "foo" is a file or dir present in the lowerdirs of an
overlay mount and you run "mv foo footmp; mv footmp foo", then the
upperdir will contain foo (in addition to any files found under foo if
it's a dir). In this situation, the double-walking differ would not
include foo as part of the diff, but the overlay differ would.
This meant that the overlay differ would potentially include extra files
in each blob for such diffs relative to the double-walking differ. As of
now, while this does increase image size, it doesn't result in any
inconsistencies in terms of the contents of images because it just
results in files/dirs getting duplicated on top of their equivalents.
However, for the upcoming DiffOp support, this inconsistency could
actually result in the same operation producing mounts with different
contents depending on which differ is used. This change is therefore
necessary in order to enforce DiffOp consistency (on top of the possible
improvements to exported image size).
The main concern here is that this could undo the performance benefits
that the overlay differ was intended to fix. However, in practice the
situations where this has worse performance are quite obscure and the
benefits should still be present.
First, consider the case where foo is a directory and the user does the
equivalent of "mv foo footmp; mv footmp foo". Even before this change,
the overlay differ would see that foo is marked as opaque and thus fall
back to using the double-walking differ. So there's no performance
regression in this case as the double-walking differ does the same
file comparisons as were added in this commit.
For the case where the user shuffles a file back and forth, there will
potentially be a slow file content based comparison if the underlying
file has a truncated nanosecond timestamp (i.e. it was unpacked from a
tar file). However, the situations in which you shuffle an individual
file without changing it (or open it for writing but then write nothing)
that is large enough in size for content comparisons to be slow are
obscure. Additionally, while the content comparison may be slow, there
will be time saved during export because the file won't be included
unnecessarily in the exported blob, so it's a tradeoff rather than a
pure loss.
In situations where the user actually did change a file and it shows up
in the upperdir, it should be extremely rare that the content comparison
code path is followed. It would require that the user changed no other
metadata of the file, including size, and both mod timestamps were the
same (which could only really happen if their underlying filesystem
lacked support for nanosecond precision and they modified the file
within 1 second of its modification in the lowerdir or they manually
changed the modtime with chtimes).
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
This updates containerd to v1.6.0-beta.3; done by running:
go get -u github.com/containerd/containerd
./hack/update-vendor
Signed-off-by: Alex Couture-Beil <alex@earthly.dev>
The "defer" is bound to the original value of the ticker, and won't stop
a ticker that's created later in the function. Example:
https://play.golang.org/p/puat5JEf5Jw
Ran into this in a health checker that periodically created buildkit
clients.
Signed-off-by: Aaron Lehmann <alehmann@netflix.com>
Before this change, if the path provided to FileOp.Rm was a symlink then
the target of the symlink would be removed instead of the symlink
itself. Now, the symlink will be removed instead. However, any symlinks
present in the parent dirs of the specified path will still be resolved
before calling os.Remove; this change only results in the base of the
specified path not being followed.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
Tõnis Tiigi
Aaron Lehmann
Shingo Omura
Erik Sipsma
Alex Couture-Beil
Akihiro Suda
Csaba Apagyi
Sebastiaan van Stijn
Kohei Tokunaga
Davis Schirmer
Anders F Björklund
sunchunming