fix(kuma-cp): Stop adding outbounds to dp for vips #2421
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2421 +/- ##
==========================================
- Coverage 52.55% 52.54% -0.01%
==========================================
Files 878 877 -1
Lines 47904 47787 -117
==========================================
- Hits 25177 25111 -66
+ Misses 20680 20644 -36
+ Partials 2047 2032 -15
Continue to review full report at Codecov.
|
|
I think I'm doing this the wrong way. No need to review this :) |
lahabana
force-pushed
the
fix/vipoutbound
branch
2 times, most recently
from
6dacf20
to
eaf130a
Compare
The mesh hash contains the vips which now ensures that the watchdog will regenerate the xds config when needed. This enables us to remove this extra outbounds in the dataplane resource and simply generate the outbounds and the dns config in the dataplane_proxy_builder Signed-off-by: Charly Molter <charly.molter@konghq.com>
lahabana
force-pushed
the
fix/vipoutbound
branch
from
eaf130a
to
e3095bb
Compare
|
@lobkovilya now it's ready to review :) |
| @@ -105,6 +108,26 @@ func (p *DataplaneProxyBuilder) resolveRouting( | |||
| return nil, nil, err | |||
| } | |||
|
|
|||
| var domains []xds.VipDomains | |||
| outbounds := dataplane.Spec.Networking.Outbound | |||
| if dataplane.Spec.Networking.GetTransparentProxying() != nil && !dataplane.Spec.IsIngress() { | |||
There was a problem hiding this comment.
I think this code won't run for Ingresses, so probably the check could be dropped
| // resolve all the domains | ||
| domains, outbounds = xds_topology.VIPOutbounds(core_model.MetaToResourceKey(dataplane.Meta), meshContext.Dataplanes.Items, zoneIngresses.Items, dnsResolver.GetVIPs(), dnsResolver.GetDomain(), matchedExternalServices) | ||
|
|
||
| // Update the outbound of the dataplane with the vips |
There was a problem hiding this comment.
Do we really have to preserve the old VIPs? I think output we have from xds_topology.VIPOutbounds is sufficient so we can always do dataplane.Spec.Networking.Outbound = outbounds
There was a problem hiding this comment.
I believe this is preserving the non vip outbounds (In case a user defined outbounds in their dataplane resource).
What we do though is making sure we're not duplicating the outbounds in case there are still the generated ones in the resource.
WDYT?
There was a problem hiding this comment.
oh right, I forgot about non-vip outbounds on k8s :)
pkg/core/xds/types.go
Outdated
| @@ -126,9 +126,15 @@ type Proxy struct { | |||
| Policies MatchedPolicies | |||
| } | |||
|
|
|||
| type VipDomains struct { | |||
There was a problem hiding this comment.
nit: we already have VIPOutbounds function, so probably better to name it VIPDomains?
pkg/xds/generator/dns_generator.go
Outdated
| } | ||
| } | ||
| func (g DNSGenerator) computeVIPs(proxy *core_xds.Proxy) map[string][]string { | ||
| meshedVips := map[string][]string{} |
There was a problem hiding this comment.
Why it became string -> []string instead of string -> string mapping? It seems like it always has an array with single element as a value:
meshedVips[domain] = []string{dnsOutbound.Address}There was a problem hiding this comment.
I'm actually squeezing a change here for supporting both ipv4 and ipv6 in the future (I can make that a different PR if you want).
There was a problem hiding this comment.
I've actually removed the change. I'll do it when I need it
pkg/xds/generator/dns_generator.go
Outdated
| } | ||
| } | ||
| } | ||
| func (g DNSGenerator) computeVIPs(proxy *core_xds.Proxy) map[string][]string { |
There was a problem hiding this comment.
It's not really compute anymore, method became much simpler, it's just a conversion from []VipDomains to []map[domain]address, so maybe just toVIPs or something like that
There was a problem hiding this comment.
Got rid of the method I think it's just more readable inline
Signed-off-by: Charly Molter <charly.molter@konghq.com>
|
@Mergifyio update |
|
Command
|
|
|
The mesh hash contains the vips which now ensures that the watchdog
will regenerate the xds config when needed.
This enables us to remove this extra outbounds in the dataplane resource
and simply generate the outbounds and the dns config in the dataplane_proxy_builder
Backwards compatibility