Fix loadbalancer handling of backends with ClusterID set#40968
Fix loadbalancer handling of backends with ClusterID set#40968giorio94 merged 3 commits intocilium:mainfrom
Conversation
To mimic the same method provided by [netip.Addr]. Additionally, let's reimplement the [AddrCluster.Less] on top of it. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
Let's make use of the newly introduced [AddrCluster.Compare] function, to ensure consistent ordering also when there are multiple backends associated with the same IP, but different ClusterID. Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
|
/test |
MrFreezeex
left a comment
There was a problem hiding this comment.
Thanks, looks good to me!
I left some small non blocking question/suggestion inline.
Also guessing this should be tagged as a bug release note rather than a misc one maybe?
Currently, the loadbalancer reconciler is affected by a bug causing the
incorrect pruning of backends if associated with a non-zero ClusterID,
because the ClusterID read from the map is ignored. Let's get this
fixed by unifying the [BackendValue.{GetAddress,GetIPCluster]] methods,
to ensure that all callers correctly account for the ClusterID.
Additionally, let's also extend the unit tests to cover this use-case
and prevent possible regressions.
Signed-off-by: Marco Iorio <marco.iorio@isovalent.com>
e4cba20 to
4415a62
Compare
I've marked it as misc because the ClusterID is never set to a value different from zero in this context today, AFAIK, so it doesn't really affect users. The main reason I've marked it for backport is to avoid having the two implementations diverge for the moment, given that the fix is trivial. But I can definitely remove the backport label is people feel against it. |
|
/test |
Looks reasonable to me thanks for the explanation! |
Please review commit by commit, and refer to the individual messages for additional details.