From 6b7382748a7dc3d4b119f888462c57b3da954d81 Mon Sep 17 00:00:00 2001 From: Yi Tao Date: Mon, 9 Jan 2023 17:30:17 +0800 Subject: [PATCH 1/3] fix: only check if condition contained in httproute --- .../controllers/gateway/httproute_controller.go | 11 ++++------- internal/controllers/gateway/route_utils.go | 14 ++++++++++++++ 2 files changed, 18 insertions(+), 7 deletions(-) diff --git a/internal/controllers/gateway/httproute_controller.go b/internal/controllers/gateway/httproute_controller.go index 2d15f1c02b..d59f62dee3 100644 --- a/internal/controllers/gateway/httproute_controller.go +++ b/internal/controllers/gateway/httproute_controller.go @@ -435,13 +435,10 @@ func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Cont // if the reference already exists and doesn't require any changes // then just leave it alone. if existingGatewayParentStatus, exists := parentStatuses[key]; exists { - // fake the time of the existing status as this wont be equal - for i := range existingGatewayParentStatus.Conditions { - existingGatewayParentStatus.Conditions[i].LastTransitionTime = gatewayParentStatus.Conditions[0].LastTransitionTime - } - - // other than the condition timestamps, check if the statuses are equal - if reflect.DeepEqual(existingGatewayParentStatus, gatewayParentStatus) { + // check if the parentRef and controllerName are equal, and expected condition presents in conditions + if reflect.DeepEqual(existingGatewayParentStatus.ParentRef, gatewayParentStatus.ParentRef) && + existingGatewayParentStatus.ControllerName == gatewayParentStatus.ControllerName && + containsCondition(existingGatewayParentStatus.Conditions, gatewayParentStatus.Conditions[0]) { continue } } diff --git a/internal/controllers/gateway/route_utils.go b/internal/controllers/gateway/route_utils.go index a88b5be2e8..4c3b6f4a88 100644 --- a/internal/controllers/gateway/route_utils.go +++ b/internal/controllers/gateway/route_utils.go @@ -630,3 +630,17 @@ func isHTTPReferenceGranted(grantSpec gatewayv1alpha2.ReferenceGrantSpec, backen } return false } + +// containsCondition returns true if a condition with same type, status, reason and message +// of expectedCondition in conditions +func containsCondition(conditions []metav1.Condition, expectedCondition metav1.Condition) bool { + for _, condition := range conditions { + if condition.Type == expectedCondition.Type && + condition.Status == expectedCondition.Status && + condition.Reason == expectedCondition.Reason && + condition.Message == expectedCondition.Message { + return true + } + } + return false +} From 66c8251fed281a50753494656026d00d9713965c Mon Sep 17 00:00:00 2001 From: Yi Tao Date: Tue, 10 Jan 2023 00:32:05 +0800 Subject: [PATCH 2/3] use lo.ContainsBy to match conditions --- .../gateway/httproute_controller.go | 6 ++++-- internal/controllers/gateway/route_utils.go | 19 +++++++------------ 2 files changed, 11 insertions(+), 14 deletions(-) diff --git a/internal/controllers/gateway/httproute_controller.go b/internal/controllers/gateway/httproute_controller.go index d59f62dee3..d3ff29533b 100644 --- a/internal/controllers/gateway/httproute_controller.go +++ b/internal/controllers/gateway/httproute_controller.go @@ -435,10 +435,12 @@ func (r *HTTPRouteReconciler) ensureGatewayReferenceStatusAdded(ctx context.Cont // if the reference already exists and doesn't require any changes // then just leave it alone. if existingGatewayParentStatus, exists := parentStatuses[key]; exists { - // check if the parentRef and controllerName are equal, and expected condition presents in conditions + // check if the parentRef and controllerName are equal, and whether the new condition is present in existing conditions if reflect.DeepEqual(existingGatewayParentStatus.ParentRef, gatewayParentStatus.ParentRef) && existingGatewayParentStatus.ControllerName == gatewayParentStatus.ControllerName && - containsCondition(existingGatewayParentStatus.Conditions, gatewayParentStatus.Conditions[0]) { + lo.ContainsBy(existingGatewayParentStatus.Conditions, func(condition metav1.Condition) bool { + return sameCondition(gatewayParentStatus.Conditions[0], condition) + }) { continue } } diff --git a/internal/controllers/gateway/route_utils.go b/internal/controllers/gateway/route_utils.go index 4c3b6f4a88..431f7a6bcc 100644 --- a/internal/controllers/gateway/route_utils.go +++ b/internal/controllers/gateway/route_utils.go @@ -631,16 +631,11 @@ func isHTTPReferenceGranted(grantSpec gatewayv1alpha2.ReferenceGrantSpec, backen return false } -// containsCondition returns true if a condition with same type, status, reason and message -// of expectedCondition in conditions -func containsCondition(conditions []metav1.Condition, expectedCondition metav1.Condition) bool { - for _, condition := range conditions { - if condition.Type == expectedCondition.Type && - condition.Status == expectedCondition.Status && - condition.Reason == expectedCondition.Reason && - condition.Message == expectedCondition.Message { - return true - } - } - return false +// sameCondition returns true if the condition has the same type, status, reason and message +// with expectedCondition. +func sameCondition(expectedCondition metav1.Condition, condition metav1.Condition) bool { + return condition.Type == expectedCondition.Type && + condition.Status == expectedCondition.Status && + condition.Reason == expectedCondition.Reason && + condition.Message == expectedCondition.Message } From 5a87f79ec974e676ae7f309763b21786a394e655 Mon Sep 17 00:00:00 2001 From: Yi Tao Date: Tue, 10 Jan 2023 01:31:28 +0800 Subject: [PATCH 3/3] add CHANGELOG --- CHANGELOG.md | 3 +++ internal/controllers/gateway/route_utils.go | 13 ++++++------- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d05ead1cf..55832b5ca4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -71,6 +71,9 @@ Adding a new version? You'll need three changes: - Disabled non-functioning mesh reporting when `--watch-namespaces` flag set. [#3336](https://github.com/Kong/kubernetes-ingress-controller/pull/3336) +- Fixed the duplicate update of status of `HTTPRoute` caused by incorrect check + of whether status is changed. + [#3346](https://github.com/Kong/kubernetes-ingress-controller/pull/3346) ### Deprecated diff --git a/internal/controllers/gateway/route_utils.go b/internal/controllers/gateway/route_utils.go index 431f7a6bcc..8327b038cb 100644 --- a/internal/controllers/gateway/route_utils.go +++ b/internal/controllers/gateway/route_utils.go @@ -631,11 +631,10 @@ func isHTTPReferenceGranted(grantSpec gatewayv1alpha2.ReferenceGrantSpec, backen return false } -// sameCondition returns true if the condition has the same type, status, reason and message -// with expectedCondition. -func sameCondition(expectedCondition metav1.Condition, condition metav1.Condition) bool { - return condition.Type == expectedCondition.Type && - condition.Status == expectedCondition.Status && - condition.Reason == expectedCondition.Reason && - condition.Message == expectedCondition.Message +// sameCondition returns true if the conditions in parameter has the same type, status, reason and message. +func sameCondition(a, b metav1.Condition) bool { + return a.Type == b.Type && + a.Status == b.Status && + a.Reason == b.Reason && + a.Message == b.Message }