Skip to content
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

Restore active mount counts on live-restore #45754

Merged
merged 1 commit into from Jun 27, 2023

Conversation

cpuguy83
Copy link
Member

@cpuguy83 cpuguy83 commented Jun 14, 2023

When live-restoring a container the volume driver needs be notified that there is an active mount for the volume.
Before this change the count is zero until the container stops and the uint64 overflows pretty much making it so the volume can never be removed until another daemon restart.

Fixes #44422

Copy link
Member

@thaJeztah thaJeztah left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@thaJeztah
Copy link
Member

docker-py failure can be ignored and is unrelated (fixed on master)

@thaJeztah
Copy link
Member

@corhere @neersighted @vvoland PTAL

Copy link
Contributor

@vvoland vvoland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, the logs need to be adjusted now though

daemon/mounts.go Outdated Show resolved Hide resolved
Copy link
Member

@neersighted neersighted left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One logging nit, otherwise LGTM.

daemon/mounts.go Outdated Show resolved Hide resolved
@cpuguy83 cpuguy83 force-pushed the fix_live_restore_local_vol_mounts branch from 19d651c to f1b6b87 Compare June 27, 2023 15:18
When live-restoring a container the volume driver needs be notified that
there is an active mount for the volume.
Before this change the count is zero until the container stops and the
uint64 overflows pretty much making it so the volume can never be
removed until another daemon restart.

Signed-off-by: Brian Goff <cpuguy83@gmail.com>
@cpuguy83 cpuguy83 force-pushed the fix_live_restore_local_vol_mounts branch from f1b6b87 to 647c2a6 Compare June 27, 2023 16:33
@thaJeztah
Copy link
Member

Looks like we should add a -f in the github actions as well;

sudo rm /etc/docker/daemon.json
  sudo service docker restart
  docker version
  docker info
  shell: /usr/bin/bash -e {0}
  env:
    DESTDIR: ./build
    BUILDKIT_REPO: moby/buildkit
    BUILDKIT_TEST_DISABLE_FEATURES: cache_backend_azblob,cache_backend_s3,merge_diff
    BUILDKIT_REF: 798ad6b0ce9f2fe86dfb2b0277e6770d0b545871
rm: cannot remove '/etc/docker/daemon.json': No such file or directory
Error: Process completed with exit code 1.

@thaJeztah thaJeztah merged commit a78c06e into moby:master Jun 27, 2023
102 checks passed
@solidgoldbomb
Copy link

Thanks for all of your work on this issue + fix @cpuguy83.

@cpuguy83 cpuguy83 deleted the fix_live_restore_local_vol_mounts branch June 28, 2023 02:03
@cpuguy83
Copy link
Member Author

Sorry it took so long to recognize where the issue was!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

live-restore in combination with docker compose is broken with docker-ce version 20.10.19 and newer
5 participants