Fix live restore for IPv6-only and multiple gateway endpoints#49150
Merged
robmry merged 5 commits intomoby:masterfrom Jan 6, 2025
Merged
Fix live restore for IPv6-only and multiple gateway endpoints#49150robmry merged 5 commits intomoby:masterfrom
robmry merged 5 commits intomoby:masterfrom
Conversation
robmry
force-pushed
the
live_restore_fixes
branch
from
1f9eeee to
fb0b8b0
Compare
thaJeztah
approved these changes
Jan 6, 2025
| }) | ||
| assert.Check(t, defFound, "default route %q not found:\n%s", expDefRoute, strings.Join(routes, "\n")) | ||
| if expDefRoute == "" { | ||
| defFound := slices.ContainsFunc(routes, func(s string) bool { |
Member
There was a problem hiding this comment.
Looks like it didn't complain before this, but probably because it's a _test.go - wondering if we should technically have such //go:build tags for these as well
(no need to do in this PR; looks like it was already there)
akerouanton
approved these changes
Jan 6, 2025
Member
akerouanton
left a comment
akerouanton
left a comment
There was a problem hiding this comment.
Just one small nit, otherwise LGTM.
| id: sbs.ID, | ||
| controller: sbs.c, | ||
| containerID: sbs.Cid, | ||
| epPriority: sbs.EpPriority, |
Member
There was a problem hiding this comment.
It seems this method is badly named -- it does more than cleaning up old sandboxes. Maybe we could come up with a more explicit name, or add a comment stating what it's used for? (either in this PR if you have to update it, or in a follow-up)
Contributor
Author
There was a problem hiding this comment.
I renamed it to sandboxRestore, with comment "sandboxRestore restores Sandbox objects from the store, deleting them if they're not active." ... I think that covers it?
Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
On live-restore, the Sandbox tries to restore state in the osSbox by telling it about interface, routes, and gateways that would have been set up by the previous incarnation of the daemon. Restoring gateways has been broken since commit 1832774 (Allow separate IPv4/IPv6 gateway endpoints.) ... which didn't properly deal with searching for the "dstName" of an interface based on its IPv6 address. This commit fixes that, and splits the osSbox restore into three parts: - Restore the interfaces, including finding the "dstName". - Restore routes, unchanged, they're just a copy of the sandbox's StaticRoutes - Restore gateway info - if the Sandbox's gateway endpoint has an IP address (v4 or v6, depending on which addr family/families it's acting as the gateway for), store that. If not, the default route is bound to the interface, so store that. Signed-off-by: Rob Murray <[email protected]>
Signed-off-by: Rob Murray <[email protected]>
robmry
force-pushed
the
live_restore_fixes
branch
from
fb0b8b0 to
39c0517
Compare
Contributor
Author
|
Unrelated failure in CI ... |
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.
- What I did
On live-restore, the Sandbox tries to restore state in the osSbox by telling it about interface, routes, and gateways that would have been set up by the previous incarnation of the daemon.
Restoring gateways has been broken since commit 1832774 (Allow separate IPv4/IPv6 gateway endpoints) ... which didn't properly deal with searching for the "dstName" of an interface based on its IPv6 address.
- How I did it
Split the osSbox restore into three parts:
- How to verify it
Manually ...
Also, repeat test
TestMixL3IPVlanAndBridgewith a live-restore-enabled daemon restart before running the network connects/disconnects - and add an IPv6-only network. Without the fixes in this PR, gateways/default routes could not be removed when an Endpoint leaves the Sandbox, and the IPv6-only network couldn't become a gateway because thedstNameof its interface hadn't been restored correctly.- Description for the changelog