Policy engine cleanup

[orozery]

No description provided.

[orozery]

@zivnevo just heads up
not yet working

[zivnevo]

This is somewhat more than a cleanup - it changes the algorithm for selecting the destination peer for an egress request.
Previously we had:

1. Start with all source peers for the required Import
2. Filter-out unreachable peers
3. Consult PDP to filter-out unauthorized peers
4. Consult load-balancer to select one of the remaining peers (if any)

Now we have

1. Consult load-balancer to select a peer out of the not-yet-failing source peers for the required import.
2. Check peer status. If unreachable - mark as failing and go to 1.
3. Consult PDP to check if the selected peer is authorized. If not, mark as failing and go to 1.
4. Try to authorize the request at the selected peer. If unauthorized, mark as failing and go to 1.

While the new algorithm is more coherent and has simpler implementation, we may lose some performance in the (rare?) cases where imports have a large number of sources.

@elevran , @kfirtoledo , your thoughts?
@orozery , please correct me if I'm wrong somewhere.

[orozery]

we may lose some performance in the (rare?) cases where imports have a large number of sources.

Where do you lose performance?
Actually, in most cases you gain performance since you query the PDP only for one destination, and not all possible destinations.

[orozery]

@orozery , please correct me if I'm wrong somewhere.

One correction is that in step 2, if peer is unreachable, it is not marked as failed, but rather is "delayed".

To be more precise, per each egress connection, the load-balancing scheme yields some ordering (or shuffling if you may) of the list of import sources.
The controlplane tries them out one-by-one, except for those who's peer status is "unreachable" (which is determined periodically by monitoring heartbeats).
Those skipped (or "delayed") sources are tried out as a last resort only if all other sources failed.

[kfirtoledo]

I am ok with the new algorithm, but I'm not really sure about its impact on performance because anyway most of the time is spent on opening a connection to the destination in another cluster.

[elevran]

q: why is ns not part of either source or destination attributes?

[elevran]

nit: are we ok with the default values of the other fields?

[elevran]

I am ok with the new algorithm, but I'm not really sure about its impact on performance because anyway most of the time is spent on opening a connection to the destination in another cluster.

I think performance is a secondary concern.
The main motivation for the current algorithm was to ensure we only load balance on allowed destinations. This works if you do ACL before LB and filter out the denied destinations. It is obviously more work to check all potential destinations than just the specific one selected by LB.
If you do LB first you could end up with a destination that fails ACL and then you need to redo LB - being careful not to select ACL denied peers so you don't loop forever.
In either case, we may want to filter out unreachable peers before checking policy but that's an optimization.

A side issue is whether we want to mask failures by retrying internally in the gateway or let the client handle retries the way they see fit by failing to connect.