Skip to content

Commit aee5b7e

Browse files
viktor-kurchenkotklauser
authored andcommitted
envoy: fix cluster locality handling
Make /cilium-locality-cluster use the selected xDS mode instead of always using EDS. For external Envoy, render the locality cluster with ADS config when envoy.xdsMode is ads or strict-ads, and keep the existing gRPC apiConfigSource for split mode. For embedded Envoy, reuse GetXDSConfigSource() when building the locality cluster so EDS references follow the configured xDS mode consistently. Signed-off-by: viktor-kurchenko <viktor.kurchenko@isovalent.com>
1 parent c2b823c commit aee5b7e

3 files changed

Lines changed: 62 additions & 27 deletions

File tree

install/kubernetes/cilium/files/cilium-envoy/configmap/bootstrap-config.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -277,6 +277,10 @@ staticResources:
277277
connectTimeout: "{{ .Values.envoy.connectTimeoutSeconds }}s"
278278
edsClusterConfig:
279279
edsConfig:
280+
{{- if or (eq $envoyXdsMode "ads") (eq $envoyXdsMode "strict-ads") }}
281+
ads: {}
282+
resourceApiVersion: "V3"
283+
{{- else }}
280284
apiConfigSource:
281285
apiType: "GRPC"
282286
transportApiVersion: "V3"
@@ -285,6 +289,7 @@ staticResources:
285289
clusterName: "xds-grpc-cilium"
286290
setNodeOnFirstMessageOnly: true
287291
resourceApiVersion: "V3"
292+
{{- end }}
288293
serviceName: "/cilium-locality-cluster"
289294
lbPolicy: "ROUND_ROBIN"
290295
{{- end }}

pkg/envoy/locality.go

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -57,23 +57,7 @@ func newLocalityCluster(connectTimeout int64) *envoy_config_cluster.Cluster {
5757
ClusterDiscoveryType: &envoy_config_cluster.Cluster_Type{Type: envoy_config_cluster.Cluster_EDS},
5858
ConnectTimeout: &durationpb.Duration{Seconds: connectTimeout},
5959
EdsClusterConfig: &envoy_config_cluster.Cluster_EdsClusterConfig{
60-
EdsConfig: &envoy_config_core.ConfigSource{
61-
ResourceApiVersion: envoy_config_core.ApiVersion_V3,
62-
ConfigSourceSpecifier: &envoy_config_core.ConfigSource_ApiConfigSource{
63-
ApiConfigSource: &envoy_config_core.ApiConfigSource{
64-
ApiType: envoy_config_core.ApiConfigSource_GRPC,
65-
TransportApiVersion: envoy_config_core.ApiVersion_V3,
66-
SetNodeOnFirstMessageOnly: true,
67-
GrpcServices: []*envoy_config_core.GrpcService{{
68-
TargetSpecifier: &envoy_config_core.GrpcService_EnvoyGrpc_{
69-
EnvoyGrpc: &envoy_config_core.GrpcService_EnvoyGrpc{
70-
ClusterName: CiliumXDSClusterName,
71-
},
72-
},
73-
}},
74-
},
75-
},
76-
},
60+
EdsConfig: GetXDSConfigSource(),
7761
ServiceName: LocalityClusterName,
7862
},
7963
LbPolicy: envoy_config_cluster.Cluster_ROUND_ROBIN,

pkg/envoy/locality_test.go

Lines changed: 56 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,68 @@ import (
88

99
envoy_config_bootstrap "github.com/envoyproxy/go-control-plane/envoy/config/bootstrap/v3"
1010
envoy_config_cluster "github.com/envoyproxy/go-control-plane/envoy/config/cluster/v3"
11+
corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3"
1112
"github.com/stretchr/testify/require"
13+
14+
"github.com/cilium/cilium/pkg/envoy/config"
1215
)
1316

1417
func TestAppendEmbeddedLocalityBootstrap(t *testing.T) {
15-
bs := &envoy_config_bootstrap.Bootstrap{
16-
StaticResources: &envoy_config_bootstrap.Bootstrap_StaticResources{},
18+
tests := []struct {
19+
name string
20+
xdsMode string
21+
assertEDS func(t *testing.T, edsConfig *corev3.ConfigSource)
22+
}{
23+
{
24+
name: "split",
25+
xdsMode: config.EnvoyXDSModeSplit,
26+
assertEDS: func(t *testing.T, edsConfig *corev3.ConfigSource) {
27+
apiConfigSource := edsConfig.GetApiConfigSource()
28+
require.NotNil(t, apiConfigSource)
29+
require.NotEmpty(t, apiConfigSource.GetGrpcServices())
30+
require.Equal(t, CiliumXDSClusterName, apiConfigSource.GetGrpcServices()[0].GetEnvoyGrpc().GetClusterName())
31+
require.Nil(t, edsConfig.GetAds())
32+
},
33+
},
34+
{
35+
name: "ads",
36+
xdsMode: config.EnvoyXDSModeADS,
37+
assertEDS: func(t *testing.T, edsConfig *corev3.ConfigSource) {
38+
require.NotNil(t, edsConfig.GetAds())
39+
require.Nil(t, edsConfig.GetApiConfigSource())
40+
},
41+
},
42+
{
43+
name: "strict-ads",
44+
xdsMode: config.EnvoyXDSModeStrictADS,
45+
assertEDS: func(t *testing.T, edsConfig *corev3.ConfigSource) {
46+
require.NotNil(t, edsConfig.GetAds())
47+
require.Nil(t, edsConfig.GetApiConfigSource())
48+
},
49+
},
1750
}
1851

19-
appendEmbeddedLocalityBootstrap(bs, 7, "zone-a")
52+
for _, tt := range tests {
53+
t.Run(tt.name, func(t *testing.T) {
54+
SetXDSMode(tt.xdsMode)
55+
t.Cleanup(func() { SetXDSMode("") })
56+
57+
bs := &envoy_config_bootstrap.Bootstrap{
58+
StaticResources: &envoy_config_bootstrap.Bootstrap_StaticResources{},
59+
}
60+
61+
appendEmbeddedLocalityBootstrap(bs, 7, "zone-a")
2062

21-
require.Equal(t, LocalityClusterName, bs.GetClusterManager().GetLocalClusterName())
22-
require.Equal(t, "zone-a", bs.GetNode().GetLocality().GetZone())
23-
require.Len(t, bs.GetStaticResources().GetClusters(), 1)
63+
require.Equal(t, LocalityClusterName, bs.GetClusterManager().GetLocalClusterName())
64+
require.Equal(t, "zone-a", bs.GetNode().GetLocality().GetZone())
65+
require.Len(t, bs.GetStaticResources().GetClusters(), 1)
2466

25-
cluster := bs.GetStaticResources().GetClusters()[0]
26-
require.Equal(t, LocalityClusterName, cluster.GetName())
27-
require.Equal(t, envoy_config_cluster.Cluster_EDS, cluster.GetType())
28-
require.Equal(t, CiliumXDSClusterName, cluster.GetEdsClusterConfig().GetEdsConfig().GetApiConfigSource().GetGrpcServices()[0].GetEnvoyGrpc().GetClusterName())
67+
cluster := bs.GetStaticResources().GetClusters()[0]
68+
require.Equal(t, LocalityClusterName, cluster.GetName())
69+
require.Equal(t, envoy_config_cluster.Cluster_EDS, cluster.GetType())
70+
require.Equal(t, LocalityClusterName, cluster.GetEdsClusterConfig().GetServiceName())
71+
72+
tt.assertEDS(t, cluster.GetEdsClusterConfig().GetEdsConfig())
73+
})
74+
}
2975
}

0 commit comments

Comments
 (0)