-
Notifications
You must be signed in to change notification settings - Fork 653
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Suggested clarification to whiteouts #1215
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly LGTM, one minor wording nit 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My reading of the opaque white out is different:
An opaque whiteout entry is a file with the name
.wh..wh..opq
indicating that all siblings are hidden in the lower layer.
Which means that a
itself is not deleted, only the contents of the directory. I verified that with the following example:
$ regctl manifest get sudobmitch/demo:opaque-wh
Name: sudobmitch/demo:opaque-wh
MediaType: application/vnd.docker.distribution.manifest.v2+json
Digest: sha256:9310089ee7088013005a78fdf549aac248c873c37c30559de2ee687710ca5b6c
Total Size: 2.153MB
Config:
Digest: sha256:ef5cf31c087d8b2178e189770e123dfbd104d03dc6be72a44a49e0efa708137c
MediaType: application/vnd.docker.container.image.v1+json
Size: 841B
Layers:
Digest: sha256:ec562eabd705d25bfea8c8d79e4610775e375524af00552fe871d3338261563c
MediaType: application/vnd.docker.image.rootfs.diff.tar.gzip
Size: 2.153MB
Digest: sha256:2d6473f09615f3446a3dd09cdf3d554e8080c43e8946e007c543df8feb759ee9
MediaType: application/vnd.docker.image.rootfs.diff.tar.gzip
Size: 222B
Digest: sha256:77deb7c636039bbea6511a6de67dc0fb549ba3ee9c0004bd82c986ccf4773dd9
MediaType: application/vnd.docker.image.rootfs.diff.tar.gzip
Size: 121B
$ regctl blob get sudobmitch/demo:opaque-wh sha256:2d6473f09615f3446a3dd09cdf3d554e8080c43e8946e007c543df8feb759ee9 | tar -tvzf -
drwxr-xr-x 0/0 0 2024-11-01 10:39 etc/
drwxr-xr-x 0/0 0 2024-11-01 10:39 proc/
drwxr-xr-x 0/0 0 2024-11-01 10:39 sys/
drwxr-xr-x 0/0 0 2024-11-01 10:39 usr/
drwxr-xr-x 0/0 0 2024-11-01 10:39 usr/local/
drwxr-xr-x 0/0 0 2024-11-01 10:39 usr/local/bin/
drwxr-xr-x 0/0 0 2024-11-01 10:39 usr/local/tmp/
-rw-r--r-- 0/0 12 2024-11-01 10:39 usr/local/tmp/hello.txt
$ regctl blob get sudobmitch/demo:opaque-wh sha256:77deb7c636039bbea6511a6de67dc0fb549ba3ee9c0004bd82c986ccf4773dd9 | tar -tvzf -
-rw-r--r-- 0/0 0 2024-11-01 10:42 usr/local/.wh..wh..opq
$ docker run -it --rm sudobmitch/demo:opaque-wh ls -al /usr/local
total 8
drwxr-xr-x 2 root root 4096 Nov 1 14:50 .
drwxr-xr-x 1 root root 4096 Nov 1 14:50 ..
layer.md
Outdated
a/b/c/bar | ||
``` | ||
|
||
When the next layer is created, the original `a/` directory is deleted and recreated with `a/b/c/foo`: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The a
directory itself is not deleted, only all entries in the a
directory, if my reading is correct. To make it more clear, we could also add an a/baz
that also gets deleted.
When the next layer is created, the original `a/` directory is deleted and recreated with `a/b/c/foo`: | |
When the next layer is created, the original `a/b` directory is deleted and recreated with `a/b/c/foo`: |
layer.md
Outdated
a/b/c/foo | ||
``` | ||
|
||
When processing the second layer, `a/.wh..wh..opq` is applied first, before creating the new version of `a/`, regardless of the ordering in which the whiteout file was encountered. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When processing the second layer, `a/.wh..wh..opq` is applied first, before creating the new version of `a/`, regardless of the ordering in which the whiteout file was encountered. | |
When processing the second layer, `a/.wh..wh..opq` is applied first, before creating the new version of `a/b`, regardless of the ordering in which the whiteout file was encountered. |
I tested the example from the spec (including I'm more of a podman user but tested with docker too just to see (I manually trimmed out a deprecated warning from docker for wanting to switch to buildkit). Podman works the same but it includes extra stuff like The layers from 1 and 2 both represent equivalent end states given their shared base layer. But, in isolation as a changeset, they mean different things. If we imagine taking just those change layers and applying them in turn onto some other base layer that has a directory And I just realized that these two lines
are problematic because using explicit whiteouts on every directory entry is not equivalent to an opaque dir. test scriptset -e
engine="${1:-podman}"
function get_layers() {
image_manifest=$(jq -r '.manifests[0].digest' index.json | sed 's_:_/_')
jq -r '.layers[].digest' blobs/$image_manifest | sed 's_:_/_'
}
function show_layers() {
rm -rf oci-dir && mkdir oci-dir
pushd . &>/dev/null
if [ "$engine" = "podman" ]; then
podman save --format oci-archive $1 | tar xf - -C oci-dir
else
$engine save $1 | tar xf - -C oci-dir
fi
cd oci-dir
for layer in $(get_layers | tail -n+2); do
echo $layer
tar tvf blobs/$layer
echo
done
popd &>/dev/null
}
echo "using engine $engine"
mkdir -p /tmp/question
cd /tmp/question
name=opaque-dir
cat << "EOF" > Containerfile.1
FROM busybox
RUN mkdir -p a/b/c/ && touch a/b/c/bar a/baz
RUN rm -rf a/* && mkdir -p a/b/c && touch a/b/c/foo
EOF
cat << "EOF" > Containerfile.2
FROM busybox
RUN mkdir -p a/b/c/ && touch a/b/c/bar a/baz
RUN rm -rf a/ && mkdir -p a/b/c && touch a/b/c/foo
EOF
cat << "EOF" > Containerfile.3
FROM busybox
RUN mkdir -p a/b/c/ && touch a/b/c/bar a/baz
RUN rm -rf a/*
EOF
cat << "EOF" > Containerfile.4
FROM busybox
RUN mkdir -p a/b/c/ && touch a/b/c/bar a/baz
RUN rm -rf a/*
RUN mkdir -p a/b/c && touch a/b/c/foo
EOF
for containerfile in Containerfile.{1,2,3,4}; do
echo "# $containerfile"
cat $containerfile
$engine rmi $name >/dev/null || true
if [ "$engine" = "podman" ]; then
$engine build --dns=none --no-hosts --no-hostname -f $containerfile -t $name >/dev/null
else
$engine build -f $containerfile -t $name . >/dev/null
fi
show_layers $name
echo -e "---------------------------------\n"
done
|
They are the same if you know all of the entries in the directory (which build tools often know). They are different if you either change the definition of opaque whiteouts to include the parent directory, or if layers are being rebased (possible, but not common). How does podman handle: |
I'm a bit confused by your example in how it relates to the example from the spec. Can you show the Containerfile? |
That shows podman and containerd are consistent and the existing spec should not be changed. It proves that The blob was generated by hand because my attempt to create it with buildkit resulted in individual whiteout files as recommended by the spec. |
Co-authored-by: Brandon Mitchell <[email protected]> Co-authored-by: Tianon Gravi <[email protected]> Signed-off-by: Andrew Consroe <[email protected]>
Okay I think we've reached an impasse, I've backed out the change of |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
FWIW, if you wanted to delete |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (although I think it's amusing that in the moved example, the text says we deleted a/b
but the actual example would be created by deleting and recreating a/
entirely, which I guess is what Brandon was getting at with his comment about .wh.b
)
I found it confusing that the first example given for the whiteouts section was only about opaque whiteouts even though that gets its own section. So I added a simple example with just a file and directory being deleted.
The second commit fixes what I believe is a typo in the opaque whiteout example, where it says that
a/b
is deleted, but I think it isa/
that is deleted.