Merge pull request #113 from philbrookes/gh-92

adding health check e2e tests

internal/controller/dnsrecord_healthchecks.go

@@ -174,7 +174,7 @@ func getHealthChecksConfig(dnsRecord *v1alpha1.DNSRecord) *healthChecksConfig {
 // idForEndpoint returns a unique identifier for an endpoint
 func idForEndpoint(dnsRecord *v1alpha1.DNSRecord, endpoint *externaldns.Endpoint, address string) (string, error) {
 	hash := md5.New()
-	if _, err := io.WriteString(hash, fmt.Sprintf("%s/%s@%s:%s", dnsRecord.Name, endpoint.SetIdentifier, endpoint.DNSName, address)); err != nil {
+	if _, err := io.WriteString(hash, fmt.Sprintf("%s/%s@%s:%s-%v", dnsRecord.Name, endpoint.SetIdentifier, endpoint.DNSName, address, dnsRecord.Generation)); err != nil {
 		return "", fmt.Errorf("unexpected error creating ID for endpoint %s", endpoint.SetIdentifier)
 	}
 	return fmt.Sprintf("%x", hash.Sum(nil)), nil

internal/provider/aws/health.go

@@ -52,6 +52,11 @@ func getTransitionTime(probeConditions []metav1.Condition, conditionType string,
 	return metav1.Now()
 }
 
+func (r *Route53HealthCheckReconciler) HealthCheckExists(ctx context.Context, probeStatus *v1alpha1.HealthCheckStatusProbe) (bool, error) {
+	_, exists, err := r.findHealthCheck(ctx, probeStatus)
+	return exists, err
+}
+
 func (r *Route53HealthCheckReconciler) Reconcile(ctx context.Context, spec provider.HealthCheckSpec, endpoint *externaldns.Endpoint, probeStatus *v1alpha1.HealthCheckStatusProbe, address string) provider.HealthCheckResult {
 	healthCheck, exists, err := r.findHealthCheck(ctx, probeStatus)
 	if err != nil {

internal/provider/cached.go

@@ -28,6 +28,10 @@ func NewCachedHealthCheckReconciler(provider Provider, reconciler HealthCheckRec
 	}
 }
 
+func (r *CachedHealthCheckReconciler) HealthCheckExists(ctx context.Context, probeStatus *v1alpha1.HealthCheckStatusProbe) (bool, error) {
+	return r.reconciler.HealthCheckExists(ctx, probeStatus)
+}
+
 // Delete implements HealthCheckReconciler
 func (r *CachedHealthCheckReconciler) Delete(ctx context.Context, endpoint *externaldns.Endpoint, probeStatus *v1alpha1.HealthCheckStatusProbe) (HealthCheckResult, error) {
 	id, ok := r.getHealthCheckID(endpoint)

internal/provider/fake.go

@@ -11,6 +11,9 @@ import (
 
 type FakeHealthCheckReconciler struct{}
 
+func (*FakeHealthCheckReconciler) HealthCheckExists(ctx context.Context, probeStatus *v1alpha1.HealthCheckStatusProbe) (bool, error) {
+	return true, nil
+}
 func (*FakeHealthCheckReconciler) Reconcile(_ context.Context, _ HealthCheckSpec, _ *externaldns.Endpoint, _ *v1alpha1.HealthCheckStatusProbe, _ string) HealthCheckResult {
 	return HealthCheckResult{HealthCheckCreated, "fakeID", "", "", metav1.Condition{}}
 }

internal/provider/google/health.go

@@ -18,6 +18,10 @@ func NewGCPHealthCheckReconciler() *GCPHealthCheckReconciler {
 	return &GCPHealthCheckReconciler{}
 }
 
+func (r *GCPHealthCheckReconciler) HealthCheckExists(_ context.Context, _ *v1alpha1.HealthCheckStatusProbe) (bool, error) {
+	return true, nil
+}
+
 func (r *GCPHealthCheckReconciler) Reconcile(_ context.Context, _ provider.HealthCheckSpec, _ *externaldns.Endpoint, _ *v1alpha1.HealthCheckStatusProbe, _ string) provider.HealthCheckResult {
 	return provider.HealthCheckResult{}
 }

internal/provider/health.go

@@ -12,6 +12,7 @@ import (
 type HealthCheckReconciler interface {
 	Reconcile(ctx context.Context, spec HealthCheckSpec, endpoint *externaldns.Endpoint, probeStatus *v1alpha1.HealthCheckStatusProbe, address string) HealthCheckResult
 	Delete(ctx context.Context, endpoint *externaldns.Endpoint, probeStatus *v1alpha1.HealthCheckStatusProbe) (HealthCheckResult, error)
+	HealthCheckExists(ctx context.Context, probeStatus *v1alpha1.HealthCheckStatusProbe) (bool, error)
 }
 
 type HealthCheckSpec struct {

test/e2e/fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml

@@ -0,0 +1,53 @@
+apiVersion: kuadrant.io/v1alpha1
+kind: DNSRecord
+metadata:
+  name: ${testID}
+  namespace: ${testNamespace}
+spec:
+  healthCheck:
+    endpoint: "/"
+    port: 80
+    protocol: "HTTPS"
+    failureThreshold: 3
+  endpoints:
+    - dnsName: 14byhk-2k52h1.klb.${testHostname}
+      recordTTL: 60
+      recordType: A
+      targets:
+        - 172.32.200.1
+    - dnsName: ${testHostname}
+      recordTTL: 300
+      recordType: CNAME
+      targets:
+        - klb.${testHostname}
+    - dnsName: eu.klb.${testHostname}
+      providerSpecific:
+        - name: weight
+          value: "120"
+      recordTTL: 60
+      recordType: CNAME
+      setIdentifier: 14byhk-2k52h1.klb.${testHostname}
+      targets:
+        - 14byhk-2k52h1.klb.${testHostname}
+    - dnsName: klb.${testHostname}
+      providerSpecific:
+        - name: geo-code
+          value: ${testGeoCode}
+      recordTTL: 300
+      recordType: CNAME
+      setIdentifier: ${testGeoCode}
+      targets:
+        - eu.klb.${testHostname}
+    - dnsName: klb.${testHostname}
+      providerSpecific:
+        - name: geo-code
+          value: '*'
+      recordTTL: 300
+      recordType: CNAME
+      setIdentifier: default
+      targets:
+        - eu.klb.${testHostname}
+  managedZone:
+    name: ${TEST_DNS_MANAGED_ZONE_NAME}
+  ownerID: 2bq03i
+  rootHost: ${testHostname}

test/e2e/healthcheck_test.go

@@ -0,0 +1,215 @@
+//go:build e2e
+
+package e2e
+
+import (
+	"fmt"
+	"strings"
+	"time"
+
+	. "github.com/onsi/ginkgo/v2"
+	. "github.com/onsi/gomega"
+
+	"k8s.io/apimachinery/pkg/api/errors"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/utils/strings/slices"
+	"sigs.k8s.io/controller-runtime/pkg/client"
+
+	"github.com/kuadrant/dns-operator/api/v1alpha1"
+	"github.com/kuadrant/dns-operator/test/e2e/helpers"
+)
+
+// Test Cases covering multiple creation and deletion of health checks
+var _ = Describe("Health Check Test", Serial, func() {
+	// tests can be run in parallel in the same cluster
+	var testID string
+	// testDomainName generated domain for this test e.g. t-e2e-12345.e2e.hcpapps.net
+	var testDomainName string
+	// testHostname generated hostname for this test e.g. t-gw-mgc-12345.t-e2e-12345.e2e.hcpapps.net
+	var testHostname string
+
+	var dnsRecord *v1alpha1.DNSRecord
+
+	BeforeEach(func() {
+		testID = "t-health-" + GenerateName()
+		testDomainName = strings.Join([]string{testSuiteID, testZoneDomainName}, ".")
+		testHostname = strings.Join([]string{testID, testDomainName}, ".")
+		helpers.SetTestEnv("testID", testID)
+		helpers.SetTestEnv("testHostname", testHostname)
+		helpers.SetTestEnv("testNamespace", testNamespace)
+	})
+
+	AfterEach(func(ctx SpecContext) {
+		if dnsRecord != nil {
+			err := k8sClient.Delete(ctx, dnsRecord,
+				client.PropagationPolicy(metav1.DeletePropagationForeground))
+			Expect(client.IgnoreNotFound(err)).ToNot(HaveOccurred())
+		}
+	})
+
+	Context("DNS Provider health checks", func() {
+		It("creates health checks for a health check spec", func(ctx SpecContext) {
+			healthChecksSupported := false
+			if slices.Contains(supportedHealthCheckProviders, strings.ToLower(testDNSProvider)) {
+				healthChecksSupported = true
+			}
+
+			provider, err := providerForManagedZone(ctx, testManagedZone)
+			Expect(err).To(BeNil())
+
+			By("creating a DNS Record")
+			dnsRecord = &v1alpha1.DNSRecord{}
+			err = helpers.ResourceFromFile("./fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml", dnsRecord, helpers.GetTestEnv)
+			Expect(err).ToNot(HaveOccurred())
+
+			err = k8sClient.Create(ctx, dnsRecord)
+			Expect(err).ToNot(HaveOccurred())
+
+			By("Confirming the DNS Record status")
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				if healthChecksSupported {
+					g.Expect(dnsRecord.Status.HealthCheck).ToNot(BeNil())
+					g.Expect(&dnsRecord.Status.HealthCheck.Probes).ToNot(BeNil())
+					g.Expect(len(dnsRecord.Status.HealthCheck.Probes)).ToNot(BeZero())
+					for _, condition := range dnsRecord.Status.HealthCheck.Conditions {
+						if condition.Type == "healthProbesSynced" {
+							g.Expect(condition.Status).To(Equal(metav1.ConditionTrue))
+							g.Expect(condition.Reason).To(Equal("AllProbesSynced"))
+						}
+					}
+				} else {
+					g.Expect(dnsRecord.Status.HealthCheck).ToNot(BeNil())
+					g.Expect(dnsRecord.Status.HealthCheck.Probes).To(BeNil())
+				}
+
+				for _, probe := range dnsRecord.Status.HealthCheck.Probes {
+					g.Expect(probe.Host).To(Equal(testHostname))
+					g.Expect(probe.IPAddress).To(Equal("172.32.200.1"))
+					g.Expect(probe.ID).ToNot(Equal(""))
+
+					for _, probeCondition := range probe.Conditions {
+						g.Expect(probeCondition.Type).To(Equal("ProbeSynced"))
+						g.Expect(probeCondition.Status).To(Equal(metav1.ConditionTrue))
+						g.Expect(probeCondition.Message).To(ContainSubstring(fmt.Sprintf("id: %v, address: %v, host: %v", probe.ID, probe.IPAddress, probe.Host)))
+					}
+				}
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			By("confirming the health checks exist in the provider")
+			Eventually(func(g Gomega) {
+				if !healthChecksSupported {
+					g.Expect(len(dnsRecord.Status.HealthCheck.Probes)).To(BeZero())
+				}
+				for _, healthCheck := range dnsRecord.Status.HealthCheck.Probes {
+					exists, err := provider.HealthCheckReconciler().HealthCheckExists(ctx, &healthCheck)
+					g.Expect(err).To(BeNil())
+					g.Expect(exists).To(BeTrue())
+				}
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			By("Removing the health check")
+			oldHealthCheckStatus := dnsRecord.Status.HealthCheck.DeepCopy()
+			Eventually(func(g Gomega) {
+				patchFrom := client.MergeFrom(dnsRecord.DeepCopy())
+				dnsRecord.Spec.HealthCheck = nil
+				err := k8sClient.Patch(ctx, dnsRecord, patchFrom)
+				g.Expect(err).To(BeNil())
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			By("Confirming the DNS Record status")
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
+				g.Expect(err).ToNot(HaveOccurred())
+				g.Expect(dnsRecord.Status.HealthCheck).To(BeNil())
+			})
+
+			By("confirming the health checks were removed in the provider")
+			Eventually(func(g Gomega) {
+				for _, healthCheck := range oldHealthCheckStatus.Probes {
+					exists, err := provider.HealthCheckReconciler().HealthCheckExists(ctx, &healthCheck)
+					g.Expect(err).NotTo(BeNil())
+					g.Expect(exists).To(BeFalse())
+				}
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			By("Adding a health check spec")
+			Eventually(func(g Gomega) {
+				patchFrom := client.MergeFrom(dnsRecord.DeepCopy())
+				err = helpers.ResourceFromFile("./fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml", dnsRecord, helpers.GetTestEnv)
+				g.Expect(err).ToNot(HaveOccurred())
+				err := k8sClient.Patch(ctx, dnsRecord, patchFrom)
+				g.Expect(err).To(BeNil())
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			By("Confirming the DNS Record status")
+			Eventually(func(g Gomega) {
+				err := k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
+				g.Expect(err).ToNot(HaveOccurred())
+
+				if healthChecksSupported {
+					g.Expect(dnsRecord.Status.HealthCheck).ToNot(BeNil())
+					g.Expect(&dnsRecord.Status.HealthCheck.Probes).ToNot(BeNil())
+					g.Expect(len(dnsRecord.Status.HealthCheck.Probes)).ToNot(BeZero())
+					for _, condition := range dnsRecord.Status.HealthCheck.Conditions {
+						if condition.Type == "healthProbesSynced" {
+							g.Expect(condition.Status).To(Equal(metav1.ConditionTrue))
+							g.Expect(condition.Reason).To(Equal("AllProbesSynced"))
+						}
+					}
+				} else {
+					g.Expect(dnsRecord.Status.HealthCheck).ToNot(BeNil())
+					g.Expect(dnsRecord.Status.HealthCheck.Probes).To(BeNil())
+				}
+
+				for _, probe := range dnsRecord.Status.HealthCheck.Probes {
+					g.Expect(probe.Host).To(Equal(testHostname))
+					g.Expect(probe.IPAddress).To(Equal("172.32.200.1"))
+					g.Expect(probe.ID).ToNot(Equal(""))
+
+					for _, probeCondition := range probe.Conditions {
+						g.Expect(probeCondition.Type).To(Equal("ProbeSynced"))
+						g.Expect(probeCondition.Status).To(Equal(metav1.ConditionTrue))
+						g.Expect(probeCondition.Message).To(ContainSubstring(fmt.Sprintf("id: %v, address: %v, host: %v", probe.ID, probe.IPAddress, probe.Host)))
+					}
+				}
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			By("confirming the health checks exist in the provider")
+			Eventually(func(g Gomega) {
+				if !healthChecksSupported {
+					g.Expect(len(dnsRecord.Status.HealthCheck.Probes)).To(BeZero())
+				}
+				for _, healthCheck := range dnsRecord.Status.HealthCheck.Probes {
+					exists, err := provider.HealthCheckReconciler().HealthCheckExists(ctx, &healthCheck)
+					g.Expect(err).To(BeNil())
+					g.Expect(exists).To(BeTrue())
+				}
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			By("Deleting the DNS Record")
+			oldHealthCheckStatus = dnsRecord.Status.HealthCheck.DeepCopy()
+			err = helpers.ResourceFromFile("./fixtures/healthcheck_test/geo-dnsrecord-healthchecks.yaml", dnsRecord, helpers.GetTestEnv)
+			Expect(err).ToNot(HaveOccurred())
+			Eventually(func(g Gomega) {
+				err := k8sClient.Delete(ctx, dnsRecord)
+				g.Expect(client.IgnoreNotFound(err)).To(BeNil())
+
+				err = k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)
+				g.Expect(errors.IsNotFound(err)).Should(BeTrue())
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+			By("confirming the health checks were removed in the provider")
+			Eventually(func(g Gomega) {
+				for _, healthCheck := range oldHealthCheckStatus.Probes {
+					exists, err := provider.HealthCheckReconciler().HealthCheckExists(ctx, &healthCheck)
+					g.Expect(err).NotTo(BeNil())
+					g.Expect(exists).To(BeFalse())
+				}
+			}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+		})
+	})
+})

test/e2e/helpers/fixtures.go

@@ -0,0 +1,38 @@
+package helpers
+
+import (
+	"os"
+
+	"k8s.io/apimachinery/pkg/runtime"
+	"k8s.io/apimachinery/pkg/runtime/serializer"
+	"k8s.io/client-go/kubernetes/scheme"
+)
+
+var testEnvVars map[string]string
+
+func ResourceFromFile(file string, destObject runtime.Object, expandFunc func(string) string) error {
+	decode := serializer.NewCodecFactory(scheme.Scheme).UniversalDeserializer().Decode
+	stream, err := os.ReadFile(file)
+	if err != nil {
+		return err
+	}
+	stream = []byte(os.Expand(string(stream), expandFunc))
+	_, _, err = decode(stream, nil, destObject)
+	return err
+}
+
+func GetTestEnv(key string) string {
+	if testEnvVars != nil {
+		if v, ok := testEnvVars[key]; ok {
+			return v
+		}
+	}
+	return os.Getenv(key)
+}
+
+func SetTestEnv(key, value string) {
+	if testEnvVars == nil {
+		testEnvVars = map[string]string{}
+	}
+	testEnvVars[key] = value
+}