Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from
Apr 4, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions UPGRADE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 0 additions & 4 deletions pkg/core/resources/apis/mesh/mesh_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/xds/generator/egress/external_services_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/xds/generator/egress/internal_services_generator.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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{}
Expand All @@ -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
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ mtls:
- name: ca-1
type: builtin
routing:
localityAwareLoadBalancing: true
zoneEgress: true
---
type: ZoneEgress
Expand Down Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/xds/sync/ingress_proxy_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}
Expand Down
6 changes: 3 additions & 3 deletions pkg/xds/topology/outbound.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{}
}

Expand All @@ -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{}
}

Expand Down
24 changes: 5 additions & 19 deletions pkg/xds/topology/outbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1021,7 +1007,7 @@ var _ = Describe("TrafficRoute", func() {
},
},
},
mesh: defaultMeshWithMTLSAndZoneEgressAndLB,
mesh: defaultMeshWithMTLSAndZoneEgress,
expected: core_xds.EndpointMap{
"redis": []core_xds.Endpoint{
{
Expand All @@ -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{},
},
},
Expand Down Expand Up @@ -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{},
},
},
Expand All @@ -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{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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())
Expand All @@ -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())
Expand All @@ -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() {
Expand Down