Clean up inDelete network atomically #2677
Merged
Conversation
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
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 <[email protected]> (cherry picked from commit moby/moby@c957ad0) Signed-off-by: Cory Snider <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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).ipamReleasefunction 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).deletecontains a critical section: by persisting the network to the store afteripamRelease(), 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 betweenipamRelease()anddeleteFromStore(), 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 moby/moby@c957ad0)