Fix for specifying a health check port with an ExternalName Service (#…

…6230) Signed-off-by: yangyang <[email protected]>
projectcontour · Mar 4, 2024 · 53e9159 · 53e9159
1 parent b08ab87
commit 53e9159
Show file tree

Hide file tree

Showing 4 changed files with 129 additions and 1 deletion.
diff --git a/changelogs/unreleased/6230-yangyy93-small.md b/changelogs/unreleased/6230-yangyy93-small.md
@@ -0,0 +1 @@
+Fix for specifying a health check port with an ExternalName Service.
diff --git a/internal/envoy/v3/cluster_test.go b/internal/envoy/v3/cluster_test.go
@@ -72,6 +72,29 @@ func TestCluster(t *testing.T) {
 		},
 	}
 
+	s3 := &core_v1.Service{
+		ObjectMeta: meta_v1.ObjectMeta{
+			Name:      "kuard",
+			Namespace: "default",
+		},
+		Spec: core_v1.ServiceSpec{
+			ExternalName: "foo.io",
+			Ports: []core_v1.ServicePort{
+				{
+					Name:       "http",
+					Protocol:   "TCP",
+					Port:       443,
+					TargetPort: intstr.FromInt(8080),
+				}, {
+					Name:       "health-check",
+					Protocol:   "TCP",
+					Port:       8998,
+					TargetPort: intstr.FromInt(8998),
+				},
+			},
+		},
+	}
+
 	svcExternal := &core_v1.Service{
 		ObjectMeta: meta_v1.ObjectMeta{
 			Name:      "kuard",
@@ -196,6 +219,17 @@ func TestCluster(t *testing.T) {
 				LoadAssignment:       ExternalNameClusterLoadAssignment(service(s2)),
 			},
 		},
+		"externalName service healthcheckport": {
+			cluster: &dag.Cluster{
+				Upstream: healthcheckService(s3),
+			},
+			want: &envoy_config_cluster_v3.Cluster{
+				Name:                 "default/kuard/443/da39a3ee5e",
+				AltStatName:          "default_kuard_443",
+				ClusterDiscoveryType: ClusterDiscoveryType(envoy_config_cluster_v3.Cluster_STRICT_DNS),
+				LoadAssignment:       ExternalNameClusterLoadAssignment(healthcheckService(s3)),
+			},
+		},
 		"externalName service - dns-lookup-family v4": {
 			cluster: &dag.Cluster{
 				Upstream:        service(s2),
@@ -1192,3 +1226,16 @@ func service(s *core_v1.Service, protocols ...string) *dag.Service {
 		Protocol:     protocol,
 	}
 }
+
+func healthcheckService(s *core_v1.Service) *dag.Service {
+	return &dag.Service{
+		Weighted: dag.WeightedService{
+			Weight:           1,
+			ServiceName:      s.Name,
+			ServiceNamespace: s.Namespace,
+			ServicePort:      s.Spec.Ports[0],
+			HealthPort:       s.Spec.Ports[1],
+		},
+		ExternalName: s.Spec.ExternalName,
+	}
+}
diff --git a/internal/envoy/v3/endpoint.go b/internal/envoy/v3/endpoint.go
@@ -77,11 +77,15 @@ func ClusterLoadAssignment(name string, addrs ...*envoy_config_core_v3.Address)
 
 // ExternalNameClusterLoadAssignment creates a *envoy_config_endpoint_v3.ClusterLoadAssignment pointing to service's ExternalName DNS address.
 func ExternalNameClusterLoadAssignment(service *dag.Service) *envoy_config_endpoint_v3.ClusterLoadAssignment {
-	return ClusterLoadAssignment(
+	cla := ClusterLoadAssignment(
 		xds.ClusterLoadAssignmentName(
 			types.NamespacedName{Name: service.Weighted.ServiceName, Namespace: service.Weighted.ServiceNamespace},
 			service.Weighted.ServicePort.Name,
 		),
 		SocketAddress(service.ExternalName, int(service.Weighted.ServicePort.Port)),
 	)
+	if service.Weighted.ServicePort.Port != service.Weighted.HealthPort.Port {
+		cla.Endpoints[0].LbEndpoints[0].GetEndpoint().HealthCheckConfig = HealthCheckConfig(service.Weighted.HealthPort.Port)
+	}
+	return cla
 }
diff --git a/internal/envoy/v3/endpoint_test.go b/internal/envoy/v3/endpoint_test.go
@@ -18,7 +18,10 @@ import (
 
 	envoy_config_endpoint_v3 "github.com/envoyproxy/go-control-plane/envoy/config/endpoint/v3"
 	"github.com/stretchr/testify/require"
+	core_v1 "k8s.io/api/core/v1"
+	"k8s.io/apimachinery/pkg/util/intstr"
 
+	"github.com/projectcontour/contour/internal/dag"
 	"github.com/projectcontour/contour/internal/protobuf"
 )
 
@@ -97,3 +100,76 @@ func TestClusterLoadAssignment(t *testing.T) {
 
 	protobuf.RequireEqual(t, want, got)
 }
+
+func TestExternalNameClusterLoadAssignment(t *testing.T) {
+	s1 := &dag.Service{
+		Weighted: dag.WeightedService{
+			Weight:           1,
+			ServiceName:      "kuard",
+			ServiceNamespace: "default",
+			ServicePort: core_v1.ServicePort{
+				Name:       "http",
+				Protocol:   "TCP",
+				Port:       80,
+				TargetPort: intstr.FromInt32(8080),
+			},
+			HealthPort: core_v1.ServicePort{
+				Name:       "http",
+				Protocol:   "TCP",
+				Port:       80,
+				TargetPort: intstr.FromInt32(8080),
+			},
+		},
+		ExternalName: "foo.io",
+	}
+
+	s2 := &dag.Service{
+		Weighted: dag.WeightedService{
+			Weight:           1,
+			ServiceName:      "kuard",
+			ServiceNamespace: "default",
+			ServicePort: core_v1.ServicePort{
+				Name:       "http",
+				Protocol:   "TCP",
+				Port:       80,
+				TargetPort: intstr.FromInt32(8080),
+			},
+			HealthPort: core_v1.ServicePort{
+				Name:       "http",
+				Protocol:   "TCP",
+				Port:       8998,
+				TargetPort: intstr.FromInt32(8998),
+			},
+		},
+		ExternalName: "foo.io",
+	}
+
+	got := ExternalNameClusterLoadAssignment(s1)
+	want := &envoy_config_endpoint_v3.ClusterLoadAssignment{
+		ClusterName: "default/kuard/http",
+		Endpoints: Endpoints(
+			SocketAddress("foo.io", 80),
+		),
+	}
+	protobuf.RequireEqual(t, want, got)
+
+	got = ExternalNameClusterLoadAssignment(s2)
+	want = &envoy_config_endpoint_v3.ClusterLoadAssignment{
+		ClusterName: "default/kuard/http",
+		Endpoints: []*envoy_config_endpoint_v3.LocalityLbEndpoints{
+			{
+				LbEndpoints: []*envoy_config_endpoint_v3.LbEndpoint{
+					{
+						HostIdentifier: &envoy_config_endpoint_v3.LbEndpoint_Endpoint{
+							Endpoint: &envoy_config_endpoint_v3.Endpoint{
+								Address:           SocketAddress("foo.io", 80),
+								HealthCheckConfig: HealthCheckConfig(8998),
+							},
+						},
+					},
+				},
+			},
+		},
+	}
+	protobuf.RequireEqual(t, want, got)
+}