feat(kuma-cp): do not require lb property external service routing (#…

…4109)

Signed-off-by: Łukasz Dziedziak <lukidzi@gmail.com>

UPGRADE.md

@@ -12,6 +12,11 @@ does not have any particular instructions.
 
 controlPlane.resources is now on object instead of a string. Any existing value should be adapted accordingly.
 
+### Zone egress and ExternalService
+
+When an `ExternalService` has the tag `kuma.io/zone` and `ZoneEgress` is enabled then the request flow will be different after upgrading Kuma to the newest version.
+Previously, the request to the `ExternalService` goes through the `ZoneEgress` in the current zone. The newest version flow is different, and when `ExternalService` is defined in a different zone then the request will go through local `ZoneEgress` to `ZoneIngress` in zone where `ExternalService` is defined and leave the cluster through `ZoneEgress` in this cluster. To keep previous behavior, remove the `kuma.io/zone` tag from the `ExternalService` definition.
+
 ### Zone egress
 
 Previously, when mTLS was configured and `ZoneEgress` deployed, requests were routed automatically through `ZoneEgress`. Now it's required to

pkg/core/resources/apis/mesh/mesh_helpers.go

@@ -42,10 +42,6 @@ func (m *MeshResource) LocalityAwareLbEnabled() bool {
 	return m != nil && m.Spec.GetRouting().GetLocalityAwareLoadBalancing()
 }
 
-func (m *MeshResource) LocalityAwareExternalServicesEnabled() bool {
-	return m.ZoneEgressEnabled() && m.LocalityAwareLbEnabled()
-}
-
 func (m *MeshResource) GetLoggingBackend(name string) *mesh_proto.LoggingBackend {
 	backends := map[string]*mesh_proto.LoggingBackend{}
 	for _, backend := range m.Spec.GetLogging().GetBackends() {

pkg/xds/generator/egress/external_services_generator.go

@@ -117,7 +117,7 @@ func (*ExternalServicesGenerator) buildServices(
 
 	for serviceName, endpoints := range endpointMap {
 		if len(endpoints) > 0 && endpoints[0].IsExternalService() &&
-			(!meshResources.Mesh.LocalityAwareExternalServicesEnabled() || endpoints[0].IsReachableFromZone(localZone)) {
+			(!meshResources.Mesh.ZoneEgressEnabled() || endpoints[0].IsReachableFromZone(localZone)) {
 			services[serviceName] = true
 		}
 	}

pkg/xds/generator/egress/internal_services_generator.go

@@ -27,7 +27,7 @@ func (g *InternalServicesGenerator) Generate(
 	apiVersion := proxy.APIVersion
 	endpointMap := meshResources.EndpointMap
 	destinations := buildDestinations(meshResources.TrafficRoutes)
-	services := g.buildServices(endpointMap, meshResources.Mesh.LocalityAwareExternalServicesEnabled(), zone)
+	services := g.buildServices(endpointMap, meshResources.Mesh.ZoneEgressEnabled(), zone)
 	meshName := meshResources.Mesh.GetMeta().GetName()
 
 	g.addFilterChains(
@@ -124,7 +124,7 @@ func (*InternalServicesGenerator) generateCDS(
 
 func (*InternalServicesGenerator) buildServices(
 	endpointMap core_xds.EndpointMap,
-	localityAwareExternalServicesEnabled bool,
+	zoneEgressEnabled bool,
 	localZone string,
 ) map[string]bool {
 	services := map[string]bool{}
@@ -134,7 +134,7 @@ func (*InternalServicesGenerator) buildServices(
 			continue
 		}
 		internalService := !endpoints[0].IsExternalService()
-		zoneExternalService := localityAwareExternalServicesEnabled && endpoints[0].IsExternalService() && !endpoints[0].IsReachableFromZone(localZone)
+		zoneExternalService := zoneEgressEnabled && endpoints[0].IsExternalService() && !endpoints[0].IsReachableFromZone(localZone)
 		if internalService || zoneExternalService {
 			services[serviceName] = true
 		}

pkg/xds/generator/egress/testdata/06.mixed-services-with-external-in-other-zone.golden.yaml

@@ -98,16 +98,13 @@ resources:
             envoy.lb:
               kuma.io/protocol: http2
               kuma.io/zone: zone-2
-              kuma.io/zone-external-service: "true"
               mesh: mesh-1
             envoy.transport_socket_match:
               kuma.io/protocol: http2
               kuma.io/zone: zone-2
-              kuma.io/zone-external-service: "true"
               mesh: mesh-1
       locality:
         zone: zone-2
-      priority: 1
 - name: mesh-1:service-1-zone-2
   resource:
     '@type': type.googleapis.com/envoy.config.endpoint.v3.ClusterLoadAssignment

pkg/xds/generator/egress/testdata/input/06.mixed-services-with-external-in-other-zone.yaml

@@ -6,7 +6,6 @@ mtls:
   - name: ca-1
     type: builtin
 routing:
-  localityAwareLoadBalancing: true
   zoneEgress: true
 ---
 type: ZoneEgress
@@ -64,10 +63,10 @@ availableServices:
     kuma.io/service: externalservice-2
     kuma.io/protocol: http2
     kuma.io/zone: zone-2
-    kuma.io/zone-external-service: "true"
     mesh: mesh-1
   instances: 30
   mesh: mesh-1
+  externalService: true
 ---
 type: ExternalService
 name: externalservice-1

pkg/xds/sync/ingress_proxy_builder.go

@@ -150,7 +150,7 @@ func (p *IngressProxyBuilder) getIngressExternalServices(ctx context.Context) (*
 	var meshes []*core_mesh.MeshResource
 
 	for _, mesh := range meshList.Items {
-		if mesh.LocalityAwareExternalServicesEnabled() {
+		if mesh.ZoneEgressEnabled() {
 			meshes = append(meshes, mesh)
 		}
 	}

pkg/xds/topology/outbound.go

@@ -55,7 +55,7 @@ func BuildRemoteEndpointMap(
 
 	fillIngressOutbounds(outbound, zoneIngresses, nil, zone, mesh)
 
-	if mesh.LocalityAwareExternalServicesEnabled() {
+	if mesh.ZoneEgressEnabled() {
 		fillExternalServicesReachableFromZone(outbound, externalServices, mesh, loader, zone)
 	} else {
 		fillExternalServicesOutbounds(outbound, externalServices, mesh, loader, zone)
@@ -237,7 +237,7 @@ func fillIngressOutbounds(
 					}
 					// this is necessary for correct spiffe generation for dp when
 					// traffic is routed: egress -> ingress -> egress
-					if mesh.LocalityAwareExternalServicesEnabled() && service.ExternalService {
+					if mesh.ZoneEgressEnabled() && service.ExternalService {
 						endpoint.ExternalService = &core_xds.ExternalService{}
 					}
 
@@ -251,7 +251,7 @@ func fillIngressOutbounds(
 					Weight:   serviceInstances,
 					Locality: locality,
 				}
-				if mesh.LocalityAwareExternalServicesEnabled() && service.ExternalService {
+				if mesh.ZoneEgressEnabled() && service.ExternalService {
 					endpoint.ExternalService = &core_xds.ExternalService{}
 				}
 

pkg/xds/topology/outbound_test.go

@@ -41,20 +41,6 @@ var _ = Describe("TrafficRoute", func() {
 			},
 		},
 	}
-	defaultMeshWithMTLSAndZoneEgressAndLB := &core_mesh.MeshResource{
-		Meta: &test_model.ResourceMeta{
-			Name: defaultMeshName,
-		},
-		Spec: &mesh_proto.Mesh{
-			Mtls: &mesh_proto.Mesh_Mtls{
-				EnabledBackend: "ca-1",
-			},
-			Routing: &mesh_proto.Routing{
-				LocalityAwareLoadBalancing: true,
-				ZoneEgress:                 true,
-			},
-		},
-	}
 	defaultMeshWithMTLSAndZoneEgressDisabled := &core_mesh.MeshResource{
 		Meta: &test_model.ResourceMeta{
 			Name: defaultMeshName,
@@ -1021,7 +1007,7 @@ var _ = Describe("TrafficRoute", func() {
 						},
 					},
 				},
-				mesh: defaultMeshWithMTLSAndZoneEgressAndLB,
+				mesh: defaultMeshWithMTLSAndZoneEgress,
 				expected: core_xds.EndpointMap{
 					"redis": []core_xds.Endpoint{
 						{
@@ -1037,7 +1023,7 @@ var _ = Describe("TrafficRoute", func() {
 							Port:            10002,
 							Tags:            map[string]string{mesh_proto.ServiceTag: "service-in-zone2", mesh_proto.ZoneTag: "zone-2"},
 							Weight:          1, // local weight is bumped to 2 to factor two instances of Ingresses
-							Locality:        &core_xds.Locality{Zone: "zone-2", Priority: 1},
+							Locality:        &core_xds.Locality{Zone: "zone-2", Priority: 0},
 							ExternalService: &core_xds.ExternalService{},
 						},
 					},
@@ -1097,15 +1083,15 @@ var _ = Describe("TrafficRoute", func() {
 							},
 						},
 					},
-					mesh: defaultMeshWithMTLSAndZoneEgressAndLB,
+					mesh: defaultMeshWithMTLSAndZoneEgress,
 					expected: core_xds.EndpointMap{
 						"service-in-zone2": []core_xds.Endpoint{
 							{
 								Target:          "192.168.0.100",
 								Port:            12345,
 								Tags:            map[string]string{mesh_proto.ServiceTag: "service-in-zone2", mesh_proto.ZoneTag: "zone-2", "mesh": "default"},
 								Weight:          2, // local weight is bumped to 2 to factor two instances of Ingresses
-								Locality:        &core_xds.Locality{Zone: "zone-2", Priority: 1},
+								Locality:        &core_xds.Locality{Zone: "zone-2", Priority: 0},
 								ExternalService: &core_xds.ExternalService{},
 							},
 						},
@@ -1115,7 +1101,7 @@ var _ = Describe("TrafficRoute", func() {
 								Port:     12345,
 								Tags:     map[string]string{mesh_proto.ServiceTag: "test", mesh_proto.ZoneTag: "zone-2", "mesh": "default"},
 								Weight:   3, // local weight is bumped to 2 to factor two instances of Ingresses
-								Locality: &core_xds.Locality{Zone: "zone-2", Priority: 1},
+								Locality: &core_xds.Locality{Zone: "zone-2", Priority: 0},
 							},
 						},
 						"httpbin": []core_xds.Endpoint{

test/e2e/externalservices/localityawarelb_multizone/locality_multizone_hybrid.go

@@ -13,7 +13,7 @@ import (
 	"github.com/kumahq/kuma/test/framework/envoy_admin/stats"
 )
 
-func meshMTLSOn(mesh string, localityLb string, zoneEgress string) string {
+func meshMTLSOn(mesh string, zoneEgress string) string {
 	return fmt.Sprintf(`
 type: Mesh
 name: %s
@@ -26,9 +26,8 @@ networking:
   outbound:
     passthrough: false
 routing:
-  localityAwareLoadBalancing: %s
   zoneEgress: %s
-`, mesh, localityLb, zoneEgress)
+`, mesh, zoneEgress)
 }
 
 func externalService(mesh string, ip string) string {
@@ -89,7 +88,7 @@ var _ = E2EBeforeSuite(func() {
 
 	Expect(NewClusterSetup().
 		Install(Kuma(config_core.Global)).
-		Install(YamlUniversal(meshMTLSOn(defaultMesh, "true", "true"))).
+		Install(YamlUniversal(meshMTLSOn(defaultMesh, "true"))).
 		Setup(global)).To(Succeed())
 
 	E2EDeferCleanup(global.DismissCluster)
@@ -154,7 +153,7 @@ var _ = E2EBeforeSuite(func() {
 func ExternalServicesOnMultizoneHybridWithLocalityAwareLb() {
 	BeforeEach(func() {
 		Expect(global.GetKumactlOptions().
-			KumactlApplyFromString(meshMTLSOn(defaultMesh, "true", "true")),
+			KumactlApplyFromString(meshMTLSOn(defaultMesh, "true")),
 		).To(Succeed())
 
 		k8sCluster := zone1.(*K8sCluster)
@@ -256,11 +255,11 @@ func ExternalServicesOnMultizoneHybridWithLocalityAwareLb() {
 		Eventually(EgressStats(zone4, filterEgress), "15s", "1s").Should(stats.BeGreaterThanZero())
 	})
 
-	It("requests should be routed through local zone egress when locality aware load balancing is disabled", func() {
+	It("requests should be routed directly through local sidecar when zone egress disabled", func() {
 		// given mesh with locality aware load balancing disabled
 		mesh := "demo"
 		err := NewClusterSetup().
-			Install(YamlUniversal(meshMTLSOn(mesh, "false", "true"))).
+			Install(YamlUniversal(meshMTLSOn(mesh, "false"))).
 			Install(YamlUniversal(zoneExternalService(mesh, zone4.GetApp("external-service-in-zone1").GetIP(), "demo-es-in-zone1", "kuma-1-zone"))).
 			Setup(global)
 		Expect(err).ToNot(HaveOccurred())
@@ -281,10 +280,8 @@ func ExternalServicesOnMultizoneHybridWithLocalityAwareLb() {
 		)
 		filterIngress := "cluster.demo-es-in-zone1.upstream_rq_total"
 
-		// and no request on path through ingress
-		Eventually(EgressStats(zone4, filterEgress), "15s", "1s").Should(stats.BeEqualZero())
-		Eventually(EgressStats(zone1, filterEgress), "15s", "1s").Should(stats.BeEqualZero())
-		Eventually(func(g Gomega) { // there is no stat because external service is not exposed through ingress
+		// and there is no stat because external service is not exposed through ingress
+		Eventually(func(g Gomega) {
 			s, err := zone1.GetZoneIngressEnvoyTunnel().GetStats(filterIngress)
 			g.Expect(err).ToNot(HaveOccurred())
 			g.Expect(s.Stats).To(BeEmpty())
@@ -296,10 +293,17 @@ func ExternalServicesOnMultizoneHybridWithLocalityAwareLb() {
 		Expect(err).ToNot(HaveOccurred())
 		Expect(stdout).To(ContainSubstring("HTTP/1.1 200 OK"))
 
-		// then should route:
-		// app -> zone egress (zone4) -> external service
-		Eventually(EgressStats(zone4, filterEgress), "15s", "1s").Should(stats.BeGreaterThanZero())
-		Eventually(EgressStats(zone1, filterEgress), "15s", "1s").Should(stats.BeEqualZero())
+		// then there is no stat because external service is not exposed through egress
+		Eventually(func(g Gomega) {
+			s, err := zone1.GetZoneIngressEnvoyTunnel().GetStats(filterEgress)
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(s.Stats).To(BeEmpty())
+		}, "15s", "1s").Should(Succeed())
+		Eventually(func(g Gomega) {
+			s, err := zone4.GetZoneIngressEnvoyTunnel().GetStats(filterEgress)
+			g.Expect(err).ToNot(HaveOccurred())
+			g.Expect(s.Stats).To(BeEmpty())
+		}, "15s", "1s").Should(Succeed())
 	})
 
 	It("should fail request when ingress is down", func() {