Skip to content

Restore active mount counts on live-restore#45754

Merged
thaJeztah merged 1 commit intomoby:masterfrom
cpuguy83:fix_live_restore_local_vol_mounts
Jun 27, 2023
Merged

Restore active mount counts on live-restore#45754
thaJeztah merged 1 commit intomoby:masterfrom
cpuguy83:fix_live_restore_local_vol_mounts

Conversation

@cpuguy83
Copy link
Copy Markdown
Member

@cpuguy83 cpuguy83 commented Jun 14, 2023 •
edited
Loading

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

@cpuguy83 cpuguy83 mentioned this pull request Jun 14, 2023
Closed
@cpuguy83 cpuguy83 force-pushed the fix_live_restore_local_vol_mounts branch from e0e26f7 to 6781308 Compare June 17, 2023 20:30
@cpuguy83 cpuguy83 marked this pull request as ready for review June 17, 2023 20:35
@cpuguy83 cpuguy83 changed the title WIP: Restore active mount counts on live-restore Restore active mount counts on live-restore Jun 17, 2023
@cpuguy83 cpuguy83 force-pushed the fix_live_restore_local_vol_mounts branch 2 times, most recently from 3c57ca9 to 19d651c Compare June 17, 2023 21:11
@thaJeztah thaJeztah added this to the 25.0.0 milestone Jun 22, 2023
thaJeztah
thaJeztah approved these changes Jun 22, 2023
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member

thaJeztah commented Jun 22, 2023

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

@thaJeztah
Copy link
Copy Markdown
Member

thaJeztah commented Jun 27, 2023

@corhere @neersighted @vvoland PTAL

@thaJeztah thaJeztah mentioned this pull request Jun 27, 2023
Merged
This was referenced Jun 27, 2023
Merged
Closed
vvoland
vvoland reviewed Jun 27, 2023
View reviewed changes
Copy link
Copy Markdown
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

neersighted
neersighted requested changes Jun 27, 2023
View reviewed changes
Copy link
Copy Markdown
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.

@cpuguy83 cpuguy83 force-pushed the fix_live_restore_local_vol_mounts branch from 19d651c to f1b6b87 Compare June 27, 2023 15:18
@cpuguy83
Restore active mount counts on live-restore
647c2a6 
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
Copy Markdown
Member

thaJeztah commented Jun 27, 2023

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
@solidgoldbomb
Copy link
Copy Markdown

solidgoldbomb commented Jun 28, 2023

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
Copy Markdown
Member Author

cpuguy83 commented Jun 28, 2023

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

@thaJeztah thaJeztah mentioned this pull request Jun 28, 2023
Merged
This was referenced Jul 7, 2023
Closed
Merged
Merged
@corhere corhere mentioned this pull request Jul 7, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@thaJeztah thaJeztah thaJeztah approved these changes

@neersighted neersighted neersighted approved these changes

@vvoland vvoland vvoland approved these changes

Assignees

No one assigned

Labels

area/volumes Volumes kind/bugfix PR's that fix bugs process/cherry-picked

Projects

None yet

Milestone

25.0.0

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

@cpuguy83 @thaJeztah @solidgoldbomb @neersighted @vvoland