Kv lsp enabled flag #4794
Kv lsp enabled flag #4794
Conversation
Doing so will allow dynamically controlling the LSP during cases like vm live migration in order to optimize downtime. This will be introduced in future commits. Signed-off-by: Ram Lavi <[email protected]>
currently all pods have the LSP.Enabled option set to true by default. During migration, it makes sense to handoff the network traffic from the source to target pod as soon as the target pod is up and ready for traffic. The handoff is done by closing the LSP of the source port. Doing this significantly reduces the migration downtime. This traffic handoff optimization is currently restricted to VM with only secondary interfaces, and layer2 topology. Signed-off-by: Ram Lavi <[email protected]>
Signed-off-by: Enrique Llorente <[email protected]>
| @@ -1178,6 +1187,6 @@ func (bnc *BaseNetworkController) wasPodReleasedBeforeStartup(uid, nad string) b | |||
| return bnc.releasedPodsBeforeStartup[nad].Has(uid) | |||
| } | |||
|
|
|||
| func isAllowedForMigration(isSecondary, isPrimaryNetwork, isl2Topology bool) bool { | |||
| return isSecondary && !isPrimaryNetwork && isl2Topology | |||
| func (bnc *BaseNetworkController) isAllowedForMigration() bool { | |||
There was a problem hiding this comment.
I'd have this as a function instead. There's no need for the layer3 controller to have this method for instance.
Also, should't localnet be also a "customer" of this feature ?... The exact same mechanism should be re-used there to improve the migration downtime, right ?
There was a problem hiding this comment.
Having a function that is going to receive NetInfo is kind of the same also if will be in the same context as layer3, also I am total agains a free function receiving random booleans.
There was a problem hiding this comment.
I don't understand what you mean, but FWIW, I also don't want a function that receives random booleans.
| SourcePod: vmPods[0], | ||
| TargetPod: vmPods[1], |
There was a problem hiding this comment.
could we trim down the objects ? IIUC we just require the labels on the pods. Right ?
| if isSourcePodAlreadyStale(pod, vmPods) && isTargetPodReady(getTargetPod(vmPods)) { | ||
| return true, nil | ||
| if len(vmPods) > 2 { | ||
| return nil, fmt.Errorf("unexpected live migration state at pods: %+v", vmPods) |
There was a problem hiding this comment.
too much info IMHO - while I understand having all potential relevant information at hand is helpful, we'd drown in data ...
What this PR does and why is it needed
Which issue(s) this PR fixes
Fixes #
Special notes for reviewers
How to verify it
Details to documentation updates
Description for the changelog
Does this PR introduce a user-facing change?