Repository navigation
Conversation
A recreated instance keeps its attachment identity, so after it moves node both nodes hold an advertisement for it until the old node's teardown withdraws its own. Each advertisement's reconcile wrote its own node into the attachment status, and every status write re-enqueued both, so the node flipped several times a second and every node router added and removed the attachment's routes on each flip. CNI ADD now stamps the advertisement with when it last published it, and the controller only projects status from the advertisement published most recently, falling back to creation time for older advertisements. Fixes #1385
Review fixes: when the newest advertisement is withdrawn while an older one remains, project from the survivor right away instead of leaving the attachment on a node with no advertisement. A stamped advertisement beats an unstamped one, so nodes not yet on the stamping CNI never win on their first creation time. Check currency only once an attachment is found.
…g by time Replaces the last-ADD timestamp ordering. A CNI ADD now marks every other node's advertisement for its attachment replaced by this node, and clears the mark on its own. The cloud controller never projects status from a replaced advertisement, and hands the attachment to the current one when a replaced advertisement wrote last or the current one is withdrawn. The old advertisement is marked rather than deleted: CNI DEL leaves it to the old node's GC, which reclaims the old host link through it.
scotwells
changed the title
Review fixes: a mark naming a node with no advertisement for the attachment any more no longer holds the old advertisement back, so a rolled-back or late ADD cannot leave the attachment without status. A failed mark logs instead of failing the ADD. The replaced advertisement's handover no longer needs its router, and withdrawal only re-projects when the attachment was on the withdrawn node. Documents the mark.
Review fix: letting a mark lapse once the replacing advertisement was gone reported delivery on the node the instance left, possibly a dead one, after the instance stopped. Marks now stay until their advertisement is collected, and a failed ADD that deletes its freshly created advertisement removes the marks it wrote, where they still name its node.
Replaces the replaced-by mark. CNI DEL now releases what its ADD published: it removes its container's entries from this node's BGPAdvertisement and deletes the advertisement once no container it records is alive, so a replacement whose ADD ran first keeps it. As a fallback for a predecessor whose DEL never ran, CNI ADD deletes the attachment's advertisements on other nodes, and galactic-cni's stale tap reclaim now removes a detached tap whose attachment is advertised only on another node, so the old node's tap does not outlive the deleted advertisement.
scotwells
changed the title
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
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.
Requested by Scot Schuchert-Wells · project thread
Fixes #1385. Related: datum-cloud/enhancements#921, datum-cloud/compute#630.
Problem
A recreated instance keeps its attachment identity. When it moves node, the old node's advertisement stayed until that node's GC collected it, because CNI DEL never deleted advertisements. Meanwhile the cloud controller wrote each advertisement's node into the attachment status in turn, so the status flipped between nodes several times a second, every node router added and removed the instance's routes on each flip, and its DNS lookups were denied for minutes.
Change
galactic-bgpremoves its container's entries from this node's advertisement and deletes the advertisement once no container it records is still alive. A replacement on the same node whose ADD ran first keeps it. Writes carry the resourceVersion, so a concurrent ADD is never overwritten.galactic-cni's stale tap reclaim now also removes a detached tap (no VMM holds it, seen on two passes) whose attachment is advertised only on another node, even though its VPCAttachment still exists. Before, it needed no VPCAttachment at all.Testing