AlinsRan commented on code in PR #2856:
URL:
https://github.com/apache/apisix-ingress-controller/pull/2856#discussion_r3946920565
##########
internal/provider/apisix/status.go:
##########
@@ -42,32 +55,101 @@ import (
func (d *apisixProvider) handleStatusUpdate(statusUpdateMap
map[types.NamespacedNameKind][]string) {
// Mark all resources in the current failure set as failed.
for nnk, msgs := range statusUpdateMap {
- d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
- apiv2.ConditionReasonSyncFailed,
- false,
- 0,
- strings.Join(msgs, "; "),
- ))
+ d.updateStatus(nnk, failureCondition(nnk, strings.Join(msgs, ";
")))
}
// Mark resources that exist only in the previous failure history as
successful.
for nnk := range d.statusUpdateMap {
if _, ok := statusUpdateMap[nnk]; !ok {
- d.updateStatus(nnk, cutils.NewConditionTypeAccepted(
- apiv2.ConditionReasonAccepted,
- true,
- 0,
- "",
- ))
+ d.updateStatus(nnk, successCondition(nnk))
+ if nnk.Kind == types.KindGatewayProxy {
+ d.recordGatewayProxyRecoveredEvent(nnk)
+ }
}
}
// Update the failure history with the current failure set.
d.statusUpdateMap = statusUpdateMap
}
+// failureCondition and successCondition pick which condition a
NamespacedNameKind
+// gets: GatewayProxyConditionDataPlaneAvailable for a GatewayProxy, the
existing
+// Accepted/SyncFailed condition for everything else.
+func failureCondition(nnk types.NamespacedNameKind, msg string)
metav1.Condition {
+ if nnk.Kind == types.KindGatewayProxy {
+ return newGatewayProxyDataPlaneAvailableCondition(false,
GatewayProxyReasonDataPlaneInstanceUnavailable, msg)
+ }
+ return cutils.NewConditionTypeAccepted(apiv2.ConditionReasonSyncFailed,
false, 0, msg)
+}
+
+func successCondition(nnk types.NamespacedNameKind) metav1.Condition {
+ if nnk.Kind == types.KindGatewayProxy {
+ return newGatewayProxyDataPlaneAvailableCondition(true,
GatewayProxyReasonDataPlaneAvailable, "")
+ }
+ return cutils.NewConditionTypeAccepted(apiv2.ConditionReasonAccepted,
true, 0, "")
+}
+
+func newGatewayProxyDataPlaneAvailableCondition(available bool, reason, msg
string) metav1.Condition {
+ conditionStatus := metav1.ConditionFalse
+ if available {
+ conditionStatus = metav1.ConditionTrue
+ }
+ return metav1.Condition{
+ Type: GatewayProxyConditionDataPlaneAvailable,
+ Status: conditionStatus,
+ LastTransitionTime: metav1.Now(),
+ Reason: reason,
+ Message: cutils.TruncateConditionMessage(msg),
+ }
+}
+
+// recordGatewayProxyRecoveredEvent fires once a GatewayProxy leaves the
failure
+// history entirely, i.e. every instance took the last sync.
+func (d *apisixProvider) recordGatewayProxyRecoveredEvent(nnk
types.NamespacedNameKind) {
+ if d.EventRecorder == nil {
+ return
+ }
+ gatewayProxy := &apiv1alpha1.GatewayProxy{
+ ObjectMeta: metav1.ObjectMeta{Name: nnk.Name, Namespace:
nnk.Namespace},
+ }
+ d.EventRecorder.Event(gatewayProxy, corev1.EventTypeNormal,
GatewayProxyReasonDataPlaneAvailable,
+ "every data plane instance took the last sync")
+}
+
+// recordFailedEndpointEvents fires one Warning event per failed
EndpointStatus entry,
+// so each instance's own failure history (when it started, how often) is
visible on
+// its own, not folded into everyone else's.
+func (d *apisixProvider) recordFailedEndpointEvents(nnk
types.NamespacedNameKind, endpoints []adctypes.EndpointStatus) {
+ if d.EventRecorder == nil {
+ return
+ }
+ gatewayProxy := &apiv1alpha1.GatewayProxy{
+ ObjectMeta: metav1.ObjectMeta{Name: nnk.Name, Namespace:
nnk.Namespace},
+ }
+ for _, ep := range endpoints {
+ if ep.Success {
+ continue
+ }
+ d.EventRecorder.Event(gatewayProxy, corev1.EventTypeWarning,
GatewayProxyReasonDataPlaneInstanceUnavailable,
+ fmt.Sprintf("%s: %s", ep.Server, ep.Reason))
+ }
+}
+
//nolint:gocyclo
func (d *apisixProvider) updateStatus(nnk types.NamespacedNameKind, condition
metav1.Condition) {
switch nnk.Kind {
+ case types.KindGatewayProxy:
+ // Unlike the route kinds below, the condition lands on the
GatewayProxy's own
+ // top-level Status.Conditions, not on a per-parent entry.
+ d.updater.Update(status.Update{
Review Comment:
This routes a new Kind through `UpdateHandler` that `statusEqual` doesn't
know about. `internal/controller/status/updater.go:179` has no
`*v1alpha1.GatewayProxy` case, so it falls through to `default: return false` —
two identical conditions never compare equal, and the dedup shortcut at
`updater.go:113` never fires.
Verified with a probe test (control included):
```
statusEqual(GatewayProxy{c}, GatewayProxy{c}) == false // no case ->
default
statusEqual(Consumer{}, Consumer{}) == true // has a case
```
`newGatewayProxyDataPlaneAvailableCondition` also stamps a fresh
`metav1.Now()` every cycle, so the serialized object differs too and the
apiserver's no-op-write optimization doesn't save us either. Every sync issues
a real `Status().Update()` and bumps `resourceVersion`.
That write then fans out. `gateway_controller.go` is the only one of the 13
GatewayProxy watchers without a `WithEventFilter`, and its
`Watches(&v1alpha1.GatewayProxy{}, ...)` carries no predicate either, so a
status-only update enqueues every Gateway whose `parametersRef` points here.
`Reconcile` unconditionally calls `Provider.Update`
(`gateway_controller.go:259`), which reaches `defer d.syncNotify()`
(`provider.go:169`). `syncNotify` is an independent wake source in the sync
loop's `select`, so it bypasses the retrier completely:
```
GatewayProxy status write -> Gateway reconcile -> syncNotify -> sync fails
-> status write -> ...
```
While an endpoint stays down, this turns what should be a backoff
(`RetryMaxDelay` 1000s, or the 1h ticker) into a continuous full resync:
healthy instances get the entire config rewritten every cycle, and `failed to
run http sync for server` floods the log. It stops on its own once the data
plane recovers, but not before.
Adding a `case *v1alpha1.GatewayProxy` to `statusEqual` breaks the cycle at
the source — `cmpIgnoreLastTT` already ignores `LastTransitionTime`, so an
unchanged condition would be skipped. Giving the `gateway_controller`
GatewayProxy watch a `GenerationChangedPredicate` would be a good second line
of defence, though that one is pre-existing rather than introduced here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
