Merge pull request #307 from Kuadrant/gh-306-remove-probes

ensure we remove healthcheckprobes if healthcheck unset

Author: maleck13

api/v1alpha1/dnshealthcheckprobe_types.go

@@ -86,7 +86,6 @@ type DNSHealthCheckProbeStatus struct {
 //+kubebuilder:object:root=true
 //+kubebuilder:subresource:status
 //+kubebuilder:printcolumn:name="Healthy",type="boolean",JSONPath=".status.healthy",description="DNSHealthCheckProbe healthy."
-//+kubebuilder:printcolumn:name="Last Checked",type="date",JSONPath=".status.lastCheckedAt",description="Last checked at."
 
 // DNSHealthCheckProbe is the Schema for the dnshealthcheckprobes API
 type DNSHealthCheckProbe struct {

bundle/manifests/dns-operator.clusterserviceversion.yaml

@@ -58,7 +58,7 @@ metadata:
     capabilities: Basic Install
     categories: Integration & Delivery
     containerImage: quay.io/kuadrant/dns-operator:latest
-    createdAt: "2024-11-14T13:59:23Z"
+    createdAt: "2024-11-15T11:06:08Z"
     description: A Kubernetes Operator to manage the lifecycle of DNS resources
     operators.operatorframework.io/builder: operator-sdk-v1.33.0
     operators.operatorframework.io/project_layout: go.kubebuilder.io/v4

bundle/manifests/kuadrant.io_dnshealthcheckprobes.yaml

@@ -19,10 +19,6 @@ spec:
       jsonPath: .status.healthy
       name: Healthy
       type: boolean
-    - description: Last checked at.
-      jsonPath: .status.lastCheckedAt
-      name: Last Checked
-      type: date
     name: v1alpha1
     schema:
       openAPIV3Schema:

charts/dns-operator/templates/manifests.yaml

@@ -20,10 +20,6 @@ spec:
       jsonPath: .status.healthy
       name: Healthy
       type: boolean
-    - description: Last checked at.
-      jsonPath: .status.lastCheckedAt
-      name: Last Checked
-      type: date
     name: v1alpha1
     schema:
       openAPIV3Schema:

config/crd/bases/kuadrant.io_dnshealthcheckprobes.yaml

@@ -19,10 +19,6 @@ spec:
       jsonPath: .status.healthy
       name: Healthy
       type: boolean
-    - description: Last checked at.
-      jsonPath: .status.lastCheckedAt
-      name: Last Checked
-      type: date
     name: v1alpha1
     schema:
       openAPIV3Schema:

internal/controller/dnsrecord_healthchecks.go

@@ -28,7 +28,7 @@ func (r *DNSRecordReconciler) ReconcileHealthChecks(ctx context.Context, dnsReco
 
 	// Probes enabled but no health check spec yet. Nothing to do
 	if dnsRecord.Spec.HealthCheck == nil {
-		return nil
+		return r.DeleteHealthChecks(ctx, dnsRecord)
 	}
 
 	// we don't support probes for wildcard hosts

internal/controller/dnsrecord_healthchecks_test.go

@@ -167,6 +167,46 @@ var _ = Describe("DNSRecordReconciler_HealthChecks", func() {
 		}, TestTimeoutMedium, time.Second).Should(Succeed())
 	})
 
+	It("Should create valid probe CRs and remove them when definition removed", func() {
+		//Create default test dnsrecord
+		Expect(k8sClient.Create(ctx, dnsRecord)).To(Succeed())
+
+		By("Validating created probes")
+		Eventually(func(g Gomega) {
+			probes := &v1alpha1.DNSHealthCheckProbeList{}
+
+			g.Expect(k8sClient.List(ctx, probes, &client.ListOptions{
+				LabelSelector: labels.SelectorFromSet(map[string]string{
+					ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord),
+				}),
+				Namespace: dnsRecord.Namespace,
+			})).To(Succeed())
+
+			g.Expect(probes.Items).To(HaveLen(2))
+
+		}, TestTimeoutMedium, time.Second).Should(Succeed())
+
+		By("updating the DNSRecord and and ensuring the healthcheck is removed")
+		Eventually(func(g Gomega) {
+			recordCopy := &v1alpha1.DNSRecord{}
+			g.Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), recordCopy)).
+				Should(Succeed())
+			recordCopy.Spec.HealthCheck = nil
+			g.Expect(k8sClient.Update(ctx, recordCopy)).
+				Should(Succeed())
+			probes := &v1alpha1.DNSHealthCheckProbeList{}
+
+			g.Expect(k8sClient.List(ctx, probes, &client.ListOptions{
+				LabelSelector: labels.SelectorFromSet(map[string]string{
+					ProbeOwnerLabel: BuildOwnerLabelValue(dnsRecord),
+				}),
+				Namespace: dnsRecord.Namespace,
+			})).To(Succeed())
+
+			g.Expect(probes.Items).To(HaveLen(0))
+		}, TestTimeoutMedium, time.Second).Should(Succeed())
+	})
+
 	It("Should create valid probe CRs with default values", func() {
 		//Create test dnsrecord with nils for optional fields
 		dnsRecord.Spec.HealthCheck = &v1alpha1.HealthCheckSpec{
@@ -495,8 +535,10 @@ var _ = Describe("DNSRecordReconciler_HealthChecks", func() {
 
 		By("Remove health spec")
 		Eventually(func(g Gomega) {
-			dnsRecord.Spec.HealthCheck = nil
-			g.Expect(k8sClient.Update(ctx, dnsRecord)).To(Succeed())
+			Expect(k8sClient.Get(ctx, client.ObjectKeyFromObject(dnsRecord), dnsRecord)).To(Succeed())
+			cpRecord := dnsRecord.DeepCopy()
+			cpRecord.Spec.HealthCheck = nil
+			g.Expect(k8sClient.Update(ctx, cpRecord)).To(Succeed())
 		}, TestTimeoutMedium, time.Second).Should(Succeed())
 
 		// we don't remove EPs if this leads to empty EPs