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>
This consists of just the base MergeOp with support for merging LLB
results that include deletions using hardlinks as the efficient path
and copies as fallback.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
This is mostly just preparation for merge-op. The existing
Extract method is updated to be usable for unlazying any type of refs
rather than just lazy blobs. The way views are created is simplified and
centralized in one location.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
The Parent method will no longer make sense with forthcoming Merge and
Diff support as refs will become capable of having multiple parents. It
was also only ever used externally to get the full chain of refs for
each layer in the ref's chain.
The newly added LayerChain method replaces Parents with a method that
just returns a slice of refs for each layer in the ref's chain. This
will work more seamlessly with Merge and Diff (in which case it returns
the "flattened" ancestors of the ref) in addition to being a bit easier
to use for the exiting cases anyways.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
Before this, descriptor handlers were not included with calls to the
exporter, which then sometimes called LoadRef and failed to get a ref
because it was lazy. This change results in the DescHandlers of the
already loaded refs to get plugged into context so they can be re-used
by the exporter if it needs to load the ref again.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
Alex Couture-Beil
Tõnis Tiigi
Erik Sipsma
Akihiro Suda
Csaba Apagyi
Sebastiaan van Stijn
Kohei Tokunaga
Davis Schirmer
Anders F Björklund
sunchunming
Aaron Lehmann