Update policy status on TargetNotFound

Updates the policy status (DNSPolicy and TLSPolicy) when the target
resource is not found. The Ready condition is set to false with the
reason "TargetNotFound" and the message is set to the appropriae error
detaling the missing object.

```
{
  "lastTransitionTime": "2023-09-26T17:20:17Z",
  "message": "target not found : Gateway.gateway.networking.k8s.io \"prod-web-istio\" not found",
  "reason": "TargetNotFound",
  "status": "False",
  "type": "Ready"
}
```

pkg/_internal/conditions/conditions.go

@@ -1,6 +1,7 @@
 package conditions
 
 import (
+	"errors"
 	"fmt"
 
 	k8smeta "k8s.io/apimachinery/pkg/api/meta"
@@ -20,8 +21,12 @@ const (
 	PolicyReasonInvalid    ConditionReason = "Invalid"
 	PolicyReasonUnknown    ConditionReason = "Unknown"
 	PolicyReasonConflicted ConditionReason = "Conflicted"
+
+	PolicyReasonTargetNotFound ConditionReason = "TargetNotFound"
 )
 
+var ErrTargetNotFound = errors.New("target not found")
+
 func BuildPolicyAffectedCondition(conditionType ConditionType, policyObject runtime.Object, targetRef metav1.Object, reason ConditionReason, err error) metav1.Condition {
 
 	condition := metav1.Condition{

pkg/controllers/dnspolicy/dnspolicy_controller.go

@@ -93,8 +93,11 @@ func (r *DNSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		if !markedForDeletion {
 			if apierrors.IsNotFound(err) {
 				log.V(3).Info("Network object not found. Cleaning up")
-				err := r.deleteResources(ctx, dnsPolicy, nil)
-				return ctrl.Result{}, err
+				delResErr := r.deleteResources(ctx, dnsPolicy, nil)
+				if delResErr == nil {
+					delResErr = err
+				}
+				return r.reconcileStatus(ctx, dnsPolicy, fmt.Errorf("%w : %w", conditions.ErrTargetNotFound, delResErr))
 			}
 			return ctrl.Result{}, err
 		}
@@ -126,25 +129,13 @@ func (r *DNSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	specErr := r.reconcileResources(ctx, dnsPolicy, targetNetworkObject)
 
-	newStatus := r.calculateStatus(dnsPolicy, specErr)
-	dnsPolicy.Status = *newStatus
-
-	if !equality.Semantic.DeepEqual(previous.Status, dnsPolicy.Status) {
-		updateErr := r.Client().Status().Update(ctx, dnsPolicy)
-		if updateErr != nil {
-			// Ignore conflicts, resource might just be outdated.
-			if apierrors.IsConflict(updateErr) {
-				return ctrl.Result{Requeue: true}, nil
-			}
-			return ctrl.Result{}, updateErr
-		}
-	}
+	statusResult, statusErr := r.reconcileStatus(ctx, dnsPolicy, specErr)
 
 	if specErr != nil {
 		return ctrl.Result{}, specErr
 	}
 
-	return ctrl.Result{}, nil
+	return statusResult, statusErr
 }
 
 func (r *DNSPolicyReconciler) reconcileResources(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, targetNetworkObject client.Object) error {
@@ -237,6 +228,28 @@ func (r *DNSPolicyReconciler) deleteResources(ctx context.Context, dnsPolicy *v1
 	return r.updateGatewayCondition(ctx, metav1.Condition{Type: string(DNSPolicyAffected)}, gatewayDiffObj)
 }
 
+func (r *DNSPolicyReconciler) reconcileStatus(ctx context.Context, dnsPolicy *v1alpha1.DNSPolicy, specErr error) (ctrl.Result, error) {
+	newStatus := r.calculateStatus(dnsPolicy, specErr)
+
+	if !equality.Semantic.DeepEqual(newStatus, dnsPolicy.Status) {
+		dnsPolicy.Status = *newStatus
+		updateErr := r.Client().Status().Update(ctx, dnsPolicy)
+		if updateErr != nil {
+			// Ignore conflicts, resource might just be outdated.
+			if apierrors.IsConflict(updateErr) {
+				return ctrl.Result{Requeue: true}, nil
+			}
+			return ctrl.Result{}, updateErr
+		}
+	}
+
+	if errors.Is(specErr, conditions.ErrTargetNotFound) {
+		return ctrl.Result{Requeue: true}, nil
+	}
+
+	return ctrl.Result{}, nil
+}
+
 func (r *DNSPolicyReconciler) calculateStatus(dnsPolicy *v1alpha1.DNSPolicy, specErr error) *v1alpha1.DNSPolicyStatus {
 	newStatus := dnsPolicy.Status.DeepCopy()
 	if specErr != nil {
@@ -257,8 +270,12 @@ func (r *DNSPolicyReconciler) readyCondition(targetNetworkObjectectKind string,
 
 	if specErr != nil {
 		cond.Status = metav1.ConditionFalse
-		cond.Reason = "ReconciliationError"
 		cond.Message = specErr.Error()
+		cond.Reason = "ReconciliationError"
+
+		if errors.Is(specErr, conditions.ErrTargetNotFound) {
+			cond.Reason = string(conditions.PolicyReasonTargetNotFound)
+		}
 	}
 
 	return cond

pkg/controllers/tlspolicy/tlspolicy_controller.go

@@ -96,10 +96,12 @@ func (r *TLSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 		if !markedForDeletion {
 			if apierrors.IsNotFound(err) {
 				log.V(3).Info("Network object not found. Cleaning up")
-				err := r.deleteResources(ctx, tlsPolicy, nil)
-				if err != nil {
-					return ctrl.Result{}, err
+				delResErr := r.deleteResources(ctx, tlsPolicy, nil)
+				if delResErr == nil {
+					delResErr = err
 				}
+				return r.reconcileStatus(ctx, tlsPolicy, fmt.Errorf("%w: %w", conditions.ErrTargetNotFound, delResErr))
+
 			}
 			return ctrl.Result{}, err
 		}
@@ -128,25 +130,13 @@ func (r *TLSPolicyReconciler) Reconcile(ctx context.Context, req ctrl.Request) (
 
 	specErr := r.reconcileResources(ctx, tlsPolicy, targetNetworkObject)
 
-	newStatus := r.calculateStatus(tlsPolicy, specErr)
-	tlsPolicy.Status = *newStatus
-
-	if !equality.Semantic.DeepEqual(previous.Status, tlsPolicy.Status) {
-		updateErr := r.Client().Status().Update(ctx, tlsPolicy)
-		if updateErr != nil {
-			// Ignore conflicts, resource might just be outdated.
-			if apierrors.IsConflict(updateErr) {
-				return ctrl.Result{Requeue: true}, nil
-			}
-			return ctrl.Result{}, updateErr
-		}
-	}
+	statusResult, statusErr := r.reconcileStatus(ctx, tlsPolicy, specErr)
 
 	if specErr != nil {
 		return ctrl.Result{}, specErr
 	}
 
-	return ctrl.Result{}, nil
+	return statusResult, statusErr
 }
 
 func (r *TLSPolicyReconciler) reconcileResources(ctx context.Context, tlsPolicy *v1alpha1.TLSPolicy, targetNetworkObject client.Object) error {
@@ -226,6 +216,28 @@ func (r *TLSPolicyReconciler) deleteResources(ctx context.Context, tlsPolicy *v1
 	return r.updateGatewayCondition(ctx, metav1.Condition{Type: string(TLSPolicyAffected)}, gatewayDiffObj)
 }
 
+func (r *TLSPolicyReconciler) reconcileStatus(ctx context.Context, tlsPolicy *v1alpha1.TLSPolicy, specErr error) (ctrl.Result, error) {
+	newStatus := r.calculateStatus(tlsPolicy, specErr)
+
+	if !equality.Semantic.DeepEqual(newStatus, tlsPolicy.Status) {
+		tlsPolicy.Status = *newStatus
+		updateErr := r.Client().Status().Update(ctx, tlsPolicy)
+		if updateErr != nil {
+			// Ignore conflicts, resource might just be outdated.
+			if apierrors.IsConflict(updateErr) {
+				return ctrl.Result{Requeue: true}, nil
+			}
+			return ctrl.Result{}, updateErr
+		}
+	}
+
+	if errors.Is(specErr, conditions.ErrTargetNotFound) {
+		return ctrl.Result{Requeue: true}, nil
+	}
+
+	return ctrl.Result{}, nil
+}
+
 func (r *TLSPolicyReconciler) calculateStatus(tlsPolicy *v1alpha1.TLSPolicy, specErr error) *v1alpha1.TLSPolicyStatus {
 	newStatus := tlsPolicy.Status.DeepCopy()
 	if specErr != nil {
@@ -246,8 +258,12 @@ func (r *TLSPolicyReconciler) readyCondition(targetNetworkObjectectKind string,
 
 	if specErr != nil {
 		cond.Status = metav1.ConditionFalse
-		cond.Reason = "ReconciliationError"
 		cond.Message = specErr.Error()
+		cond.Reason = "ReconciliationError"
+
+		if errors.Is(specErr, conditions.ErrTargetNotFound) {
+			cond.Reason = string(conditions.PolicyReasonTargetNotFound)
+		}
 	}
 
 	return cond

test/integration/dnspolicy_controller_test.go

@@ -25,6 +25,7 @@ import (
 	. "github.com/Kuadrant/multicluster-gateway-controller/pkg/controllers/dnspolicy"
 	mgcgateway "github.com/Kuadrant/multicluster-gateway-controller/pkg/controllers/gateway"
 	"github.com/Kuadrant/multicluster-gateway-controller/pkg/dns"
+	. "github.com/Kuadrant/multicluster-gateway-controller/test/util"
 )
 
 func testBuildManagedZone(domainName, ns string) *v1alpha1.ManagedZone {
@@ -194,6 +195,69 @@ var _ = Describe("DNSPolicy", Ordered, func() {
 		Expect(err).ToNot(HaveOccurred())
 	})
 
+	Context("invalid target", func() {
+		var gateway *gatewayv1beta1.Gateway
+		var dnsPolicy *v1alpha1.DNSPolicy
+		gwClassName := "istio"
+
+		BeforeEach(func() {
+			dnsPolicy = testBuildDNSPolicyWithHealthCheck("test-dns-policy", "test-gateway", testNamespace, nil)
+			Expect(k8sClient.Create(ctx, dnsPolicy)).To(BeNil())
+			Eventually(func() error { //dns policy exists
+				return k8sClient.Get(ctx, client.ObjectKey{Name: dnsPolicy.Name, Namespace: dnsPolicy.Namespace}, dnsPolicy)
+			}, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			err := k8sClient.Delete(ctx, dnsPolicy)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("should have ready condition with status false and correct reason", func() {
+			Eventually(func() error {
+				if err := k8sClient.Get(ctx, client.ObjectKey{Name: dnsPolicy.Name, Namespace: dnsPolicy.Namespace}, dnsPolicy); err != nil {
+					return err
+				}
+				readyCond := meta.FindStatusCondition(dnsPolicy.Status.Conditions, string(conditions.ConditionTypeReady))
+				if readyCond == nil {
+					return fmt.Errorf("expected dnsPolicy to have %s condition, got none",
+						string(conditions.ConditionTypeReady))
+				}
+				if readyCond.Status != metav1.ConditionFalse {
+					return fmt.Errorf("expected dnsPolicy %s condition to have status %s, got %s",
+						string(conditions.ConditionTypeReady), metav1.ConditionFalse, readyCond.Status)
+				}
+				if readyCond.Reason != string(conditions.PolicyReasonTargetNotFound) {
+					return fmt.Errorf("expected dnsPolicy %s condition to have reason %s, got %s",
+						string(conditions.ConditionTypeReady), string(conditions.PolicyReasonTargetNotFound), readyCond.Reason)
+				}
+				return nil
+			}, time.Second*15, time.Second).Should(BeNil())
+		})
+
+		It("should have ready condition with status true", func() {
+			By("creating a valid Gateway")
+
+			gateway = NewTestGateway("test-gateway", gwClassName, testNamespace).
+				WithHTTPListener("test.example.com").Gateway
+			Expect(k8sClient.Create(ctx, gateway)).To(BeNil())
+			Eventually(func() error { //gateway exists
+				return k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway)
+			}, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred())
+
+			Eventually(func() error {
+				if err := k8sClient.Get(ctx, client.ObjectKey{Name: dnsPolicy.Name, Namespace: dnsPolicy.Namespace}, dnsPolicy); err != nil {
+					return err
+				}
+				if !meta.IsStatusConditionTrue(dnsPolicy.Status.Conditions, string(conditions.ConditionTypeReady)) {
+					return fmt.Errorf("expected dnsPolicy %s condition to have status %s ", string(conditions.ConditionTypeReady), metav1.ConditionTrue)
+				}
+				return nil
+			}, time.Second*15, time.Second).Should(BeNil())
+		})
+
+	})
+
 	Context("gateway placed", func() {
 		var gateway *gatewayv1beta1.Gateway
 		var lbHash, dnsRecordName, wildcardDNSRecordName string
@@ -1330,9 +1394,11 @@ var _ = Describe("DNSPolicy", Ordered, func() {
 				}
 
 				Eventually(func() error {
+					err = k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway)
+					Expect(err).NotTo(HaveOccurred())
 					gateway.Spec.Listeners = append(gateway.Spec.Listeners, otherListener)
 					return k8sClient.Update(ctx, gateway)
-				}).Should(BeNil())
+				}, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred())
 
 				probeList := &v1alpha1.DNSHealthCheckProbeList{}
 				Eventually(func() error {

test/integration/tlspolicy_controller_test.go

@@ -12,6 +12,7 @@ import (
 	. "github.com/onsi/gomega"
 
 	"k8s.io/apimachinery/pkg/api/meta"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	logf "sigs.k8s.io/controller-runtime/pkg/log"
 	"sigs.k8s.io/controller-runtime/pkg/log/zap"
@@ -73,6 +74,71 @@ var _ = Describe("TLSPolicy", Ordered, func() {
 		Expect(err).ToNot(HaveOccurred())
 	})
 
+	Context("invalid target", func() {
+		var gateway *gatewayv1beta1.Gateway
+		var tlsPolicy *v1alpha1.TLSPolicy
+		gwClassName := "istio"
+
+		BeforeEach(func() {
+			tlsPolicy = NewTestTLSPolicy("test-tls-policy", testNamespace).
+				WithTargetGateway("test-gateway").
+				WithIssuer("testissuer", certmanv1.IssuerKind, "cert-manager.io").TLSPolicy
+			Expect(k8sClient.Create(ctx, tlsPolicy)).To(BeNil())
+			Eventually(func() error { //tls policy exists
+				return k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy)
+			}, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred())
+		})
+
+		AfterEach(func() {
+			err := k8sClient.Delete(ctx, tlsPolicy)
+			Expect(err).ToNot(HaveOccurred())
+		})
+
+		It("should have ready condition with status false and correct reason", func() {
+			Eventually(func() error {
+				if err := k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy); err != nil {
+					return err
+				}
+				readyCond := meta.FindStatusCondition(tlsPolicy.Status.Conditions, string(conditions.ConditionTypeReady))
+				if readyCond == nil {
+					return fmt.Errorf("expected tlsPolicy to have %s condition, got none",
+						string(conditions.ConditionTypeReady))
+				}
+				if readyCond.Status != metav1.ConditionFalse {
+					return fmt.Errorf("expected tlsPolicy %s condition to have status %s, got %s",
+						string(conditions.ConditionTypeReady), metav1.ConditionFalse, readyCond.Status)
+				}
+				if readyCond.Reason != string(conditions.PolicyReasonTargetNotFound) {
+					return fmt.Errorf("expected tlsPolicy %s condition to have reason %s, got %s",
+						string(conditions.ConditionTypeReady), string(conditions.PolicyReasonTargetNotFound), readyCond.Reason)
+				}
+				return nil
+			}, time.Second*15, time.Second).Should(BeNil())
+		})
+
+		It("should have ready condition with status true", func() {
+			By("creating a valid Gateway")
+
+			gateway = NewTestGateway("test-gateway", gwClassName, testNamespace).
+				WithHTTPListener("test.example.com").Gateway
+			Expect(k8sClient.Create(ctx, gateway)).To(BeNil())
+			Eventually(func() error { //gateway exists
+				return k8sClient.Get(ctx, client.ObjectKey{Name: gateway.Name, Namespace: gateway.Namespace}, gateway)
+			}, TestTimeoutMedium, TestRetryIntervalMedium).ShouldNot(HaveOccurred())
+
+			Eventually(func() error {
+				if err := k8sClient.Get(ctx, client.ObjectKey{Name: tlsPolicy.Name, Namespace: tlsPolicy.Namespace}, tlsPolicy); err != nil {
+					return err
+				}
+				if !meta.IsStatusConditionTrue(tlsPolicy.Status.Conditions, string(conditions.ConditionTypeReady)) {
+					return fmt.Errorf("expected tlsPolicy %s condition to have status %s ", string(conditions.ConditionTypeReady), metav1.ConditionTrue)
+				}
+				return nil
+			}, time.Second*15, time.Second).Should(BeNil())
+		})
+
+	})
+
 	Context("istio gateway", func() {
 		var gateway *gatewayv1beta1.Gateway
 		var tlsPolicy *v1alpha1.TLSPolicy