This is an automated email from the ASF dual-hosted git repository.
AlinsRan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/apisix-ingress-controller.git
The following commit(s) were added to refs/heads/master by this push:
new 6b95e2aa fix: surface Gateway API route translation failures in status
(#2840) (#2898)
6b95e2aa is described below
commit 6b95e2aa9d598ec09814cb982d18880c94c98e3b
Author: 玉升 <[email protected]>
AuthorDate: Mon Oct 12 15:32:09 2026 +0800
fix: surface Gateway API route translation failures in status (#2840)
(#2898)
Signed-off-by: yusheng.wang <[email protected]>
---
.../controller/apisixconsumer_controller_test.go | 5 +-
internal/controller/grpcroute_controller.go | 17 ++--
...etract_test.go => grpcroute_controller_test.go} | 90 ++++++----------------
internal/controller/httproute_controller.go | 26 ++++---
.../httproute_controller_retract_test.go | 39 +++++++++-
internal/controller/tcproute_controller.go | 16 ++--
internal/controller/tlsroute_controller.go | 16 ++--
internal/controller/udproute_controller.go | 16 ++--
8 files changed, 126 insertions(+), 99 deletions(-)
diff --git a/internal/controller/apisixconsumer_controller_test.go
b/internal/controller/apisixconsumer_controller_test.go
index 12cde685..d85866cc 100644
--- a/internal/controller/apisixconsumer_controller_test.go
+++ b/internal/controller/apisixconsumer_controller_test.go
@@ -46,18 +46,19 @@ import (
const testConsumerNamespace = "default"
// recordingProvider records the objects passed to Update and Delete, and can
be
-// told to fail a delete. Shared by the reconciler tests in this package.
+// told to fail an update or a delete. Shared by the reconciler tests in this
package.
type recordingProvider struct {
updated int
deleted []types.NamespacedName
deleteErr error
+ updateErr error
}
func (p *recordingProvider) Register(string, *http.ServeMux) {}
func (p *recordingProvider) Update(context.Context,
*provider.TranslateContext, client.Object) error {
p.updated++
- return nil
+ return p.updateErr
}
func (p *recordingProvider) Delete(_ context.Context, obj client.Object) error
{
diff --git a/internal/controller/grpcroute_controller.go
b/internal/controller/grpcroute_controller.go
index 5129af89..1218afcb 100644
--- a/internal/controller/grpcroute_controller.go
+++ b/internal/controller/grpcroute_controller.go
@@ -256,6 +256,17 @@ func (r *GRPCRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
ProcessBackendTrafficPolicy(r.Client, r.Log, tctx)
+ // Translate and sync before writing Accepted=True. A translation or
+ // buildConfig failure from Provider.Update used to leave a green status
+ // and only requeue, matching neither Ingress nor ApisixGlobalRule.
+ var syncErr error
+ if isRouteAccepted(gateways) && err == nil {
+ if syncErr = r.Provider.Update(ctx, tctx, gr); syncErr != nil {
+ acceptStatus.status = false
+ acceptStatus.msg = syncErr.Error()
+ }
+ }
+
// TODO: diff the old and new status
gr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0,
len(gateways))
for _, gateway := range gateways {
@@ -287,11 +298,7 @@ func (r *GRPCRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
UpdateStatus(r.Updater, r.Log, tctx)
if isRouteAccepted(gateways) && err == nil {
- routeToUpdate := gr
- if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
- return ctrl.Result{}, err
- }
- return ctrl.Result{}, nil
+ return ctrl.Result{}, syncErr
}
// The route still resolves to one of our Gateways but no parent
accepts it any
diff --git a/internal/controller/httproute_controller_retract_test.go
b/internal/controller/grpcroute_controller_test.go
similarity index 54%
copy from internal/controller/httproute_controller_retract_test.go
copy to internal/controller/grpcroute_controller_test.go
index 857c2fa5..35e7e869 100644
--- a/internal/controller/httproute_controller_retract_test.go
+++ b/internal/controller/grpcroute_controller_test.go
@@ -40,20 +40,7 @@ import (
"github.com/apache/apisix-ingress-controller/internal/manager/readiness"
)
-const (
- retractGatewayNamespace = "infra"
- retractRouteNamespace = "tenant"
- retractRouteName = "route"
-)
-
-// newHTTPRouteRetractFixture builds a Gateway of our class in
retractGatewayNamespace
-// and an HTTPRoute in retractRouteNamespace that names it as its parent. from
-// controls the listener's allowedRoutes, which is what revoking
cross-namespace
-// access changes.
-func newHTTPRouteRetractFixture(
- t *testing.T,
- from gatewayv1.FromNamespaces,
-) (*HTTPRouteReconciler, *recordingProvider, *recordingUpdater) {
+func newGRPCRouteFixture(t *testing.T) (*GRPCRouteReconciler,
*recordingProvider, *recordingUpdater) {
t.Helper()
scheme := runtime.NewScheme()
@@ -61,6 +48,7 @@ func newHTTPRouteRetractFixture(
require.NoError(t, gatewayv1.Install(scheme))
require.NoError(t, v1alpha1.AddToScheme(scheme))
+ from := gatewayv1.NamespacesFromAll
gatewayClass := &gatewayv1.GatewayClass{
ObjectMeta: metav1.ObjectMeta{Name: "apisix"},
Spec: gatewayv1.GatewayClassSpec{
@@ -81,13 +69,9 @@ func newHTTPRouteRetractFixture(
}},
},
}
- route := &gatewayv1.HTTPRoute{
+ route := &gatewayv1.GRPCRoute{
ObjectMeta: metav1.ObjectMeta{Namespace: retractRouteNamespace,
Name: retractRouteName},
- Spec: gatewayv1.HTTPRouteSpec{
- // A route with hostnames of its own is what makes
filterHostnames fail
- // once no listener matches, which is how a route-wide
reason used to
- // overwrite the parent's own.
- Hostnames: []gatewayv1.Hostname{"tenant.example"},
+ Spec: gatewayv1.GRPCRouteSpec{
CommonRouteSpec: gatewayv1.CommonRouteSpec{
ParentRefs: []gatewayv1.ParentReference{{
Name:
gatewayv1.ObjectName(gateway.Name),
@@ -107,7 +91,7 @@ func newHTTPRouteRetractFixture(
prov := &recordingProvider{}
updater := &recordingUpdater{}
- return &HTTPRouteReconciler{
+ return &GRPCRouteReconciler{
Client: cli,
Scheme: scheme,
Log: logr.Discard(),
@@ -117,61 +101,33 @@ func newHTTPRouteRetractFixture(
}, prov, updater
}
-func reconcileRetractHTTPRoute(t *testing.T, r *HTTPRouteReconciler)
(ctrl.Result, error) {
+func grpcAcceptedConditionFromUpdater(t *testing.T, updater *recordingUpdater)
*metav1.Condition {
t.Helper()
- return r.Reconcile(context.Background(), ctrl.Request{
- NamespacedName: k8stypes.NamespacedName{Namespace:
retractRouteNamespace, Name: retractRouteName},
- })
-}
-
-var retractRouteKey = k8stypes.NamespacedName{Namespace:
retractRouteNamespace, Name: retractRouteName}
-
-// Narrowing a listener's allowedRoutes leaves the HTTPRoute in place but
stops it
-// being accepted. The configuration an earlier reconcile published must be
-// retracted, otherwise the data plane keeps serving a route the Gateway no
longer
-// admits and only deleting the HTTPRoute clears it.
-func TestHTTPRouteReconcile_RetractsWhenListenerStopsAllowingRoute(t
*testing.T) {
- r, prov, updater := newHTTPRouteRetractFixture(t,
gatewayv1.NamespacesFromSame)
-
- result, err := reconcileRetractHTTPRoute(t, r)
-
- require.NoError(t, err)
- assert.Equal(t, ctrl.Result{}, result)
- assert.Equal(t, []k8stypes.NamespacedName{retractRouteKey},
prov.deleted)
- assert.Zero(t, prov.updated, "a route that is not accepted must not be
published")
-
- // The reason must say why this parent rejected the route. The
route-wide
- // status derived from filterHostnames would otherwise report
- // NoMatchingListenerHostname, which is a symptom of nothing having
matched
- // rather than the cause.
- require.Len(t, updater.updates, 1)
- mutated, ok :=
updater.updates[0].Mutator.Mutate(&gatewayv1.HTTPRoute{}).(*gatewayv1.HTTPRoute)
+ require.NotEmpty(t, updater.updates)
+ mutated, ok :=
updater.updates[0].Mutator.Mutate(&gatewayv1.GRPCRoute{}).(*gatewayv1.GRPCRoute)
require.True(t, ok)
require.Len(t, mutated.Status.Parents, 1)
accepted :=
meta.FindStatusCondition(mutated.Status.Parents[0].Conditions,
string(gatewayv1.RouteConditionAccepted))
require.NotNil(t, accepted)
- assert.Equal(t, metav1.ConditionFalse, accepted.Status)
- assert.Equal(t, string(gatewayv1.RouteReasonNotAllowedByListeners),
accepted.Reason)
+ return accepted
}
-// An accepted route must still be published and must not be retracted.
-func TestHTTPRouteReconcile_PublishesAcceptedRoute(t *testing.T) {
- r, prov, _ := newHTTPRouteRetractFixture(t, gatewayv1.NamespacesFromAll)
+// A translation / Provider.Update failure must be visible on the route status
+// instead of Accepted=True plus a silent requeue.
+func TestGRPCRouteReconcile_UpdateErrorIsVisibleOnStatus(t *testing.T) {
+ r, prov, updater := newGRPCRouteFixture(t)
+ prov.updateErr = errors.New("translation failed")
- _, err := reconcileRetractHTTPRoute(t, r)
+ _, err := r.Reconcile(context.Background(), ctrl.Request{
+ NamespacedName: k8stypes.NamespacedName{Namespace:
retractRouteNamespace, Name: retractRouteName},
+ })
- require.NoError(t, err)
- assert.Empty(t, prov.deleted, "an accepted route must not be retracted")
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "translation failed")
assert.Equal(t, 1, prov.updated)
-}
+ assert.Empty(t, prov.deleted)
-// A provider failure while retracting must surface so the reconcile is
retried.
-func TestHTTPRouteReconcile_RetractErrorIsReturned(t *testing.T) {
- r, prov, _ := newHTTPRouteRetractFixture(t,
gatewayv1.NamespacesFromSame)
- prov.deleteErr = errors.New("provider unavailable")
-
- _, err := reconcileRetractHTTPRoute(t, r)
-
- require.Error(t, err)
- assert.Contains(t, err.Error(), "provider unavailable")
+ accepted := grpcAcceptedConditionFromUpdater(t, updater)
+ assert.Equal(t, metav1.ConditionFalse, accepted.Status)
+ assert.Contains(t, accepted.Message, "translation failed")
}
diff --git a/internal/controller/httproute_controller.go
b/internal/controller/httproute_controller.go
index a2741791..5050cea9 100644
--- a/internal/controller/httproute_controller.go
+++ b/internal/controller/httproute_controller.go
@@ -254,6 +254,22 @@ func (r *HTTPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
acceptStatus.msg = err.Error()
}
+ // Translate and sync before writing Accepted=True. A translation or
+ // buildConfig failure from Provider.Update used to leave a green status
+ // and only requeue, matching neither Ingress nor ApisixGlobalRule.
+ var syncErr error
+ if isRouteAccepted(gateways) && err == nil {
+ routeToUpdate := hr
+ if filteredHTTPRoute != nil {
+ r.Log.V(1).Info("filtered httproute", "httproute",
utils.NamespacedName(filteredHTTPRoute))
+ routeToUpdate = filteredHTTPRoute
+ }
+ if syncErr = r.Provider.Update(ctx, tctx, routeToUpdate);
syncErr != nil {
+ acceptStatus.status = false
+ acceptStatus.msg = syncErr.Error()
+ }
+ }
+
// TODO: diff the old and new status
hr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0,
len(gateways))
for _, gateway := range gateways {
@@ -285,15 +301,7 @@ func (r *HTTPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (
UpdateStatus(r.Updater, r.Log, tctx)
if isRouteAccepted(gateways) && err == nil {
- routeToUpdate := hr
- if filteredHTTPRoute != nil {
- r.Log.V(1).Info("filtered httproute", "httproute",
utils.NamespacedName(filteredHTTPRoute))
- routeToUpdate = filteredHTTPRoute
- }
- if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
- return ctrl.Result{}, err
- }
- return ctrl.Result{}, nil
+ return ctrl.Result{}, syncErr
}
// The route still resolves to one of our Gateways but no parent
accepts it any
diff --git a/internal/controller/httproute_controller_retract_test.go
b/internal/controller/httproute_controller_retract_test.go
index 857c2fa5..c0c857c0 100644
--- a/internal/controller/httproute_controller_retract_test.go
+++ b/internal/controller/httproute_controller_retract_test.go
@@ -37,6 +37,7 @@ import (
"github.com/apache/apisix-ingress-controller/api/v1alpha1"
"github.com/apache/apisix-ingress-controller/internal/controller/config"
+
"github.com/apache/apisix-ingress-controller/internal/controller/indexer"
"github.com/apache/apisix-ingress-controller/internal/manager/readiness"
)
@@ -100,6 +101,10 @@ func newHTTPRouteRetractFixture(
cli := fake.NewClientBuilder().WithScheme(scheme).
WithObjects([]client.Object{gatewayClass, gateway, route}...).
WithStatusSubresource(route).
+ // The reconcile lists HTTPRoutePolicies by target, which the
fake client
+ // only serves once the index exists. No policy is under test
here.
+ WithIndex(&v1alpha1.HTTPRoutePolicy{}, indexer.PolicyTargetRefs,
+ func(client.Object) []string { return nil }).
Build()
readier := readiness.NewReadinessManager(cli, logr.Discard())
@@ -156,13 +161,16 @@ func
TestHTTPRouteReconcile_RetractsWhenListenerStopsAllowingRoute(t *testing.T)
// An accepted route must still be published and must not be retracted.
func TestHTTPRouteReconcile_PublishesAcceptedRoute(t *testing.T) {
- r, prov, _ := newHTTPRouteRetractFixture(t, gatewayv1.NamespacesFromAll)
+ r, prov, updater := newHTTPRouteRetractFixture(t,
gatewayv1.NamespacesFromAll)
_, err := reconcileRetractHTTPRoute(t, r)
require.NoError(t, err)
assert.Empty(t, prov.deleted, "an accepted route must not be retracted")
assert.Equal(t, 1, prov.updated)
+
+ accepted := acceptedConditionFromUpdater(t, updater)
+ assert.Equal(t, metav1.ConditionTrue, accepted.Status)
}
// A provider failure while retracting must surface so the reconcile is
retried.
@@ -175,3 +183,32 @@ func TestHTTPRouteReconcile_RetractErrorIsReturned(t
*testing.T) {
require.Error(t, err)
assert.Contains(t, err.Error(), "provider unavailable")
}
+
+func acceptedConditionFromUpdater(t *testing.T, updater *recordingUpdater)
*metav1.Condition {
+ t.Helper()
+ require.NotEmpty(t, updater.updates)
+ mutated, ok :=
updater.updates[0].Mutator.Mutate(&gatewayv1.HTTPRoute{}).(*gatewayv1.HTTPRoute)
+ require.True(t, ok)
+ require.Len(t, mutated.Status.Parents, 1)
+ accepted :=
meta.FindStatusCondition(mutated.Status.Parents[0].Conditions,
string(gatewayv1.RouteConditionAccepted))
+ require.NotNil(t, accepted)
+ return accepted
+}
+
+// A translation / Provider.Update failure must be visible on the route status
+// instead of Accepted=True plus a silent requeue.
+func TestHTTPRouteReconcile_UpdateErrorIsVisibleOnStatus(t *testing.T) {
+ r, prov, updater := newHTTPRouteRetractFixture(t,
gatewayv1.NamespacesFromAll)
+ prov.updateErr = errors.New("translation failed")
+
+ _, err := reconcileRetractHTTPRoute(t, r)
+
+ require.Error(t, err)
+ assert.Contains(t, err.Error(), "translation failed")
+ assert.Equal(t, 1, prov.updated)
+ assert.Empty(t, prov.deleted)
+
+ accepted := acceptedConditionFromUpdater(t, updater)
+ assert.Equal(t, metav1.ConditionFalse, accepted.Status)
+ assert.Contains(t, accepted.Message, "translation failed")
+}
diff --git a/internal/controller/tcproute_controller.go
b/internal/controller/tcproute_controller.go
index b8d6f97e..8428a55e 100644
--- a/internal/controller/tcproute_controller.go
+++ b/internal/controller/tcproute_controller.go
@@ -348,6 +348,16 @@ func (r *TCPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
if r.supportsL4RoutePolicy {
ProcessL4RoutePolicy(r.Client, r.Log, tctx, tr.Namespace,
tr.Name, KindTCPRoute)
}
+ // Translate and sync before writing Accepted=True. A translation or
+ // buildConfig failure from Provider.Update used to leave a green status
+ // and only requeue, matching neither Ingress nor ApisixGlobalRule.
+ var syncErr error
+ if isRouteAccepted(gateways) {
+ if syncErr = r.Provider.Update(ctx, tctx, tr); syncErr != nil {
+ acceptStatus.status = false
+ acceptStatus.msg = syncErr.Error()
+ }
+ }
tr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0,
len(gateways))
for _, gateway := range gateways {
parentStatus := gatewayv1.RouteParentStatus{}
@@ -377,11 +387,7 @@ func (r *TCPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
})
UpdateStatus(r.Updater, r.Log, tctx)
if isRouteAccepted(gateways) {
- routeToUpdate := tr
- if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
- return ctrl.Result{}, err
- }
- return ctrl.Result{}, nil
+ return ctrl.Result{}, syncErr
}
// The route still resolves to one of our Gateways but no parent
accepts it any
diff --git a/internal/controller/tlsroute_controller.go
b/internal/controller/tlsroute_controller.go
index 779b7603..f1b1792f 100644
--- a/internal/controller/tlsroute_controller.go
+++ b/internal/controller/tlsroute_controller.go
@@ -348,6 +348,16 @@ func (r *TLSRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
if r.supportsL4RoutePolicy {
ProcessL4RoutePolicy(r.Client, r.Log, tctx, tr.Namespace,
tr.Name, types.KindTLSRoute)
}
+ // Translate and sync before writing Accepted=True. A translation or
+ // buildConfig failure from Provider.Update used to leave a green status
+ // and only requeue, matching neither Ingress nor ApisixGlobalRule.
+ var syncErr error
+ if isRouteAccepted(gateways) {
+ if syncErr = r.Provider.Update(ctx, tctx, tr); syncErr != nil {
+ acceptStatus.status = false
+ acceptStatus.msg = syncErr.Error()
+ }
+ }
tr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0,
len(gateways))
for _, gateway := range gateways {
parentStatus := gatewayv1.RouteParentStatus{}
@@ -377,11 +387,7 @@ func (r *TLSRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
})
UpdateStatus(r.Updater, r.Log, tctx)
if isRouteAccepted(gateways) {
- routeToUpdate := tr
- if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
- return ctrl.Result{}, err
- }
- return ctrl.Result{}, nil
+ return ctrl.Result{}, syncErr
}
// The route still resolves to one of our Gateways but no parent
accepts it any
diff --git a/internal/controller/udproute_controller.go
b/internal/controller/udproute_controller.go
index 3ca88907..a9475941 100644
--- a/internal/controller/udproute_controller.go
+++ b/internal/controller/udproute_controller.go
@@ -348,6 +348,16 @@ func (r *UDPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
if r.supportsL4RoutePolicy {
ProcessL4RoutePolicy(r.Client, r.Log, tctx, tr.Namespace,
tr.Name, KindUDPRoute)
}
+ // Translate and sync before writing Accepted=True. A translation or
+ // buildConfig failure from Provider.Update used to leave a green status
+ // and only requeue, matching neither Ingress nor ApisixGlobalRule.
+ var syncErr error
+ if isRouteAccepted(gateways) {
+ if syncErr = r.Provider.Update(ctx, tctx, tr); syncErr != nil {
+ acceptStatus.status = false
+ acceptStatus.msg = syncErr.Error()
+ }
+ }
tr.Status.Parents = make([]gatewayv1.RouteParentStatus, 0,
len(gateways))
for _, gateway := range gateways {
parentStatus := gatewayv1.RouteParentStatus{}
@@ -377,11 +387,7 @@ func (r *UDPRouteReconciler) Reconcile(ctx
context.Context, req ctrl.Request) (c
})
UpdateStatus(r.Updater, r.Log, tctx)
if isRouteAccepted(gateways) {
- routeToUpdate := tr
- if err := r.Provider.Update(ctx, tctx, routeToUpdate); err !=
nil {
- return ctrl.Result{}, err
- }
- return ctrl.Result{}, nil
+ return ctrl.Result{}, syncErr
}
// The route still resolves to one of our Gateways but no parent
accepts it any
