Skip to content

Commit

Permalink
feat(xds): remove redundant fields
Browse files Browse the repository at this point in the history
Signed-off-by: Jay Chen <[email protected]>
  • Loading branch information
jijiechen committed Jan 3, 2024
1 parent e888b28 commit f3e2f52
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 33 deletions.
2 changes: 0 additions & 2 deletions pkg/core/xds/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,6 @@ type Routing struct {
// Since we take into account TrafficPermission to exclude external services from the map,
// it is specific for each data plane proxy.
ExternalServiceOutboundTargets EndpointMap
// Map to mark services are external even if there are no endpoints for them
ExternalServiceUnavailable map[string]struct{}
}

type CaSecret struct {
Expand Down
21 changes: 8 additions & 13 deletions pkg/xds/generator/outbound_proxy_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,8 @@ func (g OutboundProxyGenerator) Generate(ctx context.Context, _ *model.ResourceS
for _, outbound := range outbounds {
// Determine the list of destination subsets
// For one outbound listener it may contain many subsets (ex. TrafficRoute to many destinations)
routes := g.determineRoutes(proxy, outbound, clusterCache, xdsCtx.Mesh.Resource.ZoneEgressEnabled())
routes := g.determineRoutes(proxy, outbound, clusterCache,
xdsCtx.Mesh.ExternalServicesEndpointMap, xdsCtx.Mesh.Resource.ZoneEgressEnabled())
clusters := routes.Clusters()

protocol := inferProtocol(xdsCtx.Mesh, clusters)
Expand Down Expand Up @@ -324,11 +325,8 @@ func inferProtocol(meshCtx xds_context.MeshContext, clusters []envoy_common.Clus
return protocol
}

func (OutboundProxyGenerator) determineRoutes(
proxy *model.Proxy,
outbound *mesh_proto.Dataplane_Networking_Outbound,
clusterCache map[string]string,
hasEgress bool,
func (OutboundProxyGenerator) determineRoutes(proxy *model.Proxy, outbound *mesh_proto.Dataplane_Networking_Outbound,
clusterCache map[string]string, externalServiceMap model.EndpointMap, hasEgress bool,
) envoy_common.Routes {
var routes envoy_common.Routes
oface := proxy.Dataplane.Spec.Networking.ToOutboundInterface(outbound)
Expand Down Expand Up @@ -364,20 +362,17 @@ func (OutboundProxyGenerator) determineRoutes(
name = fmt.Sprintf("%s_%s", name, mesh)
}

// We assume that all the targets are either ExternalServices or not
// therefore we check only the first one
var isExternalService bool
var hasEndpoints bool

if _, ok := externalServiceMap[service]; ok {
isExternalService = true
}
if endpoints := proxy.Routing.OutboundTargets[service]; len(endpoints) > 0 {
hasEndpoints = true
isExternalService = endpoints[0].IsExternalService()
}
if endpoints := proxy.Routing.ExternalServiceOutboundTargets[service]; len(endpoints) > 0 {
hasEndpoints = true
isExternalService = true
}
if _, ok := proxy.Routing.ExternalServiceUnavailable[service]; ok {
isExternalService = true
}

// skip external services without any endpoints (instead of generating EDS as internal clusters)
Expand Down
26 changes: 23 additions & 3 deletions pkg/xds/generator/outbound_proxy_generator_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package generator_test

import (
"context"
"fmt"
"path/filepath"
"time"

Expand Down Expand Up @@ -99,6 +100,7 @@ var _ = Describe("OutboundProxyGenerator", func() {
},
Meta: meta,
},
ExternalServicesEndpointMap: createExternalServiceEndpointMap("es", "es2"),
},
}

Expand Down Expand Up @@ -965,9 +967,6 @@ var _ = Describe("OutboundProxyGenerator", func() {
},
OutboundTargets: model.EndpointMap{},
ExternalServiceOutboundTargets: model.EndpointMap{},
ExternalServiceUnavailable: map[string]struct{}{
"httpbin": {},
},
},
Metadata: &model.DataplaneMetadata{},
}
Expand All @@ -980,6 +979,7 @@ var _ = Describe("OutboundProxyGenerator", func() {
IsExternalService: true,
},
}
plainCtx.Mesh.ExternalServicesEndpointMap = createExternalServiceEndpointMap("httpbin")
rs, err := gen.Generate(context.Background(), nil, plainCtx, proxy)

// then
Expand All @@ -999,5 +999,25 @@ var _ = Describe("OutboundProxyGenerator", func() {
// and output matches golden files
Expect(actual).To(MatchGoldenYAML(filepath.Join("testdata", "outbound-proxy",
"external-services-no-endpoints.envoy.golden.yaml")))

// restore context
plainCtx.ControlPlane.CLACache = nil
plainCtx.Mesh.ServicesInformation = nil
plainCtx.Mesh.ExternalServicesEndpointMap = nil
})
})

func createExternalServiceEndpointMap(serviceNames ...string) model.EndpointMap {
epMap := model.EndpointMap{}
for idx, serviceName := range serviceNames {
epMap[serviceName] = []model.Endpoint{
{
Target: fmt.Sprintf("240.0.0.%d", idx),
Port: 80,
Tags: map[string]string{},
Weight: 1,
},
}
}
return epMap
}
15 changes: 0 additions & 15 deletions pkg/xds/sync/dataplane_proxy_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,13 +95,11 @@ func (p *DataplaneProxyBuilder) resolveRouting(
meshContext.DataSourceLoader,
p.Zone,
)
unavailableExternalServices := diffExternalServices(meshContext.Resources.ExternalServices(), endpointMap)

routing := &core_xds.Routing{
TrafficRoutes: routes,
OutboundTargets: meshContext.EndpointMap,
ExternalServiceOutboundTargets: endpointMap,
ExternalServiceUnavailable: unavailableExternalServices,
}
return routing, destinations
}
Expand Down Expand Up @@ -187,16 +185,3 @@ func (p *DataplaneProxyBuilder) matchPolicies(meshContext xds_context.MeshContex
}
return matchedPolicies, nil
}

func diffExternalServices(allExternalServices *core_mesh.ExternalServiceResourceList, endpointMap core_xds.EndpointMap) map[string]struct{} {
diff := make(map[string]struct{})

for _, external := range allExternalServices.Items {
service := external.Spec.GetService()
if _, ok := endpointMap[service]; !ok {
diff[service] = struct{}{}
}
}

return diff
}

0 comments on commit f3e2f52

Please sign in to comment.