Commit graph

375 commits

Author SHA1 Message Date
Paweł Gronowski
ad8a5a5732
c8d/list: Fix diffIDs being outputted instead of chainIDs
The `identity.ChainIDs` call was accidentally removed in
b37ced2551.

This broke the shared size calculation for images with more than one
layer that were sharing the same compressed layer.

This was could be reproduced with:
```
$ docker pull docker.io/docker/desktop-kubernetes-coredns:v1.11.1
$ docker pull docker.io/docker/desktop-kubernetes-etcd:3.5.10-0
$ docker system df
```

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-20 11:17:50 +01:00
Paweł Gronowski
0c2d83b5fb
c8d/list: Handle unpacked layers when calculating shared size
After a535a65c4b the size reported by the
image list was changed to include all platforms of that image.

This made the "shared size" calculation consider all diff ids of all the
platforms available in the image which caused "snapshot not found"
errors when multiple images were sharing the same layer which wasn't
unpacked.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-20 11:17:28 +01:00
Paweł Gronowski
731a64069f
c8d/list: Generate image summary concurrently
Run `imageSummary` concurrently to avoid being IO blocked on the
containerd gRPC.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-19 09:38:58 +01:00
Paweł Gronowski
dade279565
c8d/list: Add Images benchmark
Benchmark the `Images` implementation (image list) against an image
store with 10, 100 and 1000 random images. Currently the images are
single-platform only.

The images are generated randomly, but a fixed seed is used so the
actual testing data will be the same across different executions.

Because the content store is not a real containerd image store but a
local implementation, a small delay (500us) is added to each content
store method call. This is to simulate a real-world usage where each
containerd client call requires a gRPC call.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-19 09:38:56 +01:00
Sebastiaan van Stijn
dd146571ea
Merge pull request #47568 from vvoland/c8d-list-fix
c8d/list: Fix premature `Images` return
2024-03-18 15:28:09 +01:00
Paweł Gronowski
2e4ebf032a
c8d/list: Pass ctx to setupLabelFilter
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-18 13:50:45 +01:00
Paweł Gronowski
153de36b3f
c8d/list: Add empty index test case
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-18 12:50:10 +01:00
Paweł Gronowski
f512dba037
c8d/list: Fix premature Images return
52a80b40e2 extracted the `imageSummary`
function but introduced a bug causing the whole caller function to
return if the image should be skipped.

`imageSummary` returns a nil error and nil image when the image doesn't
have any platform or all its platforms are not available locally.
In this case that particular image should be skipped, instead of failing
the whole image list operation.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-18 10:43:12 +01:00
Paweł Gronowski
89dc2860ba
c8d/list: Handle missing configs in label filter
Don't error out the filter if an image config is missing.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-18 10:29:16 +01:00
Paweł Gronowski
6f3892dc99
c8d/list: Don't setup label filter if it's not specified
Don't run filter function which would only run through the images
reading theirs config without checking any label anyway.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-18 10:23:31 +01:00
Sebastiaan van Stijn
342923b01c
Merge pull request #47555 from rumpl/feat-c8d-prom
c8d: Prometheus metrics
2024-03-13 17:35:14 +01:00
Sebastiaan van Stijn
15122b3b1c
Merge pull request #47350 from vvoland/cache-refactor
c8d/cache: Use the same cache logic as graphdrivers
2024-03-13 17:19:36 +01:00
Djordje Lukic
388ecf65bc
c8d: Send push metrics to prom
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2024-03-13 15:03:42 +01:00
Djordje Lukic
bb3ab1edb7
c8d: Send pull metrics to prom
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2024-03-13 15:03:42 +01:00
Djordje Lukic
da245cab15
c8d: Send history metrics to prometheus
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2024-03-13 15:03:42 +01:00
Djordje Lukic
1cfd763214
c8d: Send image delete metrics to prometheus
Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2024-03-13 15:03:42 +01:00
Paweł Gronowski
2f1a32e3e5
c8d/list: Skip images with non matching platform
Currently this won't have any real effect because the platform matcher
matches all platform and is only used for sorting.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:27:12 +01:00
Paweł Gronowski
72f1f82f28
c8d/list: Remove outdated TODO
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:27:10 +01:00
Paweł Gronowski
52a80b40e2
c8d/list: Extract imageSummary function
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:27:09 +01:00
Paweł Gronowski
288a14e264
c8d/list: Simplify "best" image selection
Don't save all present images,  inline the sorting into the loop
instead.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:27:07 +01:00
Paweł Gronowski
b37ced2551
c8d/list: Count containers by their manifest
Move containers counting out of `singlePlatformImage` and count them
based on the `ImageManifest` property.

(also remove ChainIDs calculation as they're no longer used)

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:26:53 +01:00
Paweł Gronowski
a535a65c4b
c8d/list: Combine size
Multi-platform images are coalesced into one entry now.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:25:32 +01:00
Paweł Gronowski
582de4bc3c
c8d/list: Add TestImageList
Add unit test for `Images` implementation.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:25:31 +01:00
Paweł Gronowski
1b108bdfeb
daemon/c8d: Cache SnapshotService
Avoid fetching `SnapshotService` from client every time. Fetch it once
and then store when creating the image service.

This also allows to pass custom snapshotter implementation for unit
testing.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:25:29 +01:00
Paweł Gronowski
74e2f23e1a
daemon/c8d: Use i.images and i.content
Use `image.Store` and `content.Store` stored in the ImageService struct
instead of fetching it every time from containerd client.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 16:25:27 +01:00
Paweł Gronowski
e8496b1ee4
imageService: Extract common code from MakeImageCache
Both containerd and graphdriver image service use the same code to
create the cache - they only supply their own `cacheAdaptor` struct.

Extract the shared code to `cache.New`.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 15:39:42 +01:00
Paweł Gronowski
d66177591e
c8d/cache: Use the same cache logic as graphdrivers
Implement the cache adaptor for containerd image store and use the same
cache logic.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-03-07 14:58:06 +01:00
Sebastiaan van Stijn
62b33a2604
disable pulling legacy image formats by default
This patch disables pulling legacy (schema1 and schema 2, version 1) images by
default.

A `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE` environment-variable is
introduced to allow re-enabling this feature, aligning with the environment
variable used in containerd 2.0 (`CONTAINERD_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE`).

With this patch, attempts to pull a legacy image produces an error:

With graphdrivers:

    docker pull docker:1.0
    1.0: Pulling from library/docker
    [DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/

With the containerd image store enabled, output is slightly different
as it returns the error before printing the `1.0: pulling ...`:

    docker pull docker:1.0
    Error response from daemon: [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/

Using the "distribution" endpoint to resolve the digest for an image also
produces an error:

    curl -v --unix-socket /var/run/docker.sock http://foo/distribution/docker.io/library/docker:1.0/json
    *   Trying /var/run/docker.sock:0...
    * Connected to foo (/var/run/docker.sock) port 80 (#0)
    > GET /distribution/docker.io/library/docker:1.0/json HTTP/1.1
    > Host: foo
    > User-Agent: curl/7.88.1
    > Accept: */*
    >
    < HTTP/1.1 400 Bad Request
    < Api-Version: 1.45
    < Content-Type: application/json
    < Docker-Experimental: false
    < Ostype: linux
    < Server: Docker/dev (linux)
    < Date: Tue, 27 Feb 2024 16:09:42 GMT
    < Content-Length: 354
    <
    {"message":"[DEPRECATION NOTICE] Docker Image Format v1, and Docker Image manifest version 2, schema 1 support will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format, or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/"}
    * Connection #0 to host foo left intact

Starting the daemon with the `DOCKER_ENABLE_DEPRECATED_PULL_SCHEMA_1_IMAGE`
env-var set to a non-empty value allows pulling the image;

    docker pull docker:1.0
    [DEPRECATION NOTICE] Docker Image Format v1 and Docker Image manifest version 2, schema 1 support is disabled by default and will be removed in an upcoming release. Suggest the author of docker.io/library/docker:1.0 to upgrade the image to the OCI Format or Docker Image manifest v2, schema 2. More information at https://docs.docker.com/go/deprecated-image-specs/
    b0a0e6710d13: Already exists
    d193ad713811: Already exists
    ba7268c3149b: Already exists
    c862d82a67a2: Already exists
    Digest: sha256:5e7081837926c7a40e58881bbebc52044a95a62a2ea52fb240db3fc539212fe5
    Status: Image is up to date for docker:1.0
    docker.io/library/docker:1.0

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-02-28 13:57:43 +01:00
Paweł Gronowski
16aa7dd67f
c8d/pull: Output truncated id for Pulling fs layer
All other progress updates are emitted with truncated id.

```diff
$ docker pull --platform linux/amd64 alpine
Using default tag: latest
latest: Pulling from library/alpine
-sha256:4abcf20661432fb2d719aaf90656f55c287f8ca915dc1c92ec14ff61e67fbaf8: Pulling fs layer
+4abcf2066143: Download complete
Digest: sha256:c5b1261d6d3e43071626931fc004f70149baeba2c8ec672bd4f27761f8e1ad6b
Status: Image is up to date for alpine:latest
docker.io/library/alpine:latest
```

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-02-27 11:09:16 +01:00
Sebastiaan van Stijn
ffd294ebcc
Merge pull request #45967 from tianon/c8d-image-list
c8d: Adjust "image list" to return only a single item for each image store entry
2024-02-26 20:05:29 +01:00
Paweł Gronowski
14df52b709
c8d/pull: Don't emit Downloading with 0 progress
To align with the graphdrivers behavior and don't send unnecessary
progress messages.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-02-22 18:03:16 +01:00
Paweł Gronowski
ff5f780f2b
c8d/pull: Emit Pulling fs layer
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-02-22 18:03:15 +01:00
Paweł Gronowski
bddd892e91
c8d: Adjust "image list" to return only a single item for each image store entry
This will return a single entry for each name/value pair, and for now
all the "image specific" metadata (labels, config, size) should be
either "default platform" or "first platform we have locally" (which
then matches the logic for commands like `docker image inspect`, etc)
with everything else (just ID, maybe?) coming from the manifest
list/index.

That leaves room for the longer-term implementation to add new fields to
describe the _other_ images that are part of the manifest list/index.

Co-authored-by: Tianon Gravi <admwiggin@gmail.com>

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-02-14 18:44:37 +01:00
Sebastiaan van Stijn
03a17a2887
migrate image spec to github.com/moby/docker-image-spec
The specification was migrated to a separate module:
https://github.com/moby/docker-image-spec

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-02-09 19:12:18 +01:00
Djordje Lukic
f1e6958295
c8d: Use the same logic to get the present images
Inspect and history used two different ways to find the present images.
This made history fail in some cases where image inspect would work (if
a configuration of a manifest wasn't found in the content store).

With this change we now use the same logic for both inspect and history.

Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
2024-02-06 16:35:53 +01:00
Sebastiaan van Stijn
2156635843
Merge pull request #47232 from vvoland/fix-save-manifests
image/save: Fix untagged images not present in index.json
2024-02-05 19:06:54 +01:00
Paweł Gronowski
5e13f54f57
c8d/save: Handle digested reference same as ID
When saving an image treat `image@sha256:abcdef...` the same as
`abcdef...`, this makes it:

- Not export the digested tag as the image name
- Not try to export all tags from the image repository

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-29 16:29:05 +01:00
Paweł Gronowski
877ebbe038
image/cache: Check image platform
Make sure the cache candidate platform matches the requested.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-25 16:04:05 +01:00
Paweł Gronowski
537348763f
image/cache: Compare all config fields
Add checks for some image config fields that were missing.

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-25 16:03:58 +01:00
Sebastiaan van Stijn
ac2a028dcc
api/types: move image options to api/types/image
To prevent a circular import between api/types and api/types image,
the RequestPrivilegeFunc reference was not moved, but defined as
part of the PullOptions / PushOptions.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-01-24 00:10:33 +01:00
Sebastiaan van Stijn
8906adc8d4
Merge pull request #47138 from thaJeztah/move_image_backend_opt
api/types/image: move GetImageOpts to api/types/backend
2024-01-23 23:41:38 +01:00
Sebastiaan van Stijn
a3a42c459e
api/types/image: move GetImageOpts to api/types/backend
The `GetImageOpts` struct is used for options to be passed to the backend,
and are not used in client code. This struct currently is intended for internal
use only.

This patch moves the `GetImageOpts` struct to the backend package to prevent
it being imported in the client, and to make it more clear that this is part
of internal APIs, and not public-facing.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-01-22 20:45:21 +01:00
Paweł Gronowski
fb19f1fc20
c8d/snapshot: Create any platform if not specified
With containerd snapshotters enabled `docker run` currently fails when
creating a container from an image that doesn't have the default host
platform without an explicit `--platform` selection:

```
$ docker run image:amd64
Unable to find image 'asdf:amd64' locally
docker: Error response from daemon: pull access denied for asdf, repository does not exist or may require 'docker login'.
See 'docker run --help'.
```

This is confusing and the graphdriver behavior is much better here,
because it runs whatever platform the image has, but prints a warning:

```
$ docker run image:amd64
WARNING: The requested image's platform (linux/amd64) does not match the detected host platform (linux/arm64/v8) and no specific platform was requested
```

This commits changes the containerd snapshotter behavior to be the same
as the graphdriver. This doesn't affect container creation when platform
is specified explicitly.

```
$ docker run --rm --platform linux/arm64 asdf:amd64
Unable to find image 'asdf:amd64' locally
docker: Error response from daemon: pull access denied for asdf, repository does not exist or may require 'docker login'.
See 'docker run --help'.
```

Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-22 16:15:07 +01:00
Sebastiaan van Stijn
94b4765363
pkg/platforms: internalize in daemon/containerd
This matcher was only used internally in the containerd implementation of
the image store. Un-export it, and make it a local utility in that package
to prevent external use.

This package was introduced in 1616a09b61
(v24.0), and there are no known external consumers of this package, so there
should be no need to deprecate / alias the old location.

Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-01-20 22:28:56 +01:00
Sebastiaan van Stijn
5581efe7cd
rename "ociimage" var to be proper camelCase
Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
2024-01-19 12:50:13 +01:00
Paul "TBBle" Hampson
66325f7271
Implement GetLayerFolders for the containerd image store
The existing API ImageService.GetLayerFolders didn't have access to the
ID of the container, and once we have that, the snapshotter Mounts API
provides all the information we need here.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-17 16:29:28 +01:00
Paul "TBBle" Hampson
0dc07ccc3a
Unmount RWLayer during Commit
Needed for Diff on Windows. Don't remount it afterwards as the layer is
going to be released anyway.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-17 16:29:27 +01:00
Paul "TBBle" Hampson
a1f6b64e24
Don't call setupInit in a temp mount if setupInit is nil
This is consistent with layerStore's CreateRWLayer behaviour.

Potentially this can be refactored to avoid creating the -init layer,
but as noted in layerStore's initMount, this name may be special, and
should be cleared-out all-at-once.

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-17 16:29:25 +01:00
Paul "TBBle" Hampson
641050c93f
Typo fixes
* conatinerd => containerd
* ROLayer => RWLayer

Signed-off-by: Paul "TBBle" Hampson <Paul.Hampson@Pobox.com>
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-17 16:29:22 +01:00
Paweł Gronowski
8390bc5683
c8d/cache: Use ContainerConfig from content store
Signed-off-by: Paweł Gronowski <pawel.gronowski@docker.com>
2024-01-17 14:57:55 +01:00