Skip to content

Commit 1da9f0b

Browse files
committed
Sync the gateway flows immediately for BGP and UDN
g.openflowManager.updateBridgeFlowCache is called from a few places but mainly from func (g *gateway) Reconcile(). However it looks like we don't actually call g.openflowManager.requestFlowSync() once we call cache update for the flows, so this means flows don't change immediately but wait for the next periodic ticker of 15seconds which makes the tests fail in CI lane. I think this is a longstanding bug that Reconcile was not calling requestFlowSync. Let's add requestFlowSync() to inside Reconcile so that it covers mutating the flows on the bridge immediately after the cache update. Note that in Reconcile, we also call reconcile for service flows which each inside do requestFlowSync() already for service flows. This will fix immediate flowsync to be triggered from missing places for BGP and UDN like: // reconcile subcontrollers if reconcilePodNetwork { isPodNetworkAdvertisedAtNode := util.IsPodNetworkAdvertisedAtNode(netInfo, oc.name) if oc.Gateway != nil { oc.Gateway.SetDefaultPodNetworkAdvertised(isPodNetworkAdvertisedAtNode) err := oc.Gateway.Reconcile() and postFunc := func() error { if err := udng.gateway.Reconcile(); err != nil { return fmt.Errorf("failed to reconcile flows on bridge for network %s; error: %v", udng.GetNetworkName(), err) } return nil } and // delete the openflows for this network if udng.openflowManager != nil { udng.openflowManager.delNetwork(udng.NetInfo) if err := udng.gateway.Reconcile(); err != nil { return fmt.Errorf("failed to reconcile default gateway for network %s, err: %v", udng.GetNetworkName(), err) } } Given doReconcile() has a mind of its own for BGP, I also explicitly added it there since that is the path my code tests Signed-off-by: Surya Seetharaman <[email protected]>
1 parent 75dd73f commit 1da9f0b

File tree

2 files changed

+5
-4
lines changed

2 files changed

+5
-4
lines changed

go-controller/pkg/node/gateway.go

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -241,7 +241,6 @@ func (g *gateway) AddEgressIP(eip *egressipv1.EgressIP) error {
241241
if err = g.Reconcile(); err != nil {
242242
return fmt.Errorf("failed to sync gateway: %v", err)
243243
}
244-
g.openflowManager.requestFlowSync()
245244
}
246245
return nil
247246
}
@@ -258,7 +257,6 @@ func (g *gateway) UpdateEgressIP(oldEIP, newEIP *egressipv1.EgressIP) error {
258257
if err = g.Reconcile(); err != nil {
259258
return fmt.Errorf("failed to sync gateway: %v", err)
260259
}
261-
g.openflowManager.requestFlowSync()
262260
}
263261
return nil
264262
}
@@ -275,7 +273,6 @@ func (g *gateway) DeleteEgressIP(eip *egressipv1.EgressIP) error {
275273
if err = g.Reconcile(); err != nil {
276274
return fmt.Errorf("failed to sync gateway: %v", err)
277275
}
278-
g.openflowManager.requestFlowSync()
279276
}
280277
return nil
281278
}
@@ -290,7 +287,6 @@ func (g *gateway) SyncEgressIP(eips []interface{}) error {
290287
if err := g.Reconcile(); err != nil {
291288
return fmt.Errorf("failed to sync gateway: %v", err)
292289
}
293-
g.openflowManager.requestFlowSync()
294290
return nil
295291
}
296292

@@ -492,6 +488,8 @@ func (g *gateway) Reconcile() error {
492488
if err := g.openflowManager.updateBridgeFlowCache(g.nodeIPManager.ListAddresses()); err != nil {
493489
return err
494490
}
491+
// let's sync these flows immediately
492+
g.openflowManager.requestFlowSync()
495493
err := g.updateSNATRules()
496494
if err != nil {
497495
return err

go-controller/pkg/node/gateway_udn.go

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -932,9 +932,12 @@ func (udng *UserDefinedNetworkGateway) doReconcile() error {
932932
// add below OpenFlows based on the gateway mode and whether the network is advertised or not:
933933
// table=1, n_packets=0, n_bytes=0, priority=16,ip,nw_dst=128.192.0.2 actions=LOCAL (Both gateway modes)
934934
// table=1, n_packets=0, n_bytes=0, priority=15,ip,nw_dst=128.192.0.0/14 actions=output:3 (shared gateway mode)
935+
// necessary service isolation flows based on whether network is advertised or not
935936
if err := udng.openflowManager.updateBridgeFlowCache(udng.nodeIPManager.ListAddresses()); err != nil {
936937
return fmt.Errorf("error while updating logical flow for UDN %s: %s", udng.GetNetworkName(), err)
937938
}
939+
// let's sync these flows immediately
940+
udng.openflowManager.requestFlowSync()
938941

939942
if err := udng.updateAdvertisedUDNIsolationRules(isNetworkAdvertised); err != nil {
940943
return fmt.Errorf("error while updating advertised UDN isolation rules for network %s: %w", udng.GetNetworkName(), err)

0 commit comments

Comments
 (0)