Skip to content

Commit 4315965

Browse files
edenreichedenreich
committed
fix: Simplify reconcileGatewayStatus and improve URL update logic
Reconcile the Update status once with max 3 retries instead of fetch and updating two times - once for the providers summary and once for url status. Signed-off-by: Eden Reich <eden.reich@gmail.com>
1 parent 188e30a commit 4315965

1 file changed

Lines changed: 60 additions & 41 deletions

File tree

‎internal/controller/gateway_controller.go‎

Lines changed: 60 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -110,21 +110,15 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
110110
return ctrl.Result{}, err
111111
}
112112

113-
err = r.reconcileGatewayStatus(ctx, gateway)
114-
if err != nil {
115-
logger.Error(err, "Failed to reconcile Gateway status")
116-
return ctrl.Result{}, err
117-
}
118-
119113
err = r.reconcileHPA(ctx, gateway, deployment)
120114
if err != nil {
121115
logger.Error(err, "Failed to reconcile HPA")
122116
return ctrl.Result{}, err
123117
}
124118

125-
err = r.updateProvidersSummary(ctx, gateway)
119+
err = r.reconcileGatewayStatus(ctx, gateway)
126120
if err != nil {
127-
logger.Error(err, "Failed to update ProvidersSummary")
121+
logger.Error(err, "Failed to reconcile Gateway status")
128122
return ctrl.Result{}, err
129123
}
130124

@@ -135,50 +129,75 @@ func (r *GatewayReconciler) Reconcile(ctx context.Context, req ctrl.Request) (ct
135129
return ctrl.Result{}, nil
136130
}
137131

132+
// reconcileGatewayStatus updates the Gateway status with the current URL
138133
func (r *GatewayReconciler) reconcileGatewayStatus(ctx context.Context, gateway *corev1alpha1.Gateway) error {
139134
logger := log.FromContext(ctx)
140135

141-
ingressSpec := gateway.Spec.Ingress
142-
var (
143-
host string
144-
scheme string
145-
)
136+
err := r.updateProvidersSummary(ctx, gateway)
137+
if err != nil {
138+
logger.Error(err, "Failed to update ProvidersSummary")
139+
return err
140+
}
146141

147-
if ingressSpec != nil && ingressSpec.Enabled {
148-
ingress := &networkingv1.Ingress{}
149-
err := r.Get(ctx, types.NamespacedName{Name: gateway.Name, Namespace: gateway.Namespace}, ingress)
150-
if err != nil || len(ingress.Spec.Rules) == 0 {
151-
logger.V(1).Info("ingress not found or has no rules, falling back to service FQDN", "gateway", gateway.Name)
152-
host = fmt.Sprintf("%s.%s.svc.cluster.local", gateway.Name, gateway.Namespace)
142+
getHostAndScheme := func(gw *corev1alpha1.Gateway) (string, string) {
143+
ingressSpec := gw.Spec.Ingress
144+
var host, scheme string
145+
146+
if ingressSpec != nil && ingressSpec.Enabled {
147+
ingress := &networkingv1.Ingress{}
148+
err := r.Get(ctx, types.NamespacedName{Name: gw.Name, Namespace: gw.Namespace}, ingress)
149+
if err != nil || len(ingress.Spec.Rules) == 0 {
150+
logger.V(1).Info("ingress not found or has no rules, falling back to service fqdn", "gateway", gw.Name)
151+
host = fmt.Sprintf("%s.%s.svc.cluster.local", gw.Name, gw.Namespace)
152+
} else {
153+
host = ingress.Spec.Rules[0].Host
154+
}
155+
switch {
156+
case ingressSpec.TLS != nil && ingressSpec.TLS.Enabled:
157+
scheme = "https"
158+
default:
159+
scheme = "http"
160+
}
153161
} else {
154-
host = ingress.Spec.Rules[0].Host
162+
host = fmt.Sprintf("%s.%s.svc.cluster.local", gw.Name, gw.Namespace)
163+
switch {
164+
case gw.Spec.Server != nil && gw.Spec.Server.TLS != nil && gw.Spec.Server.TLS.Enabled:
165+
scheme = "https"
166+
default:
167+
scheme = "http"
168+
}
155169
}
156-
if ingressSpec.TLS != nil && ingressSpec.TLS.Enabled {
157-
scheme = "https"
158-
} else {
159-
scheme = "http"
170+
return host, scheme
171+
}
172+
173+
const maxRetries = 3
174+
var lastErr error
175+
176+
for i := 0; i < maxRetries; i++ {
177+
latest := &corev1alpha1.Gateway{}
178+
if err := r.Get(ctx, client.ObjectKeyFromObject(gateway), latest); err != nil {
179+
return err
160180
}
161-
} else {
162-
host = fmt.Sprintf("%s.%s.svc.cluster.local", gateway.Name, gateway.Namespace)
163-
if gateway.Spec.Server != nil && gateway.Spec.Server.TLS != nil && gateway.Spec.Server.TLS.Enabled {
164-
scheme = "https"
165-
} else {
166-
scheme = "http"
181+
182+
host, scheme := getHostAndScheme(latest)
183+
newURL := fmt.Sprintf("%s://%s", scheme, host)
184+
if latest.Status.URL == newURL || newURL == "" {
185+
return nil
167186
}
168-
}
169187

170-
newURL := fmt.Sprintf("%s://%s", scheme, host)
171-
if gateway.Status.URL == newURL || newURL == "" {
188+
latest.Status.URL = newURL
189+
if err := r.Status().Update(ctx, latest); err != nil {
190+
if errors.IsConflict(err) {
191+
lastErr = err
192+
continue
193+
}
194+
logger.Error(err, "failed to update gateway url in status")
195+
return err
196+
}
197+
logger.V(1).Info("updated gateway url in status", "gateway", latest.Name, "url", newURL)
172198
return nil
173199
}
174-
175-
gateway.Status.URL = newURL
176-
if err := r.Status().Update(ctx, gateway); err != nil {
177-
logger.Error(err, "failed to update gateway url in status")
178-
return err
179-
}
180-
logger.V(1).Info("updated gateway url in status", "gateway", gateway.Name, "url", newURL)
181-
return nil
200+
return lastErr
182201
}
183202

184203
func (r *GatewayReconciler) updateProvidersSummary(ctx context.Context, gateway *corev1alpha1.Gateway) error {

0 commit comments

Comments
 (0)