Before this, there was a bug triggered under the following conditions:
1. An overlay snapshotter was being used, which caused the optimization
of preparing a new snapshot off of the base layers to be triggered
2. The base layers contained a directory that had contents
3. One subsequent layer deleted that directory w/out recreating it
4. A later layer recreated the directory
In this case, what happened was a whiteout device would be created as
part of 3 above but then in step 4 the whiteout device would be removed
and replaced with a plain directory. The problem is that such a
directory doesn't block out the files from step 2 and it doesn't know
about them because they are in a lowerdir (not the upperdir being
applied to).
The simplest fix, which this commit implements, is to just set the
directory created in step 4 as opaque, which enables the correct
behavior of blocking out files below it.
This was missed in test coverage before because tests for opaque
handling always combined 3+4 into one layer, whereas the bug requires
they be separate layers. A new integration test has been added to cover
this case.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
This allows you to create refs that are single layers representing the
diff between any two arbitrary refs. The primary use case for this is
to allows users to extract the changes created by ops like Exec and
rebase them elsewhere through MergeOp. However, there is no restriction
on the inputs to DiffOp and the resulting ref's layer is simply the
layer created by running the differ on the two inputs refs
(specifically, the same differ used during exports).
A Diff ref can be mounted by itself, in which case it is defined as the
result of applying the diff to Scratch. Most use cases though will use
Diff refs as the input to a MergeOp, in which case the diff is just
applied on top of the lower merge inputs, as was the case before.
In cases like Diff(A, A->B->C) (i.e. cases where the diff is between two
refs where the lower is an ancestor of upper), the diff will be defined
as the layers separating the two refs. In other cases, the diff is just
a single layer, not re-used from the inputs, representing the diff
between the two refs (which can be defined as the layer "Diff(A,B)" that
satisfies "Merge(A, Diff(A,B)) == B").
Note that there is technically a meaningful difference between the
"unmerge" behavior of extracting the layers separating diffs and the
"simple diff" of just running the differ on the two refs. Namely, in the
case where there are "intermediate deletes" (i.e. deletes that only
exist in layers between A and B but not between A and B by themselves),
then the simple diff and unmerge can create different results when
plugged into a MergeOp. This is due to the fact that intermediate
deletes will apply to the merge when using the unmerge behavior, but not
when using the simple diff. This is on top of the fact that the simple
diff inherently has a "flattening" behavior where multiple layers are
squashed into a single one.
So, in the case where lower is an ancestor of upper, we choose to follow
the unmerge behavior, but it's possible users may prefer the simple diff
behavior. As of right now, they won't be able to do so, but if needed we
can add the ability to choose which behavior is followed in the future.
This could be done through a flag provided to DiffOp or possibly by
adapting llb.Copy to support this type of behavior with the same
efficiency as DiffOp.
Signed-off-by: Erik Sipsma <erik@sipsma.dev>
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>
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>
Due to current deficiencies in the Windows implementation of
containerd's Mount.Mount, we need different behaviour for mounting on
Windows and Unix platforms.
Specifically:
* Windows mounts can only mount in-place, and hence only one mount
should be in the list.
* BuildKit doesn't own the mount directory, so should not try and remove
it on unmount.
Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
```
[5/5] RUN --mount=target=/go/src/github.com/moby/buildkit gometalinter ...
0.435 util/rootless/specconv/specconv_linux.go:1:⚠️ file is not goimported (goimports)
1.320 cache/manager.go:1:⚠️ file is not goimported (goimports)
1.335 cache/manager_test.go:1:⚠️ file is not goimported (goimports)
1.337 cache/migrate_v2.go:1:⚠️ file is not goimported (goimports)
1.342 cache/refs.go:1:⚠️ file is not goimported (goimports)
1.454 cache/remotecache/registry/registry.go:1:⚠️ file is not goimported (goimports)
2.285 cmd/buildctl/build.go:1:⚠️ file is not goimported (goimports)
3.082 executor/oci/user.go:1:⚠️ file is not goimported (goimports)
4.333 session/content/content_test.go:1:⚠️ file is not goimported (goimports)
4.614 snapshot/containerd/content.go:1:⚠️ file is not goimported (goimports)
4.721 solver/errdefs/vertex.go:1:⚠️ file is not goimported (goimports)
6.066 util/network/cniprovider/cni.go:1:⚠️ file is not goimported (goimports)
ERROR: executor failed running [/bin/sh -c gometalinter --config=gometalinter.json ./...]: buildkit-runc did not terminate successfully
```
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
Refactor the interface to avoid such issues in the future.
BuildKit own mounts are stateless and not affected but
a different mountable implementation could get confused.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
It is enhancement which allows to unpack image into containerd
snapshotter storage by `--output type=image,<.>=<.>,unpack=true`.
In order to support this feature, we needs to extend the Snapshotter
witwh `Name() string` function. Because we needs to set gc label for
snapshotter which need snapshotter name.
fix: #908
Signed-off-by: Wei Fu <fuweid89@gmail.com>
Replaces previous mutable.Freeze logic with
commits that can live together with mutable data.
Finalize method is added if the implementation
needs to make sure that the immutable ref is
flushed to the driver. Refs are automaitcally
finalized when writable layers are created on
top of them.
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
Erik Sipsma
Kohei Tokunaga
CrazyMax
Koichi Shiraishi
Tonis Tiigi
Paul "TBBle" Hampson
Sebastiaan van Stijn
Akihiro Suda
Wei Fu
Derek McGowan
Iskander (Alex) Sharipov
John Howard
Akihiro Suda