bzp2010 commented on code in PR #2856:
URL: 
https://github.com/apache/apisix-ingress-controller/pull/2856#discussion_r3988660173


##########
api/v1alpha1/gatewayproxy_types.go:
##########
@@ -178,6 +179,19 @@ type GatewayProxy struct {
        // GatewayProxySpec defines configuration of gateway proxy instances,
        // including networking settings, global plugins, and plugin metadata.
        Spec GatewayProxySpec `json:"spec,omitempty"`
+
+       // Status defines the current state of Gateway Proxy.
+       //
+       // +kubebuilder:default={conditions: {{type: "DataPlaneAvailable", 
status: "Unknown", reason:"Pending", message:"Waiting for controller", 
lastTransitionTime: "1970-01-01T00:00:00Z"}}}

Review Comment:
   Same fix as the other GatewayProxy status comment: the condition is now 
written unconditionally every round based on that round's actual outcome, with 
no dependency on any remembered history. A GatewayProxy that's always been 
healthy gets True the first time it syncs, and a restart can no longer leave a 
stale False stuck forever, since nothing carries over between rounds for 
GatewayProxy specifically.



##########
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{

Review Comment:
   recordFailedEndpointEvents now fetches the real GatewayProxy via the client 
before firing the Event, instead of building a bare Name/Namespace stub, so 
involvedObject carries the real UID and shows up under `kubectl describe 
gatewayproxy`.



-- 
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]

Reply via email to