containerd-integration: "failed to export layer: snapshot already exists" for concurrent docker commit

[HollowMan6]

Description

With beta feature Use containerd for pulling and storing images enabled in docker-desktop, when two docker commit are executed at the same time and the two container images are of the same origin, error will occur:

Error response from daemon: failed to export layer: snapshot "sha256:8b811a30cb94c227fb2ae61a2a1ec1e93381dbef06f9ea6b5c06df4f27651fed-parent-view": already exists

Which is OK if Use containerd for pulling and storing images is disabled. So I would say it's a bug.

Reproduce

1. docker image pull nginx
2. docker create --name nginx-image-1-tmp nginx
3. docker create --name nginx-image-2-tmp nginx
4. docker commit nginx-image-1-tmp nginx-image-1 &
   docker commit nginx-image-2-tmp nginx-image-2 &

Expected behavior

When Use containerd for pulling and storing images is enabled, even two docker commit are executed at the same time and the two container images are of the same origin, it should still work.

docker version

Client: Docker Engine - Community
 Cloud integration: v1.0.29
 Version:           20.10.21
 API version:       1.41
 Go version:        go1.18.7
 Git commit:        baeda1f
 Built:             Tue Oct 25 18:02:14 2022
 OS/Arch:           linux/amd64
 Context:           desktop-linux
 Experimental:      true

Server: Docker Desktop 4.14.0 (91374)
 Engine:
  Version:          22.06.0-beta.0-407-g7a5cce156e.m
  API version:      1.43 (minimum version 1.12)
  Go version:       go1.18.4
  Git commit:       7a5cce156e
  Built:            Thu Oct 13 08:07:59 2022
  OS/Arch:          linux/amd64
  Experimental:     false
 containerd:
  Version:          1.6.9
  GitCommit:        1c90a442489720eec95342e1789ee8a5e1b9536f
 runc:
  Version:          1.1.4
  GitCommit:        v1.1.4-0-g5fd4c4d
 docker-init:
  Version:          0.19.0
  GitCommit:        de40ad0

docker info

Client:
 Context:    desktop-linux
 Debug Mode: false
 Plugins:
  app: Docker App (Docker Inc., v0.9.1-beta3)
  buildx: Docker Buildx (Docker Inc., v0.9.1-docker)
  compose: Docker Compose (Docker Inc., v2.12.2)
  dev: Docker Dev Environments (Docker Inc., v0.0.3)
  extension: Manages Docker extensions (Docker Inc., v0.2.13)
  sbom: View the packaged-based Software Bill Of Materials (SBOM) for an image (Anchore Inc., 0.6.0)
  scan: Docker Scan (Docker Inc., v0.21.0)

Server:
 Containers: 2
  Running: 0
  Paused: 0
  Stopped: 2
 Images: 3
 Server Version: 22.06.0-beta.0-407-g7a5cce156e.m
 Storage Driver: stargz
  driver-type: io.containerd.snapshotter.v1
 Logging Driver: json-file
 Cgroup Driver: cgroupfs
 Cgroup Version: 2
 Plugins:
  Volume: local
  Network: bridge host ipvlan macvlan null overlay
  Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog
 Swarm: inactive
 Runtimes: runc io.containerd.runc.v2
 Default Runtime: runc
 Init Binary: docker-init
 containerd version: 1c90a442489720eec95342e1789ee8a5e1b9536f
 runc version: v1.1.4-0-g5fd4c4d
 init version: de40ad0
 Security Options:
  seccomp
   Profile: builtin
  cgroupns
 Kernel Version: 5.15.49-linuxkit
 Operating System: Docker Desktop
 OSType: linux
 Architecture: x86_64
 CPUs: 10
 Total Memory: 3.675GiB
 Name: docker-desktop
 ID: ef8e10f2-c626-45a1-8e6b-3d80aa37cc30
 Docker Root Dir: /var/lib/docker
 Debug Mode: false
 HTTP Proxy: http.docker.internal:3128
 HTTPS Proxy: http.docker.internal:3128
 No Proxy: hubproxy.docker.internal
 Registry: https://index.docker.io/v1/
 Labels:
 Experimental: false
 Insecure Registries:
  hubproxy.docker.internal:5000
  127.0.0.0/8
 Live Restore Enabled: false

Additional Info

Maybe it's a containerd issue. Two containers can have images of the exact same origin, so it shoud be OK if same layer of snapshot exists. So maybe I guess the following line and something alike should change into a warning instead of an error:

moby/vendor/github.com/containerd/containerd/metadata/snapshot.go

Line 435 in 18f8ab6

rerr = fmt.Errorf("snapshot %q: %w", key, errdefs.ErrAlreadyExists)

If that fix would cause issues, then maybe you should implement transaction lock, and waiting for a lock to get released and try again when snapshot already exists.

[vvoland]

It's indeed a bug in containerd. It's been already fixed though, but is not a part of the 1.6 release we use.
I opened a PR to backport this fix: containerd/containerd#7756

[HollowMan6]

Cool! Closing this issue.

[vvoland]

This still needs a containerd revendor on our side once new 1.6 version is released. I'll keep it on my TODO list 😄
Thanks a lot for the report!