[23.0 backport] libnetwork: clean up inDelete network atomically#45376
Merged
thaJeztah merged 1 commit intoApr 26, 2023
moby:23.0moby/moby:23.0from
thaJeztah:23.0_backport_delete_network_more_atomicallythaJeztah/docker:23.0_backport_delete_network_more_atomicallyCopy head branch name to clipboard
Merged
[23.0 backport] libnetwork: clean up inDelete network atomically#45376thaJeztah merged 1 commit intomoby:23.0moby/moby:23.0from thaJeztah:23.0_backport_delete_network_more_atomicallythaJeztah/docker:23.0_backport_delete_network_more_atomicallyCopy head branch name to clipboard
thaJeztah merged 1 commit into
moby:23.0moby/moby:23.0from
thaJeztah:23.0_backport_delete_network_more_atomicallythaJeztah/docker:23.0_backport_delete_network_more_atomicallyCopy head branch name to clipboard
Conversation
The (*network).ipamRelease function nils out the network's IPAM info fields, putting the network struct into an inconsistent state. The network-restore startup code panics if it tries to restore a network from a struct which has fewer IPAM config entries than IPAM info entries. Therefore (*network).delete contains a critical section: by persisting the network to the store after ipamRelease(), the datastore will contain an inconsistent network until the deletion operation completes and finishes deleting the network from the datastore. If for any reason the deletion operation is interrupted between ipamRelease() and deleteFromStore(), the daemon will crash on startup when it tries to restore the network. Updating the datastore after releasing the network's IPAM pools may have served a purpose in the past, when a global datastore was used for intra-cluster communication and the IPAM allocator had persistent global state, but nowadays there is no global datastore and the IPAM allocator has no persistent state whatsoever. Remove the vestigial datastore update as it is no longer necessary and only serves to cause problems. If the network deletion is interrupted before the network is deleted from the datastore, the deletion will resume during the next daemon startup, including releasing the IPAM pools. Signed-off-by: Cory Snider <csnider@mirantis.com> (cherry picked from commit c957ad0) Signed-off-by: Sebastiaan van Stijn <github@gone.nl>
corhere
approved these changes
Apr 24, 2023
vvoland
approved these changes
Apr 26, 2023
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The (*network).ipamRelease function nils out the network's IPAM info fields, putting the network struct into an inconsistent state. The network-restore startup code panics if it tries to restore a network from a struct which has fewer IPAM config entries than IPAM info entries. Therefore (*network).delete contains a critical section: by persisting the network to the store after ipamRelease(), the datastore will contain an inconsistent network until the deletion operation completes and finishes deleting the network from the datastore. If for any reason the deletion operation is interrupted between ipamRelease() and deleteFromStore(), the daemon will crash on startup when it tries to restore the network.
Updating the datastore after releasing the network's IPAM pools may have served a purpose in the past, when a global datastore was used for intra-cluster communication and the IPAM allocator had persistent global state, but nowadays there is no global datastore and the IPAM allocator has no persistent state whatsoever. Remove the vestigial datastore update as it is no longer necessary and only serves to cause problems. If the network deletion is interrupted before the network is deleted from the datastore, the deletion will resume during the next daemon startup, including releasing the IPAM pools.
(cherry picked from commit c957ad0)
- What I did
- How I did it
- How to verify it
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)