ignore contention for secret events

Signed-off-by: chiragkyal <[email protected]>

pkg/router/controller/contention.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/client-go/tools/cache"
 
 	routev1 "github.com/openshift/api/route/v1"
@@ -36,6 +37,18 @@ const (
 	stateContended
 )
 
+// ignoreIngressConditionReason is a set of reasons for ingress conditions that should be ignored
+// when comparing if two route ingresses are the same. This is used to avoid false positives
+// mainly when the state of the ExternalCertificate is changed.
+var (
+	ignoreIngressConditionReason sets.String = sets.NewString(
+		ExtCrtStatusReasonValidationFailed,
+		ExtCrtStatusReasonSecretRecreated,
+		ExtCrtStatusReasonSecretUpdated,
+		ExtCrtStatusReasonSecretDeleted,
+	)
+)
+
 type trackerElement struct {
 	at    time.Time
 	state elementState
@@ -255,7 +268,7 @@ func ingressEqual(a, b *routev1.RouteIngress) bool {
 }
 
 // ingressConditionsEqual determines if the route ingress conditions are equal,
-// while ignoring LastTransitionTime.
+// while ignoring LastTransitionTime and any reason in ignoreIngressConditionReason.
 func ingressConditionsEqual(a, b []routev1.RouteIngressCondition) bool {
 	if len(a) != len(b) {
 		return false
@@ -279,8 +292,11 @@ func ingressConditionsEqual(a, b []routev1.RouteIngressCondition) bool {
 	return true
 }
 
-// conditionsEqual compares two RouteIngressConditions, ignoring LastTransitionTime.
+// conditionsEqual compares two RouteIngressConditions, ignoring LastTransitionTime and any reason in ignoreIngressConditionReason..
 func conditionsEqual(a, b *routev1.RouteIngressCondition) bool {
+	if ignoreIngressConditionReason.Has(a.Reason) || ignoreIngressConditionReason.Has(b.Reason) {
+		return true
+	}
 	return a.Type == b.Type &&
 		a.Status == b.Status &&
 		a.Reason == b.Reason &&

pkg/router/controller/route_secret_manager.go

@@ -19,6 +19,14 @@ import (
 	"k8s.io/client-go/tools/cache"
 )
 
+const (
+	ExtCrtStatusReasonValidationFailed = "ExternalCertificateValidationFailed"
+	ExtCrtStatusReasonSecretRecreated  = "ExternalCertificateSecretRecreated"
+	ExtCrtStatusReasonSecretUpdated    = "ExternalCertificateSecretUpdated"
+	ExtCrtStatusReasonSecretDeleted    = "ExternalCertificateSecretDeleted"
+	ExtCrtStatusReasonGetFailed        = "ExternalCertificateGetFailed"
+)
+
 // RouteSecretManager implements the router.Plugin interface to register
 // or unregister route with secretManger if externalCertificate is used.
 // It also reads the referenced secret to update in-memory tls.Certificate and tls.Key
@@ -269,7 +277,7 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
 				// The route should *remain* rejected until it's re-evaluated
 				// by all the plugins (including this plugin). Once passes, the route will become active again.
 				msg := fmt.Sprintf("secret %q recreated for route %q", secret.Name, key)
-				p.recorder.RecordRouteRejection(route, "ExternalCertificateSecretRecreated", msg)
+				p.recorder.RecordRouteRejection(route, ExtCrtStatusReasonSecretRecreated, msg)
 			}
 		},
 
@@ -291,9 +299,9 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
 			// - If the route is admitted (Admitted=True), record an update event.
 			// - If the route is not admitted, record a rejection event (keep it rejected).
 			if isRouteAdmittedTrue(route.DeepCopy()) {
-				p.recorder.RecordRouteUpdate(route, "ExternalCertificateSecretUpdated", msg)
+				p.recorder.RecordRouteUpdate(route, ExtCrtStatusReasonSecretUpdated, msg)
 			} else {
-				p.recorder.RecordRouteRejection(route, "ExternalCertificateSecretUpdated", msg)
+				p.recorder.RecordRouteRejection(route, ExtCrtStatusReasonSecretUpdated, msg)
 			}
 		},
 
@@ -314,7 +322,7 @@ func (p *RouteSecretManager) generateSecretHandler(namespace, routeName string)
 			}
 
 			// Reject this route
-			p.recorder.RecordRouteRejection(route, "ExternalCertificateSecretDeleted", msg)
+			p.recorder.RecordRouteRejection(route, ExtCrtStatusReasonSecretDeleted, msg)
 		},
 	}
 }
@@ -329,7 +337,7 @@ func (p *RouteSecretManager) validate(route *routev1.Route) error {
 	fldPath := field.NewPath("spec").Child("tls").Child("externalCertificate")
 	if err := routeapihelpers.ValidateTLSExternalCertificate(route, fldPath, p.sarClient, p.secretsGetter).ToAggregate(); err != nil {
 		log.Error(err, "skipping route due to invalid externalCertificate configuration", "namespace", route.Namespace, "route", route.Name)
-		p.recorder.RecordRouteRejection(route, "ExternalCertificateValidationFailed", err.Error())
+		p.recorder.RecordRouteRejection(route, ExtCrtStatusReasonValidationFailed, err.Error())
 		p.plugin.HandleRoute(watch.Deleted, route)
 		return err
 	}
@@ -345,7 +353,7 @@ func (p *RouteSecretManager) populateRouteTLSFromSecret(route *routev1.Route) er
 	secret, err := p.secretManager.GetSecret(context.TODO(), route.Namespace, route.Name)
 	if err != nil {
 		log.Error(err, "failed to get referenced secret")
-		p.recorder.RecordRouteRejection(route, "ExternalCertificateGetFailed", err.Error())
+		p.recorder.RecordRouteRejection(route, ExtCrtStatusReasonGetFailed, err.Error())
 		p.plugin.HandleRoute(watch.Deleted, route)
 		return err
 	}