diff --git a/pkg/router/controller/contention.go b/pkg/router/controller/contention.go index 0f159bf73..b47ba4e48 100644 --- a/pkg/router/controller/contention.go +++ b/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 && diff --git a/pkg/router/controller/route_secret_manager.go b/pkg/router/controller/route_secret_manager.go index b6c819ae3..b37ade310 100644 --- a/pkg/router/controller/route_secret_manager.go +++ b/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 }