From 10007ac20b43445f9bc684589ba611d8538a919d Mon Sep 17 00:00:00 2001 From: Thibault Jamet Date: Fri, 29 Nov 2024 15:35:32 +0100 Subject: [PATCH] refactor the way healtcheck is added Goal --- Ease tests and increase code readability Change-Id: I12dc66552733eaa5e6258df00a659d6bf410f9c3 --- pkg/controllers/ingress_controller.go | 65 ++++++++++++++-------- pkg/controllers/ingress_controller_test.go | 50 +++++++++++++++++ 2 files changed, 93 insertions(+), 22 deletions(-) diff --git a/pkg/controllers/ingress_controller.go b/pkg/controllers/ingress_controller.go index bae2ee1..b1b8dd1 100644 --- a/pkg/controllers/ingress_controller.go +++ b/pkg/controllers/ingress_controller.go @@ -27,6 +27,11 @@ import ( externaldnsk8siov1alpha1 "sigs.k8s.io/external-dns/endpoint" ) +const ( + HealthcheckIDProperty = "aws/health-check-id" + WeightProperty = "aws/weight" +) + type annotationFilter struct { key string value string @@ -133,22 +138,45 @@ func (r *IngressReconciler) calculateIngressWeight(ingress netv1.Ingress) (uint, return desiredWeight, err } +func setEndpointProviderSpecificProperty(endpoint *externaldnsk8siov1alpha1.Endpoint, key, value string) { + for i, property := range endpoint.ProviderSpecific { + if property.Name == key { + endpoint.ProviderSpecific[i].Value = value + return + } + } + endpoint.ProviderSpecific = append(endpoint.ProviderSpecific, externaldnsk8siov1alpha1.ProviderSpecificProperty{ + Name: key, + Value: value, + }) +} + +func removeProviderSpecificProperty(endpoint *externaldnsk8siov1alpha1.Endpoint, key string) { + for i, property := range endpoint.ProviderSpecific { + if property.Name == key { + endpoint.ProviderSpecific = append(endpoint.ProviderSpecific[:i], endpoint.ProviderSpecific[i+1:]...) + return + } + } +} + +func setGlobalHealthCheckID(endpoint *externaldnsk8siov1alpha1.Endpoint) { + if trafficweight.Store.AWSHealthCheckID != "" { + setEndpointProviderSpecificProperty(endpoint, HealthcheckIDProperty, trafficweight.Store.AWSHealthCheckID) + } else { + removeProviderSpecificProperty(endpoint, HealthcheckIDProperty) + } +} + func (r *IngressReconciler) newDnsEndpoint(ctx context.Context, dnsEndpoint *externaldnsk8siov1alpha1.DNSEndpoint, target string, ingress netv1.Ingress) { var desiredWeight uint var err error - var healthCheckProperty *externaldnsk8siov1alpha1.ProviderSpecificProperty setOwnerRef(dnsEndpoint, &ingress) dnsEndpoint.Name = ingress.ObjectMeta.Name dnsEndpoint.Namespace = ingress.ObjectMeta.Namespace desiredWeight = uint(trafficweight.Store.DesiredWeight) - if trafficweight.Store.AWSHealthCheckID != "" { - healthCheckProperty = &externaldnsk8siov1alpha1.ProviderSpecificProperty{ - Name: "aws/health-check-id", - Value: trafficweight.Store.AWSHealthCheckID, - } - } if r.isIngressWeighted(ingress) { desiredWeight, err = r.calculateIngressWeight(ingress) if err != nil { @@ -167,25 +195,18 @@ func (r *IngressReconciler) newDnsEndpoint(ctx context.Context, dnsEndpoint *ext desiredWeight = 0 } - providerSpecificProperties := externaldnsk8siov1alpha1.ProviderSpecific{ - externaldnsk8siov1alpha1.ProviderSpecificProperty{ - Name: "aws/weight", - Value: strconv.FormatUint(uint64(desiredWeight), 10), - }, - } - if healthCheckProperty != nil { - providerSpecificProperties = append(providerSpecificProperties, *healthCheckProperty) - } - dnsEndpoint.Spec.Endpoints = append(dnsEndpoint.Spec.Endpoints, &externaldnsk8siov1alpha1.Endpoint{ + ep := &externaldnsk8siov1alpha1.Endpoint{ DNSName: rule.Host, Targets: externaldnsk8siov1alpha1.Targets{ target, }, - RecordType: "CNAME", - SetIdentifier: r.ClusterName, - ProviderSpecific: providerSpecificProperties, - }, - ) + RecordType: "CNAME", + SetIdentifier: r.ClusterName, + } + + setEndpointProviderSpecificProperty(ep, WeightProperty, strconv.FormatUint(uint64(desiredWeight), 10)) + setGlobalHealthCheckID(ep) + dnsEndpoint.Spec.Endpoints = append(dnsEndpoint.Spec.Endpoints, ep) } } diff --git a/pkg/controllers/ingress_controller_test.go b/pkg/controllers/ingress_controller_test.go index 3551c64..b23c5be 100644 --- a/pkg/controllers/ingress_controller_test.go +++ b/pkg/controllers/ingress_controller_test.go @@ -328,3 +328,53 @@ func TestIngressController(t *testing.T) { assert.Equal(t, ing.Status, newIng.Status) }) } + +func TestSetEndpointSpecificProperty(t *testing.T) { + ep := externaldnsk8siov1alpha1.DNSEndpoint{ + Spec: externaldnsk8siov1alpha1.DNSEndpointSpec{ + Endpoints: []*externaldnsk8siov1alpha1.Endpoint{{}}, + }, + } + + setEndpointProviderSpecificProperty(ep.Spec.Endpoints[0], "test", "test-value") + + require.Len(t, ep.Spec.Endpoints[0].ProviderSpecific, 1) + assert.Equal(t, "test", ep.Spec.Endpoints[0].ProviderSpecific[0].Name) + assert.Equal(t, "test-value", ep.Spec.Endpoints[0].ProviderSpecific[0].Value) + + setEndpointProviderSpecificProperty(ep.Spec.Endpoints[0], "test", "test-value-2") + require.Len(t, ep.Spec.Endpoints[0].ProviderSpecific, 1) + assert.Equal(t, "test", ep.Spec.Endpoints[0].ProviderSpecific[0].Name) + assert.Equal(t, "test-value-2", ep.Spec.Endpoints[0].ProviderSpecific[0].Value) +} + +func TestSetGlobalHealthcheckID(t *testing.T) { + t.Run("When the global healthcheck ID is set", func(t *testing.T) { + ep := externaldnsk8siov1alpha1.DNSEndpoint{ + Spec: externaldnsk8siov1alpha1.DNSEndpointSpec{ + Endpoints: []*externaldnsk8siov1alpha1.Endpoint{{}}, + }, + } + trafficweight.Store.AWSHealthCheckID = "1234" + + setGlobalHealthCheckID(ep.Spec.Endpoints[0]) + + require.Len(t, ep.Spec.Endpoints[0].ProviderSpecific, 1) + assert.Equal(t, HealthcheckIDProperty, ep.Spec.Endpoints[0].ProviderSpecific[0].Name) + assert.Equal(t, "1234", ep.Spec.Endpoints[0].ProviderSpecific[0].Value) + }) + t.Run("When the global healthcheck ID is not set", func(t *testing.T) { + ep := externaldnsk8siov1alpha1.DNSEndpoint{ + Spec: externaldnsk8siov1alpha1.DNSEndpointSpec{ + Endpoints: []*externaldnsk8siov1alpha1.Endpoint{{ + ProviderSpecific: []externaldnsk8siov1alpha1.ProviderSpecificProperty{{Name: HealthcheckIDProperty, Value: "1234"}}, + }}, + }, + } + trafficweight.Store.AWSHealthCheckID = "" + + setGlobalHealthCheckID(ep.Spec.Endpoints[0]) + + assert.Len(t, ep.Spec.Endpoints[0].ProviderSpecific, 0) + }) +}