From d8191ce4105f9bd9d16049f71ba9c79ac2afbc1f Mon Sep 17 00:00:00 2001 From: Laura Fitzgerald Date: Tue, 12 Sep 2023 17:54:09 +0100 Subject: [PATCH] refactor endpoint probe check --- pkg/controllers/dnspolicy/dns_helper.go | 96 +- pkg/controllers/dnspolicy/dns_helper_test.go | 850 +----------------- .../dnspolicy/dnspolicy_dnsrecords.go | 9 +- pkg/dns/target.go | 57 ++ pkg/dns/target_test.go | 246 +++++ 5 files changed, 315 insertions(+), 943 deletions(-) diff --git a/pkg/controllers/dnspolicy/dns_helper.go b/pkg/controllers/dnspolicy/dns_helper.go index cd9345f72..fc5e45566 100644 --- a/pkg/controllers/dnspolicy/dns_helper.go +++ b/pkg/controllers/dnspolicy/dns_helper.go @@ -3,7 +3,6 @@ package dnspolicy import ( "context" "fmt" - "regexp" "sort" "strconv" "strings" @@ -171,7 +170,7 @@ func withGatewayListener[T metav1.Object](gateway common.GatewayWrapper, listene // ab2.lb-a1b2.shop.example.com A 192.22.2.3 // ab3.lb-a1b2.shop.example.com A 192.22.2.4 -func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClusterGatewayTarget, dnsRecord *v1alpha1.DNSRecord, dnsPolicy *v1alpha1.DNSPolicy, listener gatewayv1beta1.Listener) error { +func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClusterGatewayTarget, dnsRecord *v1alpha1.DNSRecord, listener gatewayv1beta1.Listener) error { old := dnsRecord.DeepCopy() gwListenerHost := string(*listener.Hostname) @@ -197,6 +196,7 @@ func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClust geoLbName := strings.ToLower(fmt.Sprintf("%s.%s", geoCode, lbName)) var clusterEndpoints []*v1alpha1.Endpoint for _, cgwTarget := range cgwTargets { + var ipValues []string var hostValues []string for _, gwa := range cgwTarget.GatewayAddresses { @@ -256,92 +256,14 @@ func (dh *dnsHelper) setEndpoints(ctx context.Context, mcgTarget *dns.MultiClust return newEndpoints[i].SetID() < newEndpoints[j].SetID() }) - probes, err := dh.getDNSHealthCheckProbes(ctx, mcgTarget.Gateway, dnsPolicy) - if err != nil { - return err - } + dnsRecord.Spec.Endpoints = newEndpoints - // if the checks on endpoints based on probes results in there being no healthy endpoints - // ready to publish we'll publish the full set so storing those - var storeEndpoints []*v1alpha1.Endpoint - storeEndpoints = append(storeEndpoints, newEndpoints...) - // count will track whether a new endpoint has been removed. - // first newEndpoints are checked based on probe status and removed if unhealthy true and the consecutive failures are greater than the threshold. - removedEndpoints := 0 - - for i := 0; i < len(newEndpoints); i++ { - checkProbes := getProbesForEndpoint(newEndpoints[i], probes, mcgTarget.Gateway.Name, string(listener.Name)) - if len(checkProbes) == 0 { - continue - } - for _, probe := range checkProbes { - probeHealthy := true - if probe.Status.Healthy != nil { - probeHealthy = *probe.Status.Healthy - } - // if any probe for any target is reporting unhealthy remove it from the endpoint list - if !probeHealthy && probe.Spec.FailureThreshold != nil && probe.Status.ConsecutiveFailures >= *probe.Spec.FailureThreshold { - newEndpoints = append(newEndpoints[:i], newEndpoints[i+1:]...) - removedEndpoints++ - i-- - break - } - } - } - // after checkProbes are checked the newEndpoints is looped through until count is 0 - // if any are found that need to be removed because a parent with no children present - // the count will be incremented so that the newEndpoints will be traversed again such that only when a loop occurs where no - // endpoints have been removed can we consider the endpoint list to be cleaned - ipPattern := `\b(?:\d{1,3}\.){3}\d{1,3}\b` - re := regexp.MustCompile(ipPattern) - - for removedEndpoints > 0 { - endpointsLoop: - for i := 0; i < len(newEndpoints); i++ { - checkEndpoint := newEndpoints[i] - for _, target := range checkEndpoint.Targets { - if len(re.FindAllString(target, -1)) > 0 { - // don't check the children of targets which are ips. - continue endpointsLoop - } - } - children := getNumChildrenOfParent(newEndpoints, newEndpoints[i]) - if children == 0 { - newEndpoints = append(newEndpoints[:i], newEndpoints[i+1:]...) - removedEndpoints++ - } - } - removedEndpoints-- - } - - // if there are no healthy endpoints after checking, publish the full set before checks - if len(newEndpoints) == 0 { - dnsRecord.Spec.Endpoints = storeEndpoints - } else { - dnsRecord.Spec.Endpoints = newEndpoints - } if !equality.Semantic.DeepEqual(old, dnsRecord) { return dh.Update(ctx, dnsRecord) } return nil } -func getNumChildrenOfParent(endpoints []*v1alpha1.Endpoint, parent *v1alpha1.Endpoint) int { - return len(findChildren(endpoints, parent)) -} - -func findChildren(endpoints []*v1alpha1.Endpoint, parent *v1alpha1.Endpoint) []*v1alpha1.Endpoint { - var foundEPs []*v1alpha1.Endpoint - for _, endpoint := range endpoints { - for _, target := range parent.Targets { - if target == endpoint.DNSName { - foundEPs = append(foundEPs, endpoint) - } - } - } - return foundEPs -} - func createOrUpdateEndpoint(dnsName string, targets v1alpha1.Targets, recordType v1alpha1.DNSRecordType, setIdentifier string, recordTTL v1alpha1.TTL, currentEndpoints map[string]*v1alpha1.Endpoint) (endpoint *v1alpha1.Endpoint) { ok := false @@ -453,15 +375,3 @@ func (dh *dnsHelper) getDNSHealthCheckProbes(ctx context.Context, gateway *gatew return &obj, nil }) } - -func getProbesForEndpoint(endpoint *v1alpha1.Endpoint, probes []*v1alpha1.DNSHealthCheckProbe, gatewayName, listenerName string) []*v1alpha1.DNSHealthCheckProbe { - retProbes := []*v1alpha1.DNSHealthCheckProbe{} - for _, probe := range probes { - for _, target := range endpoint.Targets { - if dnsHealthCheckProbeName(target, gatewayName, listenerName) == probe.Name { - retProbes = append(retProbes, probe) - } - } - } - return retProbes -} diff --git a/pkg/controllers/dnspolicy/dns_helper_test.go b/pkg/controllers/dnspolicy/dns_helper_test.go index 0b7bbf5d1..61d07155d 100644 --- a/pkg/controllers/dnspolicy/dns_helper_test.go +++ b/pkg/controllers/dnspolicy/dns_helper_test.go @@ -8,8 +8,6 @@ import ( "strings" "testing" - "github.com/aws/aws-sdk-go/aws" - "k8s.io/apimachinery/pkg/api/equality" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -411,9 +409,6 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { mcgTarget *dns.MultiClusterGatewayTarget listener gatewayv1beta1.Listener dnsRecord *v1alpha1.DNSRecord - dnsPolicy *v1alpha1.DNSPolicy - probeOne *v1alpha1.DNSHealthCheckProbe - probeTwo *v1alpha1.DNSHealthCheckProbe wantSpec *v1alpha1.DNSRecordSpec wantErr bool }{ @@ -471,19 +466,6 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { Name: "test.example.com", }, }, - dnsPolicy: &v1alpha1.DNSPolicy{}, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "namespace", - }, - }, - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "namespace", - }, - }, wantSpec: &v1alpha1.DNSRecordSpec{ Endpoints: []*v1alpha1.Endpoint{ { @@ -600,19 +582,6 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { Name: "gw-test", }, }, - dnsPolicy: &v1alpha1.DNSPolicy{}, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "namespace", - }, - }, - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "namespace", - }, - }, wantSpec: &v1alpha1.DNSRecordSpec{ Endpoints: []*v1alpha1.Endpoint{ { @@ -753,19 +722,6 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { Name: "test.example.com", }, }, - dnsPolicy: &v1alpha1.DNSPolicy{}, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "namespace", - }, - }, - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "namespace", - }, - }, wantSpec: &v1alpha1.DNSRecordSpec{ Endpoints: []*v1alpha1.Endpoint{ { @@ -882,19 +838,6 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { Name: "test.example.com", }, }, - dnsPolicy: &v1alpha1.DNSPolicy{}, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "namespace", - }, - }, - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "namespace", - }, - }, wantSpec: &v1alpha1.DNSRecordSpec{ Endpoints: []*v1alpha1.Endpoint{ { @@ -1023,805 +966,16 @@ func Test_dnsHelper_setEndpoints(t *testing.T) { Name: "test.example.com", }, }, - dnsPolicy: &v1alpha1.DNSPolicy{}, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "namespace", - }, - }, - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "namespace", - }, - }, wantSpec: &v1alpha1.DNSRecordSpec{ Endpoints: []*v1alpha1.Endpoint{}, }, }, - { - name: "test endpoint presence when probe is present but no health status is reported yet", - listener: getTestListener("*.example.com"), - mcgTarget: &dns.MultiClusterGatewayTarget{ - Gateway: &gatewayv1beta1.Gateway{ - ObjectMeta: v1.ObjectMeta{ - Name: "testgw", - Namespace: "testns", - }, - }, - ClusterGatewayTargets: []dns.ClusterGatewayTarget{ - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "1.1.1.1", - }, - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "2.2.2.2", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-2", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.HostnameAddressType), - Value: "mylb.example.com", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - }, - }, - dnsRecord: &v1alpha1.DNSRecord{ - ObjectMeta: v1.ObjectMeta{ - Name: "test.example.com", - }, - }, - dnsPolicy: &v1alpha1.DNSPolicy{ - ObjectMeta: v1.ObjectMeta{ - Name: "testpolicy", - Namespace: "testns", - }, - }, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "testns", - }, - }, - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "testns", - }, - }, - wantSpec: &v1alpha1.DNSRecordSpec{ - Endpoints: []*v1alpha1.Endpoint{ - { - DNSName: "20qri0.lb-0ecjaw.example.com", - Targets: []string{"1.1.1.1", "2.2.2.2"}, - RecordType: "A", - RecordTTL: dns.DefaultTTL, - }, - { - DNSName: "default.lb-0ecjaw.example.com", - Targets: []string{"20qri0.lb-0ecjaw.example.com"}, - RecordType: "CNAME", - SetIdentifier: "20qri0.lb-0ecjaw.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "default.lb-0ecjaw.example.com", - Targets: []string{"mylb.example.com"}, - RecordType: "CNAME", - SetIdentifier: "mylb.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "lb-0ecjaw.example.com", - Targets: []string{"default.lb-0ecjaw.example.com"}, - RecordType: "CNAME", - SetIdentifier: "default", - RecordTTL: dns.DefaultCnameTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "geo-code", - Value: "*", - }, - }, - }, - { - DNSName: "*.example.com", - Targets: []string{"lb-0ecjaw.example.com"}, - RecordType: "CNAME", - RecordTTL: dns.DefaultCnameTTL, - }, - }, - }, - }, - { - name: "test endpoint presence when probe is present but healthy status is reported", - listener: getTestListener("test.example.com"), - mcgTarget: &dns.MultiClusterGatewayTarget{ - Gateway: &gatewayv1beta1.Gateway{ - ObjectMeta: v1.ObjectMeta{ - Name: "testgw", - Namespace: "testns", - }, - }, - ClusterGatewayTargets: []dns.ClusterGatewayTarget{ - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "1.1.1.1", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "2.2.2.2", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - }, - }, - dnsRecord: &v1alpha1.DNSRecord{ - ObjectMeta: v1.ObjectMeta{ - Name: "test.example.com", - }, - }, - dnsPolicy: &v1alpha1.DNSPolicy{ - ObjectMeta: v1.ObjectMeta{ - Name: "testpolicy", - Namespace: "testns", - }, - }, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "testns", - }, - Status: v1alpha1.DNSHealthCheckProbeStatus{ - Healthy: aws.Bool(true), - }, - }, - - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "testns", - }, - Status: v1alpha1.DNSHealthCheckProbeStatus{ - Healthy: aws.Bool(true), - }, - }, - wantSpec: &v1alpha1.DNSRecordSpec{ - Endpoints: []*v1alpha1.Endpoint{ - { - DNSName: "20qri0.lb-0ecjaw.test.example.com", - Targets: []string{"1.1.1.1"}, - RecordType: "A", - RecordTTL: dns.DefaultTTL, - }, - { - DNSName: "20qri0.lb-0ecjaw.test.example.com", - Targets: []string{"2.2.2.2"}, - RecordType: "A", - RecordTTL: dns.DefaultTTL, - }, - { - DNSName: "default.lb-0ecjaw.test.example.com", - Targets: []string{"20qri0.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "20qri0.lb-0ecjaw.test.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "default.lb-0ecjaw.test.example.com", - Targets: []string{"20qri0.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "20qri0.lb-0ecjaw.test.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "lb-0ecjaw.test.example.com", - Targets: []string{"default.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "default", - RecordTTL: dns.DefaultCnameTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "geo-code", - Value: "*", - }, - }, - }, - { - DNSName: "test.example.com", - Targets: []string{"lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - RecordTTL: dns.DefaultCnameTTL, - }, - }, - }, - }, - { - name: "test removal of endpoint based on probe", - listener: getTestListener("test.example.com"), - mcgTarget: &dns.MultiClusterGatewayTarget{ - Gateway: &gatewayv1beta1.Gateway{ - ObjectMeta: v1.ObjectMeta{ - Name: "testgw", - Namespace: "testns", - }, - }, - ClusterGatewayTargets: []dns.ClusterGatewayTarget{ - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "2.2.2.2", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-2", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "1.1.1.1", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - }, - }, - dnsRecord: &v1alpha1.DNSRecord{ - ObjectMeta: v1.ObjectMeta{ - Name: "test.example.com", - }, - }, - dnsPolicy: &v1alpha1.DNSPolicy{ - ObjectMeta: v1.ObjectMeta{ - Name: "testpolicy", - Namespace: "testns", - }, - }, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "testns", - }, - Spec: v1alpha1.DNSHealthCheckProbeSpec{ - FailureThreshold: aws.Int(4), - }, - Status: v1alpha1.DNSHealthCheckProbeStatus{ - Healthy: aws.Bool(false), - ConsecutiveFailures: 54, - }, - }, - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "testns", - }, - Spec: v1alpha1.DNSHealthCheckProbeSpec{ - FailureThreshold: aws.Int(4), - }, - Status: v1alpha1.DNSHealthCheckProbeStatus{ - Healthy: aws.Bool(false), - ConsecutiveFailures: 2, - }, - }, - wantSpec: &v1alpha1.DNSRecordSpec{ - Endpoints: []*v1alpha1.Endpoint{ - { - DNSName: "20qri0.lb-0ecjaw.test.example.com", - Targets: []string{"2.2.2.2"}, - RecordType: "A", - RecordTTL: dns.DefaultTTL, - }, - { - DNSName: "default.lb-0ecjaw.test.example.com", - Targets: []string{"20qri0.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "20qri0.lb-0ecjaw.test.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "lb-0ecjaw.test.example.com", - Targets: []string{"default.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "default", - RecordTTL: dns.DefaultCnameTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "geo-code", - Value: "*", - }, - }, - }, - { - DNSName: "test.example.com", - Targets: []string{"lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - RecordTTL: dns.DefaultCnameTTL, - }, - }, - }, - }, - { - name: "test no removal of endpoint when all probes are unhealthy", - listener: getTestListener("test.example.com"), - mcgTarget: &dns.MultiClusterGatewayTarget{ - Gateway: &gatewayv1beta1.Gateway{ - ObjectMeta: v1.ObjectMeta{ - Name: "testgw", - Namespace: "testns", - }, - }, - ClusterGatewayTargets: []dns.ClusterGatewayTarget{ - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "1.1.1.1", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-2", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "2.2.2.2", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - }, - }, - dnsRecord: &v1alpha1.DNSRecord{ - ObjectMeta: v1.ObjectMeta{ - Name: "test.example.com", - }, - }, - dnsPolicy: &v1alpha1.DNSPolicy{ - ObjectMeta: v1.ObjectMeta{ - Name: "testpolicy", - Namespace: "testns", - }, - }, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "testns", - }, - Spec: v1alpha1.DNSHealthCheckProbeSpec{ - FailureThreshold: aws.Int(4), - }, - Status: v1alpha1.DNSHealthCheckProbeStatus{ - Healthy: aws.Bool(false), - ConsecutiveFailures: 6, - }, - }, - - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), - Namespace: "testns", - }, - Spec: v1alpha1.DNSHealthCheckProbeSpec{ - FailureThreshold: aws.Int(4), - }, - Status: v1alpha1.DNSHealthCheckProbeStatus{ - Healthy: aws.Bool(false), - ConsecutiveFailures: 6, - }, - }, - wantSpec: &v1alpha1.DNSRecordSpec{ - Endpoints: []*v1alpha1.Endpoint{ - { - DNSName: "20qri0.lb-0ecjaw.test.example.com", - Targets: []string{"1.1.1.1"}, - RecordType: "A", - RecordTTL: dns.DefaultTTL, - }, - { - DNSName: "2pj3we.lb-0ecjaw.test.example.com", - Targets: []string{"2.2.2.2"}, - RecordType: "A", - RecordTTL: dns.DefaultTTL, - }, - { - DNSName: "default.lb-0ecjaw.test.example.com", - Targets: []string{"20qri0.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "20qri0.lb-0ecjaw.test.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "default.lb-0ecjaw.test.example.com", - Targets: []string{"2pj3we.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "2pj3we.lb-0ecjaw.test.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "lb-0ecjaw.test.example.com", - Targets: []string{"default.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "default", - RecordTTL: dns.DefaultCnameTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "geo-code", - Value: "*", - }, - }, - }, - { - DNSName: "test.example.com", - Targets: []string{"lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - RecordTTL: dns.DefaultCnameTTL, - }, - }, - }, - }, - { - name: "test endpoint presence when probes are present for multiple listeners on the same cluster", - listener: getTestListener("test.example.com"), - mcgTarget: &dns.MultiClusterGatewayTarget{ - Gateway: &gatewayv1beta1.Gateway{ - ObjectMeta: v1.ObjectMeta{ - Name: "testgw", - Namespace: "testns", - }, - }, - ClusterGatewayTargets: []dns.ClusterGatewayTarget{ - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "1.1.1.1", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - { - - ClusterGateway: &dns.ClusterGateway{ - Cluster: &testutil.TestResource{ - ObjectMeta: v1.ObjectMeta{ - Name: "test-cluster-1", - }, - }, - GatewayAddresses: []gatewayv1beta1.GatewayAddress{ - { - Type: testutil.Pointer(gatewayv1beta1.IPAddressType), - Value: "2.2.2.2", - }, - }, - }, - Geo: testutil.Pointer(dns.GeoCode("default")), - Weight: testutil.Pointer(120), - }, - }, - }, - dnsRecord: &v1alpha1.DNSRecord{ - ObjectMeta: v1.ObjectMeta{ - Name: "test.example.com", - }, - }, - dnsPolicy: &v1alpha1.DNSPolicy{ - ObjectMeta: v1.ObjectMeta{ - Name: "testpolicy", - Namespace: "testns", - }, - }, - probeOne: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), - Namespace: "testns", - }, - Status: v1alpha1.DNSHealthCheckProbeStatus{ - Healthy: aws.Bool(true), - }, - }, - - probeTwo: &v1alpha1.DNSHealthCheckProbe{ - ObjectMeta: v1.ObjectMeta{ - Labels: commonDNSRecordLabels( - types.NamespacedName{ - Namespace: "testns", - Name: "testgw"}, - types.NamespacedName{ - Namespace: "testns", - Name: "testpolicy", - }), - Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "other"), - Namespace: "testns", - }, - Spec: v1alpha1.DNSHealthCheckProbeSpec{ - FailureThreshold: aws.Int(4), - }, - Status: v1alpha1.DNSHealthCheckProbeStatus{ - Healthy: aws.Bool(false), - ConsecutiveFailures: 6, - }, - }, - wantSpec: &v1alpha1.DNSRecordSpec{ - Endpoints: []*v1alpha1.Endpoint{ - { - DNSName: "20qri0.lb-0ecjaw.test.example.com", - Targets: []string{"1.1.1.1"}, - RecordType: "A", - RecordTTL: dns.DefaultTTL, - }, - { - DNSName: "20qri0.lb-0ecjaw.test.example.com", - Targets: []string{"2.2.2.2"}, - RecordType: "A", - RecordTTL: dns.DefaultTTL, - }, - { - DNSName: "default.lb-0ecjaw.test.example.com", - Targets: []string{"20qri0.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "20qri0.lb-0ecjaw.test.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "default.lb-0ecjaw.test.example.com", - Targets: []string{"20qri0.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "20qri0.lb-0ecjaw.test.example.com", - RecordTTL: dns.DefaultTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "weight", - Value: "120", - }, - }, - }, - { - DNSName: "lb-0ecjaw.test.example.com", - Targets: []string{"default.lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - SetIdentifier: "default", - RecordTTL: dns.DefaultCnameTTL, - ProviderSpecific: []v1alpha1.ProviderSpecificProperty{ - { - Name: "geo-code", - Value: "*", - }, - }, - }, - { - DNSName: "test.example.com", - Targets: []string{"lb-0ecjaw.test.example.com"}, - RecordType: "CNAME", - RecordTTL: dns.DefaultCnameTTL, - }, - }, - }, - }, } for _, testCase := range testCases { t.Run(testCase.name, func(t *testing.T) { - f := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(testCase.dnsRecord, testCase.probeOne, testCase.probeTwo).Build() + f := fake.NewClientBuilder().WithScheme(testScheme(t)).WithObjects(testCase.dnsRecord).Build() s := dnsHelper{Client: f} - if err := s.setEndpoints(context.TODO(), testCase.mcgTarget, testCase.dnsRecord, testCase.dnsPolicy, testCase.listener); (err != nil) != testCase.wantErr { + if err := s.setEndpoints(context.TODO(), testCase.mcgTarget, testCase.dnsRecord, testCase.listener); (err != nil) != testCase.wantErr { t.Errorf("SetEndpoints() error = %v, wantErr %v", err, testCase.wantErr) } diff --git a/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go b/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go index 7ffc94463..e79cae733 100644 --- a/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go +++ b/pkg/controllers/dnspolicy/dnspolicy_dnsrecords.go @@ -111,9 +111,14 @@ func (r *DNSPolicyReconciler) reconcileGatewayDNSRecords(ctx context.Context, ga if err != nil { return fmt.Errorf("failed to create multi cluster gateway target for listener %s : %s ", listener.Name, err) } - log.Info("setting dns dnsTargets for gateway listener", "listener", dnsRecord.Name, "values", mcgTarget) - if err := r.dnsHelper.setEndpoints(ctx, mcgTarget, dnsRecord, dnsPolicy, listener); err != nil { + log.Info("setting dns dnsTargets for gateway listener", "listener", dnsRecord.Name, "values", mcgTarget) + probes, err := r.dnsHelper.getDNSHealthCheckProbes(ctx, mcgTarget.Gateway, dnsPolicy) + if err != nil { + return err + } + mcgTarget.RemoveUnhealthyGatewayAddresses(probes, listener) + if err := r.dnsHelper.setEndpoints(ctx, mcgTarget, dnsRecord, listener); err != nil { return fmt.Errorf("failed to add dns record dnsTargets %s %v", err, mcgTarget) } } diff --git a/pkg/dns/target.go b/pkg/dns/target.go index 1e9d41812..041e821b0 100644 --- a/pkg/dns/target.go +++ b/pkg/dns/target.go @@ -153,6 +153,63 @@ func (t *ClusterGatewayTarget) setGeo(defaultGeo GeoCode) { t.Geo = &geoCode } +func (t *MultiClusterGatewayTarget) RemoveUnhealthyGatewayAddresses(probes []*v1alpha1.DNSHealthCheckProbe, listener gatewayv1beta1.Listener) { + + //If we have no probes we can't determine health so return unmodified + if len(probes) == 0 { + return + } + + //Build a map of gateway addresses and their health status + gwAddressHealth := map[string]bool{} + allunhealthy := true + for _, cgt := range t.ClusterGatewayTargets { + for _, gwa := range cgt.GatewayAddresses { + probe := getProbeForGatewayAddress(probes, gwa, t.Gateway.Name, string(listener.Name)) + if probe == nil { + continue + } + probeHealthy := true + if probe.Status.Healthy != nil { + probeHealthy = *probe.Status.Healthy + } + if probeHealthy && probe.Spec.FailureThreshold != nil && probe.Status.ConsecutiveFailures < *probe.Spec.FailureThreshold { + allunhealthy = false + } + gwAddressHealth[gwa.Value] = probeHealthy + + } + } + //If we have no matching probes for our current addresses, or we have no healthy probes, return unmodified + if len(gwAddressHealth) == 0 || allunhealthy { + return + } + + // Remove all unhealthy addresses, we know by this point at least one of our addresses is healthy + for _, cgt := range t.ClusterGatewayTargets { + healthyAddresses := []gatewayv1beta1.GatewayAddress{} + for _, gwa := range cgt.GatewayAddresses { + if healthy, exists := gwAddressHealth[gwa.Value]; exists && healthy { + healthyAddresses = append(healthyAddresses, gwa) + } + } + cgt.GatewayAddresses = healthyAddresses + } +} + +func getProbeForGatewayAddress(probes []*v1alpha1.DNSHealthCheckProbe, gwa gatewayv1beta1.GatewayAddress, gatewayName, listenerName string) *v1alpha1.DNSHealthCheckProbe { + for _, probe := range probes { + if dnsHealthCheckProbeName(gwa.Value, gatewayName, listenerName) == probe.Name { + return probe + } + } + return nil +} + +func dnsHealthCheckProbeName(address, gatewayName, listenerName string) string { + return fmt.Sprintf("%s-%s-%s", address, gatewayName, listenerName) +} + func (t *ClusterGatewayTarget) setWeight(defaultWeight int, customWeights []*v1alpha1.CustomWeight) error { weight := defaultWeight for k := range customWeights { diff --git a/pkg/dns/target_test.go b/pkg/dns/target_test.go index 7dc48e6ee..f051baa5c 100644 --- a/pkg/dns/target_test.go +++ b/pkg/dns/target_test.go @@ -3,11 +3,13 @@ package dns import ( + "fmt" "reflect" "testing" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" v1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "sigs.k8s.io/gateway-api/apis/v1alpha2" gatewayv1beta1 "sigs.k8s.io/gateway-api/apis/v1beta1" "github.com/Kuadrant/multicluster-gateway-controller/pkg/apis/v1alpha1" @@ -596,3 +598,247 @@ func buildGatewayAddress(value string) []gatewayv1beta1.GatewayAddress { }, } } + +func TestMultiClusterGatewayTarget_RemoveUnhealthyGatewayAddresses(t *testing.T) { + type fields struct { + Gateway *gatewayv1beta1.Gateway + ClusterGatewayTargets []ClusterGatewayTarget + LoadBalancing *v1alpha1.LoadBalancingSpec + } + type args struct { + probes []*v1alpha1.DNSHealthCheckProbe + listener gatewayv1beta1.Listener + } + tests := []struct { + name string + fields fields + args args + want []ClusterGatewayTarget + }{ + { + name: "healthy probes or no probes results in all gateways being kept", + fields: fields{ + ClusterGatewayTargets: []ClusterGatewayTarget{ + { + ClusterGateway: &ClusterGateway{ + GatewayAddresses: []v1alpha2.GatewayAddress{ + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "2.2.2.2", + }, + }, + }, + }, + }, + Gateway: &gatewayv1beta1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + }, + }, + args: args{ + probes: []*v1alpha1.DNSHealthCheckProbe{ + { + ObjectMeta: v1.ObjectMeta{ + Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), + Namespace: "namespace", + }, + Status: v1alpha1.DNSHealthCheckProbeStatus{ + ConsecutiveFailures: 0, + Healthy: testutil.Pointer(true), + }, + Spec: v1alpha1.DNSHealthCheckProbeSpec{ + FailureThreshold: testutil.Pointer(5), + }, + }, + { + ObjectMeta: v1.ObjectMeta{ + Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), + Namespace: "namespace", + }, + Status: v1alpha1.DNSHealthCheckProbeStatus{ + ConsecutiveFailures: 0, + Healthy: testutil.Pointer(true), + }, + Spec: v1alpha1.DNSHealthCheckProbeSpec{ + FailureThreshold: testutil.Pointer(5), + }, + }, + }, + listener: gatewayv1beta1.Listener{Name: "test"}, + }, + want: []ClusterGatewayTarget{ + { + ClusterGateway: &ClusterGateway{ + GatewayAddresses: []v1alpha2.GatewayAddress{ + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "2.2.2.2", + }, + }, + }, + }, + }, + }, + { + name: "some unhealthy probes results in the removal of a gateway", + fields: fields{ + ClusterGatewayTargets: []ClusterGatewayTarget{ + { + ClusterGateway: &ClusterGateway{ + GatewayAddresses: []v1alpha2.GatewayAddress{ + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "2.2.2.2", + }, + }, + }, + }, + }, + Gateway: &gatewayv1beta1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + }, + }, + args: args{ + probes: []*v1alpha1.DNSHealthCheckProbe{ + { + ObjectMeta: v1.ObjectMeta{ + Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), + Namespace: "namespace", + }, + Status: v1alpha1.DNSHealthCheckProbeStatus{ + ConsecutiveFailures: 6, + Healthy: testutil.Pointer(false), + }, + Spec: v1alpha1.DNSHealthCheckProbeSpec{ + FailureThreshold: testutil.Pointer(5), + }, + }, + { + ObjectMeta: v1.ObjectMeta{ + Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), + Namespace: "namespace", + }, + Status: v1alpha1.DNSHealthCheckProbeStatus{ + ConsecutiveFailures: 0, + Healthy: testutil.Pointer(true), + }, + Spec: v1alpha1.DNSHealthCheckProbeSpec{ + FailureThreshold: testutil.Pointer(5), + }, + }, + }, + listener: gatewayv1beta1.Listener{Name: "test"}, + }, + want: []ClusterGatewayTarget{ + { + ClusterGateway: &ClusterGateway{ + GatewayAddresses: []v1alpha2.GatewayAddress{ + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "2.2.2.2", + }, + }, + }, + }, + }, + }, + { + name: "all unhealthy probes results in all gateways being kept", + fields: fields{ + ClusterGatewayTargets: []ClusterGatewayTarget{ + { + ClusterGateway: &ClusterGateway{ + GatewayAddresses: []v1alpha2.GatewayAddress{ + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "2.2.2.2", + }, + }, + }, + }, + }, + Gateway: &gatewayv1beta1.Gateway{ + ObjectMeta: v1.ObjectMeta{Name: "testgw"}, + }, + }, + args: args{ + probes: []*v1alpha1.DNSHealthCheckProbe{ + { + ObjectMeta: v1.ObjectMeta{ + Name: dnsHealthCheckProbeName("1.1.1.1", "testgw", "test"), + Namespace: "namespace", + }, + Status: v1alpha1.DNSHealthCheckProbeStatus{ + ConsecutiveFailures: 6, + Healthy: testutil.Pointer(false), + }, + Spec: v1alpha1.DNSHealthCheckProbeSpec{ + FailureThreshold: testutil.Pointer(5), + }, + }, + { + ObjectMeta: v1.ObjectMeta{ + Name: dnsHealthCheckProbeName("2.2.2.2", "testgw", "test"), + Namespace: "namespace", + }, + Status: v1alpha1.DNSHealthCheckProbeStatus{ + ConsecutiveFailures: 6, + Healthy: testutil.Pointer(false), + }, + Spec: v1alpha1.DNSHealthCheckProbeSpec{ + FailureThreshold: testutil.Pointer(5), + }, + }, + }, + listener: gatewayv1beta1.Listener{Name: "test"}, + }, + want: []ClusterGatewayTarget{ + { + ClusterGateway: &ClusterGateway{ + GatewayAddresses: []v1alpha2.GatewayAddress{ + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "1.1.1.1", + }, + { + Type: testutil.Pointer(gatewayv1beta1.IPAddressType), + Value: "2.2.2.2", + }, + }, + }, + }, + }, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t1 *testing.T) { + mgt := &MultiClusterGatewayTarget{ + Gateway: tt.fields.Gateway, + ClusterGatewayTargets: tt.fields.ClusterGatewayTargets, + LoadBalancing: tt.fields.LoadBalancing, + } + mgt.RemoveUnhealthyGatewayAddresses(tt.args.probes, tt.args.listener) + if !reflect.DeepEqual(mgt.ClusterGatewayTargets, tt.want) { + for _, target := range mgt.ClusterGatewayTargets { + fmt.Println(target) + } + t.Errorf("got = %v, want %v", mgt.ClusterGatewayTargets, tt.want) + } + }) + } +}